From: Bartosz Golaszewski <[email protected]>
Provide a uaccess helper that allows callers to copy a single line from
user memory. This is useful for debugfs write callbacks.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
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 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
--
With Best Regards,
Andy Shevchenko
On Fri, Sep 4, 2020 at 6:35 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <[email protected]>
> >
> > 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
<[email protected]> wrote:
> On Fri, Sep 4, 2020 at 6:35 PM Andy Shevchenko
> <[email protected]> wrote:
> > On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <[email protected]>
> > 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
--
With Best Regards,
Andy Shevchenko
On Mon, Sep 7, 2020 at 12:19 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Mon, Sep 7, 2020 at 1:05 PM Bartosz Golaszewski
> <[email protected]> wrote:
> > On Fri, Sep 4, 2020 at 6:35 PM Andy Shevchenko
> > <[email protected]> wrote:
> > > On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote:
> > > > From: Bartosz Golaszewski <[email protected]>
>
> > > 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
> <[email protected]> wrote:
> >
> > On Mon, Sep 7, 2020 at 1:05 PM Bartosz Golaszewski
> > <[email protected]> wrote:
> > > On Fri, Sep 4, 2020 at 6:35 PM Andy Shevchenko
> > > <[email protected]> wrote:
> > > > On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote:
> > > > > From: Bartosz Golaszewski <[email protected]>
> >
> > > > 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?
--
With Best Regards,
Andy Shevchenko
On Mon, Sep 7, 2020 at 1:45 PM Andy Shevchenko
<[email protected]> 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
> > <[email protected]> wrote:
> > >
> > > On Mon, Sep 7, 2020 at 1:05 PM Bartosz Golaszewski
> > > <[email protected]> wrote:
> > > > On Fri, Sep 4, 2020 at 6:35 PM Andy Shevchenko
> > > > <[email protected]> wrote:
> > > > > On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote:
> > > > > > From: Bartosz Golaszewski <[email protected]>
> > >
> > > > > 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