| Message ID | 20200904154547.3836-4-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | gpio: mockup: support dynamically created and removed chips | expand |
On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Provide a uaccess helper that allows callers to copy a single line from > user memory. This is useful for debugfs write callbacks. Doesn't mm/util.c provides us something like this? strndup_user()? > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > --- > include/linux/uaccess.h | 3 +++ > lib/usercopy.c | 37 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 40 insertions(+) > > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h > index 94b285411659..5aedd8ac5c31 100644 > --- a/include/linux/uaccess.h > +++ b/include/linux/uaccess.h > @@ -333,6 +333,9 @@ long strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr, > long count); > long strnlen_user_nofault(const void __user *unsafe_addr, long count); > > +ssize_t getline_from_user(char *dst, size_t dst_size, > + const char __user *src, size_t src_size); > + > /** > * get_kernel_nofault(): safely attempt to read from a location > * @val: read into this variable > diff --git a/lib/usercopy.c b/lib/usercopy.c > index b26509f112f9..55aaaf93d847 100644 > --- a/lib/usercopy.c > +++ b/lib/usercopy.c > @@ -87,3 +87,40 @@ int check_zeroed_user(const void __user *from, size_t size) > return -EFAULT; > } > EXPORT_SYMBOL(check_zeroed_user); > + > +/** > + * getline_from_user - Copy a single line from user > + * @dst: Where to copy the line to > + * @dst_size: Size of the destination buffer > + * @src: Where to copy the line from > + * @src_size: Size of the source user buffer > + * > + * Copies a number of characters from given user buffer into the dst buffer. > + * The number of bytes is limited to the lesser of the sizes of both buffers. > + * If the copied string contains a newline, its first occurrence is replaced > + * by a NULL byte in the destination buffer. Otherwise the function ensures > + * the copied string is NULL-terminated. > + * > + * Returns the number of copied bytes or a negative error number on failure. > + */ > + > +ssize_t getline_from_user(char *dst, size_t dst_size, > + const char __user *src, size_t src_size) > +{ > + size_t size = min_t(size_t, dst_size, src_size); > + char *c; > + int ret; > + > + ret = copy_from_user(dst, src, size); > + if (ret) > + return -EFAULT; > + > + dst[size - 1] = '\0'; > + > + c = strchrnul(dst, '\n'); > + if (*c) > + *c = '\0'; > + > + return c - dst; > +} > +EXPORT_SYMBOL(getline_from_user); > -- > 2.26.1 >
On Fri, Sep 4, 2020 at 6:35 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > Provide a uaccess helper that allows callers to copy a single line from > > user memory. This is useful for debugfs write callbacks. > > Doesn't mm/util.c provides us something like this? > strndup_user()? > Yes, there's both strndup_user() as well as strncpy_from_user(). The problem is that they rely on the strings being NULL-terminated. This is not guaranteed for debugfs file_operations write callbacks. We need some helper that takes the minimum of bytes provided by userspace and the buffer size and figure out how many bytes to actually copy IMO. Bart
On Mon, Sep 7, 2020 at 1:05 PM Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > On Fri, Sep 4, 2020 at 6:35 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Doesn't mm/util.c provides us something like this? > > strndup_user()? > > > > Yes, there's both strndup_user() as well as strncpy_from_user(). The > problem is that they rely on the strings being NULL-terminated. This > is not guaranteed for debugfs file_operations write callbacks. We need > some helper that takes the minimum of bytes provided by userspace and > the buffer size and figure out how many bytes to actually copy IMO. Wouldn't this [1] approach work? [1]: https://elixir.bootlin.com/linux/v5.9-rc3/source/arch/x86/kernel/cpu/mtrr/if.c#L93
On Mon, Sep 7, 2020 at 12:19 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Mon, Sep 7, 2020 at 1:05 PM Bartosz Golaszewski > <bgolaszewski@baylibre.com> wrote: > > On Fri, Sep 4, 2020 at 6:35 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote: > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > Doesn't mm/util.c provides us something like this? > > > strndup_user()? > > > > > > > Yes, there's both strndup_user() as well as strncpy_from_user(). The > > problem is that they rely on the strings being NULL-terminated. This > > is not guaranteed for debugfs file_operations write callbacks. We need > > some helper that takes the minimum of bytes provided by userspace and > > the buffer size and figure out how many bytes to actually copy IMO. > > Wouldn't this [1] approach work? > > [1]: https://elixir.bootlin.com/linux/v5.9-rc3/source/arch/x86/kernel/cpu/mtrr/if.c#L93 > Sure, but this is pretty much what I do in getline_from_user(). If anything we should port mtrr_write() to using getline_from_user() once it's available upstream, no? Bart
On Mon, Sep 07, 2020 at 12:28:05PM +0200, Bartosz Golaszewski wrote: > On Mon, Sep 7, 2020 at 12:19 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > > > On Mon, Sep 7, 2020 at 1:05 PM Bartosz Golaszewski > > <bgolaszewski@baylibre.com> wrote: > > > On Fri, Sep 4, 2020 at 6:35 PM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote: > > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > > > Doesn't mm/util.c provides us something like this? > > > > strndup_user()? > > > > > > > > > > Yes, there's both strndup_user() as well as strncpy_from_user(). The > > > problem is that they rely on the strings being NULL-terminated. This > > > is not guaranteed for debugfs file_operations write callbacks. We need > > > some helper that takes the minimum of bytes provided by userspace and > > > the buffer size and figure out how many bytes to actually copy IMO. > > > > Wouldn't this [1] approach work? > > > > [1]: https://elixir.bootlin.com/linux/v5.9-rc3/source/arch/x86/kernel/cpu/mtrr/if.c#L93 > > > > Sure, but this is pretty much what I do in getline_from_user(). If > anything we should port mtrr_write() to using getline_from_user() once > it's available upstream, no? But you may provide getline_from_user() as inline in the same header where strncpy_from_user() is declared. It will be like 3 LOCs?
On Mon, Sep 7, 2020 at 1:45 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Mon, Sep 07, 2020 at 12:28:05PM +0200, Bartosz Golaszewski wrote: > > On Mon, Sep 7, 2020 at 12:19 PM Andy Shevchenko > > <andy.shevchenko@gmail.com> wrote: > > > > > > On Mon, Sep 7, 2020 at 1:05 PM Bartosz Golaszewski > > > <bgolaszewski@baylibre.com> wrote: > > > > On Fri, Sep 4, 2020 at 6:35 PM Andy Shevchenko > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote: > > > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > > > > > Doesn't mm/util.c provides us something like this? > > > > > strndup_user()? > > > > > > > > > > > > > Yes, there's both strndup_user() as well as strncpy_from_user(). The > > > > problem is that they rely on the strings being NULL-terminated. This > > > > is not guaranteed for debugfs file_operations write callbacks. We need > > > > some helper that takes the minimum of bytes provided by userspace and > > > > the buffer size and figure out how many bytes to actually copy IMO. > > > > > > Wouldn't this [1] approach work? > > > > > > [1]: https://elixir.bootlin.com/linux/v5.9-rc3/source/arch/x86/kernel/cpu/mtrr/if.c#L93 > > > > > > > Sure, but this is pretty much what I do in getline_from_user(). If > > anything we should port mtrr_write() to using getline_from_user() once > > it's available upstream, no? > > But you may provide getline_from_user() as inline in the same header where > strncpy_from_user() is declared. It will be like 3 LOCs? > May be more than that. I'll see what I can do. Bart
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h index 94b285411659..5aedd8ac5c31 100644 --- a/include/linux/uaccess.h +++ b/include/linux/uaccess.h @@ -333,6 +333,9 @@ long strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr, long count); long strnlen_user_nofault(const void __user *unsafe_addr, long count); +ssize_t getline_from_user(char *dst, size_t dst_size, + const char __user *src, size_t src_size); + /** * get_kernel_nofault(): safely attempt to read from a location * @val: read into this variable diff --git a/lib/usercopy.c b/lib/usercopy.c index b26509f112f9..55aaaf93d847 100644 --- a/lib/usercopy.c +++ b/lib/usercopy.c @@ -87,3 +87,40 @@ int check_zeroed_user(const void __user *from, size_t size) return -EFAULT; } EXPORT_SYMBOL(check_zeroed_user); + +/** + * getline_from_user - Copy a single line from user + * @dst: Where to copy the line to + * @dst_size: Size of the destination buffer + * @src: Where to copy the line from + * @src_size: Size of the source user buffer + * + * Copies a number of characters from given user buffer into the dst buffer. + * The number of bytes is limited to the lesser of the sizes of both buffers. + * If the copied string contains a newline, its first occurrence is replaced + * by a NULL byte in the destination buffer. Otherwise the function ensures + * the copied string is NULL-terminated. + * + * Returns the number of copied bytes or a negative error number on failure. + */ + +ssize_t getline_from_user(char *dst, size_t dst_size, + const char __user *src, size_t src_size) +{ + size_t size = min_t(size_t, dst_size, src_size); + char *c; + int ret; + + ret = copy_from_user(dst, src, size); + if (ret) + return -EFAULT; + + dst[size - 1] = '\0'; + + c = strchrnul(dst, '\n'); + if (*c) + *c = '\0'; + + return c - dst; +} +EXPORT_SYMBOL(getline_from_user);