public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] uaccess: Introduce copy_from_user_nul helper
@ 2026-01-12  7:30 Fushuai Wang
  2026-01-12  7:30 ` [PATCH v2 1/6] uaccess: Add " Fushuai Wang
                   ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Fushuai Wang @ 2026-01-12  7:30 UTC (permalink / raw)
  To: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
	vmalik, kees, dave.hansen, luto, mingo, bp, hpa, rostedt,
	mhiramat, brauner, jack, cyphar
  Cc: linux-kernel, x86, linux-trace-kernel, wangfushuai

From: Fushuai Wang <wangfushuai@baidu.com>

Hi all,

This patch series introduces a new helper function, copy_from_user_nul,
to simplify cases where a buffer copied from userspace needs to be
NUL-terminated. In many places, after using copy_from_user, we manually
add a NUL byte to the end of the buffer to prevent string overflows and
ensure safe string handling.

The new helper performs both the copy and the NUL-termination in one
step, reducing repetitive code and the chance of errors.

v1 -> v2: Rewrite commit message and add some actual code improvements

--WANG

Fushuai Wang (6):
  uaccess: Add copy_from_user_nul helper
  x86/tlb: Use copy_from_user_nul() instead of copy_from_user()
  tracing: Use copy_from_user_nul() instead of copy_from_user()
  userns: Use copy_from_user_nul() instead of copy_from_user()
  time: Use copy_from_user_nul() instead of copy_from_user()
  kstrtox: Use copy_from_user_nul() instead of copy_from_user()

 arch/x86/mm/tlb.c         |  3 +--
 include/linux/uaccess.h   | 19 +++++++++++++++++++
 kernel/time/test_udelay.c |  3 +--
 kernel/trace/trace.c      |  3 +--
 kernel/user_namespace.c   |  3 +--
 lib/kstrtox.c             |  6 ++----
 6 files changed, 25 insertions(+), 12 deletions(-)

-- 
2.36.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
  2026-01-12  7:30 [PATCH v2 0/6] uaccess: Introduce copy_from_user_nul helper Fushuai Wang
@ 2026-01-12  7:30 ` Fushuai Wang
  2026-01-12  9:23   ` Alice Ryhl
  2026-01-13 18:00   ` Yury Norov
  2026-01-12  7:30 ` [PATCH v2 2/6] x86/tlb: Use copy_from_user_nul() instead of copy_from_user() Fushuai Wang
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 21+ messages in thread
From: Fushuai Wang @ 2026-01-12  7:30 UTC (permalink / raw)
  To: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
	vmalik, kees, dave.hansen, luto, mingo, bp, hpa, rostedt,
	mhiramat, brauner, jack, cyphar
  Cc: linux-kernel, x86, linux-trace-kernel, wangfushuai

From: Fushuai Wang <wangfushuai@baidu.com>

Many places call copy_from_user() to copy a buffer from user space,
and then manually add a NULL terminator to the destination buffer,
e.g.:

	if (copy_from_user(dest, src, len))
		return -EFAULT;
	dest[len] = '\0';

This is repetitive and error-prone. Add a copy_from_user_nul() helper to
simplify such patterns. It copied n bytes from user space to kernel space,
and NUL-terminates the destination buffer.

Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
---
 include/linux/uaccess.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 1f3804245c06..fe9a1db600c7 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -224,6 +224,25 @@ copy_from_user(void *to, const void __user *from, unsigned long n)
 #endif
 }
 
+/*
+ * copy_from_user_nul - Copy a block of data from user space and NUL-terminate
+ *
+ * @to:   Destination address, in kernel space. This buffer must be at least
+ *        @n+1 bytes long!
+ * @from: Source address, in user space.
+ * @n:    Number of bytes to copy.
+ *
+ * Return: 0 on success, -EFAULT on failure.
+ */
+static __always_inline int __must_check
+copy_from_user_nul(void *to, const void __user *from, unsigned long n)
+{
+	if (copy_from_user(to, from, n))
+		return -EFAULT;
+	((char *)to)[n] = '\0';
+	return 0;
+}
+
 static __always_inline unsigned long __must_check
 copy_to_user(void __user *to, const void *from, unsigned long n)
 {
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 2/6] x86/tlb: Use copy_from_user_nul() instead of copy_from_user()
  2026-01-12  7:30 [PATCH v2 0/6] uaccess: Introduce copy_from_user_nul helper Fushuai Wang
  2026-01-12  7:30 ` [PATCH v2 1/6] uaccess: Add " Fushuai Wang
@ 2026-01-12  7:30 ` Fushuai Wang
  2026-01-12  7:30 ` [PATCH v2 3/6] tracing: " Fushuai Wang
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Fushuai Wang @ 2026-01-12  7:30 UTC (permalink / raw)
  To: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
	vmalik, kees, dave.hansen, luto, mingo, bp, hpa, rostedt,
	mhiramat, brauner, jack, cyphar
  Cc: linux-kernel, x86, linux-trace-kernel, wangfushuai

From: Fushuai Wang <wangfushuai@baidu.com>

Use copy_from_user_nul() instead of copy_from_user() to simplify
the code.

No functional change.

Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
---
 arch/x86/mm/tlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index f5b93e01e347..ae81e5cb510e 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -1807,10 +1807,9 @@ static ssize_t tlbflush_write_file(struct file *file,
 	int ceiling;
 
 	len = min(count, sizeof(buf) - 1);
-	if (copy_from_user(buf, user_buf, len))
+	if (copy_from_user_nul(buf, user_buf, len))
 		return -EFAULT;
 
-	buf[len] = '\0';
 	if (kstrtoint(buf, 0, &ceiling))
 		return -EINVAL;
 
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 3/6] tracing: Use copy_from_user_nul() instead of copy_from_user()
  2026-01-12  7:30 [PATCH v2 0/6] uaccess: Introduce copy_from_user_nul helper Fushuai Wang
  2026-01-12  7:30 ` [PATCH v2 1/6] uaccess: Add " Fushuai Wang
  2026-01-12  7:30 ` [PATCH v2 2/6] x86/tlb: Use copy_from_user_nul() instead of copy_from_user() Fushuai Wang
@ 2026-01-12  7:30 ` Fushuai Wang
  2026-01-12 15:43   ` Steven Rostedt
  2026-01-13 17:05   ` Yury Norov
  2026-01-12  7:30 ` [PATCH v2 4/6] userns: " Fushuai Wang
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 21+ messages in thread
From: Fushuai Wang @ 2026-01-12  7:30 UTC (permalink / raw)
  To: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
	vmalik, kees, dave.hansen, luto, mingo, bp, hpa, rostedt,
	mhiramat, brauner, jack, cyphar
  Cc: linux-kernel, x86, linux-trace-kernel, wangfushuai

From: Fushuai Wang <wangfushuai@baidu.com>

Use copy_from_user_nul() instead of copy_from_user() to simplify
the code.

No functional change.

Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
---
 kernel/trace/trace.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index baec63134ab6..b6ffd006fcf9 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -11266,10 +11266,9 @@ ssize_t trace_parse_run_command(struct file *file, const char __user *buffer,
 		if (size >= WRITE_BUFSIZE)
 			size = WRITE_BUFSIZE - 1;
 
-		if (copy_from_user(kbuf, buffer + done, size))
+		if (copy_from_user_nul(kbuf, buffer + done, size))
 			return -EFAULT;
 
-		kbuf[size] = '\0';
 		buf = kbuf;
 		do {
 			tmp = strchr(buf, '\n');
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 4/6] userns: Use copy_from_user_nul() instead of copy_from_user()
  2026-01-12  7:30 [PATCH v2 0/6] uaccess: Introduce copy_from_user_nul helper Fushuai Wang
                   ` (2 preceding siblings ...)
  2026-01-12  7:30 ` [PATCH v2 3/6] tracing: " Fushuai Wang
@ 2026-01-12  7:30 ` Fushuai Wang
  2026-01-12  7:30 ` [PATCH v2 5/6] time: " Fushuai Wang
  2026-01-12  7:30 ` [PATCH v2 6/6] kstrtox: " Fushuai Wang
  5 siblings, 0 replies; 21+ messages in thread
From: Fushuai Wang @ 2026-01-12  7:30 UTC (permalink / raw)
  To: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
	vmalik, kees, dave.hansen, luto, mingo, bp, hpa, rostedt,
	mhiramat, brauner, jack, cyphar
  Cc: linux-kernel, x86, linux-trace-kernel, wangfushuai

From: Fushuai Wang <wangfushuai@baidu.com>

Use copy_from_user_nul() instead of copy_from_user() to simplify
the code.

No functional change.

Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
---
 kernel/user_namespace.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 03cb63883d04..881a05a3502c 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -1239,9 +1239,8 @@ ssize_t proc_setgroups_write(struct file *file, const char __user *buf,
 
 	/* What was written? */
 	ret = -EFAULT;
-	if (copy_from_user(kbuf, buf, count))
+	if (copy_from_user_nul(kbuf, buf, count))
 		goto out;
-	kbuf[count] = '\0';
 	pos = kbuf;
 
 	/* What is being requested? */
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 5/6] time: Use copy_from_user_nul() instead of copy_from_user()
  2026-01-12  7:30 [PATCH v2 0/6] uaccess: Introduce copy_from_user_nul helper Fushuai Wang
                   ` (3 preceding siblings ...)
  2026-01-12  7:30 ` [PATCH v2 4/6] userns: " Fushuai Wang
@ 2026-01-12  7:30 ` Fushuai Wang
  2026-01-12  7:30 ` [PATCH v2 6/6] kstrtox: " Fushuai Wang
  5 siblings, 0 replies; 21+ messages in thread
From: Fushuai Wang @ 2026-01-12  7:30 UTC (permalink / raw)
  To: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
	vmalik, kees, dave.hansen, luto, mingo, bp, hpa, rostedt,
	mhiramat, brauner, jack, cyphar
  Cc: linux-kernel, x86, linux-trace-kernel, wangfushuai

From: Fushuai Wang <wangfushuai@baidu.com>

Use copy_from_user_nul() instead of copy_from_user() to simplify
the code.

No functional change.

Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
---
 kernel/time/test_udelay.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/time/test_udelay.c b/kernel/time/test_udelay.c
index 783f2297111b..08ff8fd43471 100644
--- a/kernel/time/test_udelay.c
+++ b/kernel/time/test_udelay.c
@@ -107,9 +107,8 @@ static ssize_t udelay_test_write(struct file *file, const char __user *buf,
 	if (count >= sizeof(lbuf))
 		return -EINVAL;
 
-	if (copy_from_user(lbuf, buf, count))
+	if (copy_from_user_nul(lbuf, buf, count))
 		return -EFAULT;
-	lbuf[count] = '\0';
 
 	ret = sscanf(lbuf, "%d %d", &usecs, &iters);
 	if (ret < 1)
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 6/6] kstrtox: Use copy_from_user_nul() instead of copy_from_user()
  2026-01-12  7:30 [PATCH v2 0/6] uaccess: Introduce copy_from_user_nul helper Fushuai Wang
                   ` (4 preceding siblings ...)
  2026-01-12  7:30 ` [PATCH v2 5/6] time: " Fushuai Wang
@ 2026-01-12  7:30 ` Fushuai Wang
  5 siblings, 0 replies; 21+ messages in thread
From: Fushuai Wang @ 2026-01-12  7:30 UTC (permalink / raw)
  To: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
	vmalik, kees, dave.hansen, luto, mingo, bp, hpa, rostedt,
	mhiramat, brauner, jack, cyphar
  Cc: linux-kernel, x86, linux-trace-kernel, wangfushuai

From: Fushuai Wang <wangfushuai@baidu.com>

Use copy_from_user_nul() instead of copy_from_user() to simplify
the code.

No functional change.

Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
---
 lib/kstrtox.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index bdde40cd69d7..e4ee72b75e09 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -402,9 +402,8 @@ int kstrtobool_from_user(const char __user *s, size_t count, bool *res)
 	char buf[4];
 
 	count = min(count, sizeof(buf) - 1);
-	if (copy_from_user(buf, s, count))
+	if (copy_from_user_nul(buf, s, count))
 		return -EFAULT;
-	buf[count] = '\0';
 	return kstrtobool(buf, res);
 }
 EXPORT_SYMBOL(kstrtobool_from_user);
@@ -416,9 +415,8 @@ int f(const char __user *s, size_t count, unsigned int base, type *res)	\
 	char buf[1 + sizeof(type) * 8 + 1 + 1];				\
 									\
 	count = min(count, sizeof(buf) - 1);				\
-	if (copy_from_user(buf, s, count))				\
+	if (copy_from_user_nul(buf, s, count))				\
 		return -EFAULT;						\
-	buf[count] = '\0';						\
 	return g(buf, base, res);					\
 }									\
 EXPORT_SYMBOL(f)
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
  2026-01-12  7:30 ` [PATCH v2 1/6] uaccess: Add " Fushuai Wang
@ 2026-01-12  9:23   ` Alice Ryhl
  2026-01-12 10:00     ` Fushuai Wang
  2026-01-13 18:00   ` Yury Norov
  1 sibling, 1 reply; 21+ messages in thread
From: Alice Ryhl @ 2026-01-12  9:23 UTC (permalink / raw)
  To: Fushuai Wang
  Cc: tglx, peterz, mathieu.desnoyers, akpm, yury.norov, vmalik, kees,
	dave.hansen, luto, mingo, bp, hpa, rostedt, mhiramat, brauner,
	jack, cyphar, linux-kernel, x86, linux-trace-kernel, wangfushuai

On Mon, Jan 12, 2026 at 03:30:34PM +0800, Fushuai Wang wrote:
> From: Fushuai Wang <wangfushuai@baidu.com>
> 
> Many places call copy_from_user() to copy a buffer from user space,
> and then manually add a NULL terminator to the destination buffer,
> e.g.:
> 
> 	if (copy_from_user(dest, src, len))
> 		return -EFAULT;
> 	dest[len] = '\0';
> 
> This is repetitive and error-prone. Add a copy_from_user_nul() helper to
> simplify such patterns. It copied n bytes from user space to kernel space,
> and NUL-terminates the destination buffer.
> 
> Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>

Hmm, this function is very very similar to strncpy_from_user(). Should
they be using that instead?

Alice

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
  2026-01-12  9:23   ` Alice Ryhl
@ 2026-01-12 10:00     ` Fushuai Wang
  2026-01-12 11:20       ` Alice Ryhl
  0 siblings, 1 reply; 21+ messages in thread
From: Fushuai Wang @ 2026-01-12 10:00 UTC (permalink / raw)
  To: aliceryhl
  Cc: akpm, bp, brauner, cyphar, dave.hansen, fushuai.wang, hpa, jack,
	kees, linux-kernel, linux-trace-kernel, luto, mathieu.desnoyers,
	mhiramat, mingo, peterz, rostedt, tglx, vmalik, wangfushuai, x86,
	yury.norov

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 999 bytes --]

>> From: Fushuai Wang <wangfushuai@baidu.com>
>> 
>> Many places call copy_from_user() to copy a buffer from user space,
>> and then manually add a NULL terminator to the destination buffer,
>> e.g.:
>> 
>> 	if (copy_from_user(dest, src, len))
>> 		return -EFAULT;
>> 	dest[len] = '\0';
>> 
>> This is repetitive and error-prone. Add a copy_from_user_nul() helper to
>> simplify such patterns. It copied n bytes from user space to kernel space,
>> and NUL-terminates the destination buffer.
>> 
>> Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
>
> Hmm, this function is very very similar to strncpy_from_user(). Should
> they be using that instead?
> 
> Alice

The strncpy_from_user() is for NUL-terminated strings and stops at the
first NUL in userspace. But copy_from_user_nul() always copies a fixed length
and adds a NUL at the end in kernel space, even if userspace data doesn’t
contain a NUL.

So I think they are for different cases and can’t replace each other.

---
Regards,
WANG

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
  2026-01-12 10:00     ` Fushuai Wang
@ 2026-01-12 11:20       ` Alice Ryhl
  2026-01-12 12:22         ` Fushuai Wang
  0 siblings, 1 reply; 21+ messages in thread
From: Alice Ryhl @ 2026-01-12 11:20 UTC (permalink / raw)
  To: Fushuai Wang
  Cc: akpm, bp, brauner, cyphar, dave.hansen, hpa, jack, kees,
	linux-kernel, linux-trace-kernel, luto, mathieu.desnoyers,
	mhiramat, mingo, peterz, rostedt, tglx, vmalik, wangfushuai, x86,
	yury.norov

On Mon, Jan 12, 2026 at 11:01 AM Fushuai Wang <fushuai.wang@linux.dev> wrote:
>
> >> From: Fushuai Wang <wangfushuai@baidu.com>
> >>
> >> Many places call copy_from_user() to copy a buffer from user space,
> >> and then manually add a NULL terminator to the destination buffer,
> >> e.g.:
> >>
> >>      if (copy_from_user(dest, src, len))
> >>              return -EFAULT;
> >>      dest[len] = '\0';
> >>
> >> This is repetitive and error-prone. Add a copy_from_user_nul() helper to
> >> simplify such patterns. It copied n bytes from user space to kernel space,
> >> and NUL-terminates the destination buffer.
> >>
> >> Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
> >
> > Hmm, this function is very very similar to strncpy_from_user(). Should
> > they be using that instead?
> >
> > Alice
>
> The strncpy_from_user() is for NUL-terminated strings and stops at the
> first NUL in userspace. But copy_from_user_nul() always copies a fixed length
> and adds a NUL at the end in kernel space, even if userspace data doesn’t
> contain a NUL.
>
> So I think they are for different cases and can’t replace each other.

strncpy_from_user() succeeds even if userspace data does not contain a
nul. Then it reads length bytes.

As far as I can tell, when a nul byte is present, none of these kernel
use-cases use data after the nul byte. So the behavior is identical
except that copy_from_user_nul() may result in EFAULT if there are
unmapped bytes between the first nul byte in `src` and `src+len`.

Alice

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
  2026-01-12 11:20       ` Alice Ryhl
@ 2026-01-12 12:22         ` Fushuai Wang
  2026-01-12 13:22           ` Borislav Petkov
  0 siblings, 1 reply; 21+ messages in thread
From: Fushuai Wang @ 2026-01-12 12:22 UTC (permalink / raw)
  To: aliceryhl
  Cc: akpm, bp, brauner, cyphar, dave.hansen, fushuai.wang, hpa, jack,
	kees, linux-kernel, linux-trace-kernel, luto, mathieu.desnoyers,
	mhiramat, mingo, peterz, rostedt, tglx, vmalik, wangfushuai, x86,
	yury.norov

> strncpy_from_user() succeeds even if userspace data does not contain a
> nul. Then it reads length bytes.

Yes, but if there is no NUL byte in the user buf, whether you use
strncpy_from_user() or copy_from_user(), you need to manually add
a '\0' in the kernel buf to ensure it is properly NUL-terminated.
like:

	ret = strncpy_from_user(&buffer[0], arg, sizeof(buffer) - 1);
	if (ret < 0) {
		ret = -EFAULT;
		break;
	}
	buffer[sizeof(buffer) - 1] = '\0';

So I do not think copy_from_user() + '\0' can be instead of strncpy_from_user().
I think strncpy_from_user() can only be used without manually appending '\0'
if someone are certain that the user buf contains a NUL byte.

---
Regards,
WANG

> As far as I can tell, when a nul byte is present, none of these kernel
> use-cases use data after the nul byte. So the behavior is identical
> except that copy_from_user_nul() may result in EFAULT if there are
> unmapped bytes between the first nul byte in `src` and `src+len`.
> 
> Alice


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
  2026-01-12 12:22         ` Fushuai Wang
@ 2026-01-12 13:22           ` Borislav Petkov
  2026-01-12 15:40             ` Alice Ryhl
  0 siblings, 1 reply; 21+ messages in thread
From: Borislav Petkov @ 2026-01-12 13:22 UTC (permalink / raw)
  To: Fushuai Wang
  Cc: aliceryhl, akpm, brauner, cyphar, dave.hansen, hpa, jack, kees,
	linux-kernel, linux-trace-kernel, luto, mathieu.desnoyers,
	mhiramat, mingo, peterz, rostedt, tglx, vmalik, wangfushuai, x86,
	yury.norov

On Mon, Jan 12, 2026 at 08:22:36PM +0800, Fushuai Wang wrote:
> > strncpy_from_user() succeeds even if userspace data does not contain a
> > nul. Then it reads length bytes.
> 
> Yes, but if there is no NUL byte in the user buf, whether you use
> strncpy_from_user() or copy_from_user(), you need to manually add
> a '\0' in the kernel buf to ensure it is properly NUL-terminated.

This looks like a bunch of churn to save a "= \0" line.

The more important question, IMO, would be whether there are cases in the
kernel which *miss* a NUL termination, audit them and fix them.

That'll give you a better idea whether such a *_nul() helper is even needed.

Because converting only a handful of obvious places in the face of thousands
of copy_from_user() invocations in the kernel is not doing anything useful.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
  2026-01-12 13:22           ` Borislav Petkov
@ 2026-01-12 15:40             ` Alice Ryhl
  2026-01-12 16:28               ` Steven Rostedt
  0 siblings, 1 reply; 21+ messages in thread
From: Alice Ryhl @ 2026-01-12 15:40 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Fushuai Wang, akpm, brauner, cyphar, dave.hansen, hpa, jack, kees,
	linux-kernel, linux-trace-kernel, luto, mathieu.desnoyers,
	mhiramat, mingo, peterz, rostedt, tglx, vmalik, wangfushuai, x86,
	yury.norov

On Mon, Jan 12, 2026 at 02:22:38PM +0100, Borislav Petkov wrote:
> On Mon, Jan 12, 2026 at 08:22:36PM +0800, Fushuai Wang wrote:
> > > strncpy_from_user() succeeds even if userspace data does not contain a
> > > nul. Then it reads length bytes.
> > 
> > Yes, but if there is no NUL byte in the user buf, whether you use
> > strncpy_from_user() or copy_from_user(), you need to manually add
> > a '\0' in the kernel buf to ensure it is properly NUL-terminated.
> 
> This looks like a bunch of churn to save a "= \0" line.
> 
> The more important question, IMO, would be whether there are cases in the
> kernel which *miss* a NUL termination, audit them and fix them.
> 
> That'll give you a better idea whether such a *_nul() helper is even needed.
> 
> Because converting only a handful of obvious places in the face of thousands
> of copy_from_user() invocations in the kernel is not doing anything useful.

I'm getting the impression that strncpy_from_user() is the real problem
here. It should append a nul byte.

That would also be a lot less error-prone than the suggested new API.
Requiring a length of N+1 is not the most immediately-obvious API.

Alice

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 3/6] tracing: Use copy_from_user_nul() instead of copy_from_user()
  2026-01-12  7:30 ` [PATCH v2 3/6] tracing: " Fushuai Wang
@ 2026-01-12 15:43   ` Steven Rostedt
  2026-01-13 17:05   ` Yury Norov
  1 sibling, 0 replies; 21+ messages in thread
From: Steven Rostedt @ 2026-01-12 15:43 UTC (permalink / raw)
  To: Fushuai Wang
  Cc: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
	vmalik, kees, dave.hansen, luto, mingo, bp, hpa, mhiramat,
	brauner, jack, cyphar, linux-kernel, x86, linux-trace-kernel,
	wangfushuai

On Mon, 12 Jan 2026 15:30:36 +0800
Fushuai Wang <fushuai.wang@linux.dev> wrote:

> From: Fushuai Wang <wangfushuai@baidu.com>
> 
> Use copy_from_user_nul() instead of copy_from_user() to simplify
> the code.
> 
> No functional change.
> 
> Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>

Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve

> ---
>  kernel/trace/trace.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index baec63134ab6..b6ffd006fcf9 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -11266,10 +11266,9 @@ ssize_t trace_parse_run_command(struct file *file, const char __user *buffer,
>  		if (size >= WRITE_BUFSIZE)
>  			size = WRITE_BUFSIZE - 1;
>  
> -		if (copy_from_user(kbuf, buffer + done, size))
> +		if (copy_from_user_nul(kbuf, buffer + done, size))
>  			return -EFAULT;
>  
> -		kbuf[size] = '\0';
>  		buf = kbuf;
>  		do {
>  			tmp = strchr(buf, '\n');


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
  2026-01-12 15:40             ` Alice Ryhl
@ 2026-01-12 16:28               ` Steven Rostedt
  2026-01-12 16:37                 ` Alice Ryhl
  0 siblings, 1 reply; 21+ messages in thread
From: Steven Rostedt @ 2026-01-12 16:28 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Borislav Petkov, Fushuai Wang, akpm, brauner, cyphar, dave.hansen,
	hpa, jack, kees, linux-kernel, linux-trace-kernel, luto,
	mathieu.desnoyers, mhiramat, mingo, peterz, tglx, vmalik,
	wangfushuai, x86, yury.norov

On Mon, 12 Jan 2026 15:40:53 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> I'm getting the impression that strncpy_from_user() is the real problem
> here. It should append a nul byte.
> 
> That would also be a lot less error-prone than the suggested new API.
> Requiring a length of N+1 is not the most immediately-obvious API.

Should we make a strscpy_from_user() API that acts the same as strscpy()
but from user space?

-- Steve

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
  2026-01-12 16:28               ` Steven Rostedt
@ 2026-01-12 16:37                 ` Alice Ryhl
  0 siblings, 0 replies; 21+ messages in thread
From: Alice Ryhl @ 2026-01-12 16:37 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Borislav Petkov, Fushuai Wang, akpm, brauner, cyphar, dave.hansen,
	hpa, jack, kees, linux-kernel, linux-trace-kernel, luto,
	mathieu.desnoyers, mhiramat, mingo, peterz, tglx, vmalik,
	wangfushuai, x86, yury.norov

On Mon, Jan 12, 2026 at 5:28 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Mon, 12 Jan 2026 15:40:53 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
>
> > I'm getting the impression that strncpy_from_user() is the real problem
> > here. It should append a nul byte.
> >
> > That would also be a lot less error-prone than the suggested new API.
> > Requiring a length of N+1 is not the most immediately-obvious API.
>
> Should we make a strscpy_from_user() API that acts the same as strscpy()
> but from user space?

Sounds quite reasonable to me.

Alice

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 3/6] tracing: Use copy_from_user_nul() instead of copy_from_user()
  2026-01-12  7:30 ` [PATCH v2 3/6] tracing: " Fushuai Wang
  2026-01-12 15:43   ` Steven Rostedt
@ 2026-01-13 17:05   ` Yury Norov
  2026-01-13 18:03     ` Steven Rostedt
  1 sibling, 1 reply; 21+ messages in thread
From: Yury Norov @ 2026-01-13 17:05 UTC (permalink / raw)
  To: Fushuai Wang
  Cc: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
	vmalik, kees, dave.hansen, luto, mingo, bp, hpa, rostedt,
	mhiramat, brauner, jack, cyphar, linux-kernel, x86,
	linux-trace-kernel, wangfushuai

On Mon, Jan 12, 2026 at 03:30:36PM +0800, Fushuai Wang wrote:
> From: Fushuai Wang <wangfushuai@baidu.com>
> 
> Use copy_from_user_nul() instead of copy_from_user() to simplify
> the code.
> 
> No functional change.
> 
> Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
> ---
>  kernel/trace/trace.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index baec63134ab6..b6ffd006fcf9 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -11266,10 +11266,9 @@ ssize_t trace_parse_run_command(struct file *file, const char __user *buffer,
>  		if (size >= WRITE_BUFSIZE)
>  			size = WRITE_BUFSIZE - 1;
>  
> -		if (copy_from_user(kbuf, buffer + done, size))
> +		if (copy_from_user_nul(kbuf, buffer + done, size))
>  			return -EFAULT;

This hides the original error. Can you switch it to:

                err = copy_xxx();
                if (err)
                        return err;

I understand that in this case EFAULT is the only possible error, but
the above pattern is really error-prone, and is reproduced again and
again over the kernel.

> -		kbuf[size] = '\0';
>  		buf = kbuf;
>  		do {
>  			tmp = strchr(buf, '\n');
> -- 
> 2.36.1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
  2026-01-12  7:30 ` [PATCH v2 1/6] uaccess: Add " Fushuai Wang
  2026-01-12  9:23   ` Alice Ryhl
@ 2026-01-13 18:00   ` Yury Norov
  2026-01-13 18:17     ` Yury Norov
  2026-01-16  8:42     ` Fushuai Wang
  1 sibling, 2 replies; 21+ messages in thread
From: Yury Norov @ 2026-01-13 18:00 UTC (permalink / raw)
  To: Fushuai Wang
  Cc: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
	vmalik, kees, dave.hansen, luto, mingo, bp, hpa, rostedt,
	mhiramat, brauner, jack, cyphar, linux-kernel, x86,
	linux-trace-kernel, wangfushuai

On Mon, Jan 12, 2026 at 03:30:34PM +0800, Fushuai Wang wrote:
> From: Fushuai Wang <wangfushuai@baidu.com>
> 
> Many places call copy_from_user() to copy a buffer from user space,
> and then manually add a NULL terminator to the destination buffer,
> e.g.:

6 is not many

> 
> 	if (copy_from_user(dest, src, len))
> 		return -EFAULT;
> 	dest[len] = '\0';
> 
> This is repetitive and error-prone. Add a copy_from_user_nul() helper to
> simplify such patterns. It copied n bytes from user space to kernel space,
> and NUL-terminates the destination buffer.
> 
> Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>

I checked the cases you've found, and all them clearly abuse
copy_from_user(). For example, #2 in tlbflush_write_file():

        if (copy_from_user(buf, user_buf, len))
                return -EFAULT;

        buf[len] = '\0';
        if (kstrtoint(buf, 0, &ceiling))
                return -EINVAL;

should be:
        
        len = strncpy_from_user(buf, user_buf, len);
        if (len < 0)
                return len;

        ret = kstrtoint(buf, 0, &ceiling);
        if (ret)
                return ret;

See, if you use the right API, you don't need this weird
copy_from_user_nul(). Also notice how nice the original version hides
possible ERANGE in kstrtoint().

Patches #3-5 in the series again copy strings with raw non-string API,
so should be converted to some flavor of strcpy().

#6 patches lib/kstrtox, which makes little sense because the whole
purpose of that library is to handle raw pieces of memory as valid
C strings. One would expect such patterns in library code, and I'd
prefer having them explicit.

I find copy_{from,to}_user_nul() useful for objects that must be
null-terminated, and may have \0 somewhere in the middle. Those are
not C strings. I suspect this isn't a popular format across the kernel. 

On the other hand, adding the _nul() version of copy_from_user() would
make an API abuse like above simpler, which is a bad thing.

Can you drop copy_from_user_nul() and submit a series that switches
string manipulations to the dedicated string functions?

Thanks,
Yury

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 3/6] tracing: Use copy_from_user_nul() instead of copy_from_user()
  2026-01-13 17:05   ` Yury Norov
@ 2026-01-13 18:03     ` Steven Rostedt
  0 siblings, 0 replies; 21+ messages in thread
From: Steven Rostedt @ 2026-01-13 18:03 UTC (permalink / raw)
  To: Yury Norov
  Cc: Fushuai Wang, tglx, peterz, mathieu.desnoyers, akpm, aliceryhl,
	yury.norov, vmalik, kees, dave.hansen, luto, mingo, bp, hpa,
	mhiramat, brauner, jack, cyphar, linux-kernel, x86,
	linux-trace-kernel, wangfushuai

On Tue, 13 Jan 2026 12:05:13 -0500
Yury Norov <ynorov@nvidia.com> wrote:

> On Mon, Jan 12, 2026 at 03:30:36PM +0800, Fushuai Wang wrote:
> > From: Fushuai Wang <wangfushuai@baidu.com>
> > 
> > Use copy_from_user_nul() instead of copy_from_user() to simplify
> > the code.
> > 
> > No functional change.
> > 
> > Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
> > ---
> >  kernel/trace/trace.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index baec63134ab6..b6ffd006fcf9 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -11266,10 +11266,9 @@ ssize_t trace_parse_run_command(struct file *file, const char __user *buffer,
> >  		if (size >= WRITE_BUFSIZE)
> >  			size = WRITE_BUFSIZE - 1;
> >  
> > -		if (copy_from_user(kbuf, buffer + done, size))
> > +		if (copy_from_user_nul(kbuf, buffer + done, size))
> >  			return -EFAULT;  
> 
> This hides the original error. Can you switch it to:
> 
>                 err = copy_xxx();
>                 if (err)
>                         return err;

No, the current way is fine. It's failing on reading user space. EFAULT
is good enough.

-- Steve


> 
> I understand that in this case EFAULT is the only possible error, but
> the above pattern is really error-prone, and is reproduced again and
> again over the kernel.
> 
> > -		kbuf[size] = '\0';
> >  		buf = kbuf;
> >  		do {
> >  			tmp = strchr(buf, '\n');
> > -- 
> > 2.36.1  


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
  2026-01-13 18:00   ` Yury Norov
@ 2026-01-13 18:17     ` Yury Norov
  2026-01-16  8:42     ` Fushuai Wang
  1 sibling, 0 replies; 21+ messages in thread
From: Yury Norov @ 2026-01-13 18:17 UTC (permalink / raw)
  To: Fushuai Wang
  Cc: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
	vmalik, kees, dave.hansen, luto, mingo, bp, hpa, rostedt,
	mhiramat, brauner, jack, cyphar, linux-kernel, x86,
	linux-trace-kernel, wangfushuai

On Tue, Jan 13, 2026 at 01:00:25PM -0500, Yury Norov wrote:
> On Mon, Jan 12, 2026 at 03:30:34PM +0800, Fushuai Wang wrote:
> > From: Fushuai Wang <wangfushuai@baidu.com>
> > 
> > Many places call copy_from_user() to copy a buffer from user space,
> > and then manually add a NULL terminator to the destination buffer,
> > e.g.:
> 
> 6 is not many
> 
> > 
> > 	if (copy_from_user(dest, src, len))
> > 		return -EFAULT;
> > 	dest[len] = '\0';
> > 
> > This is repetitive and error-prone. Add a copy_from_user_nul() helper to
> > simplify such patterns. It copied n bytes from user space to kernel space,
> > and NUL-terminates the destination buffer.
> > 
> > Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
> 
> I checked the cases you've found, and all them clearly abuse
> copy_from_user(). For example, #2 in tlbflush_write_file():
> 
>         if (copy_from_user(buf, user_buf, len))
>                 return -EFAULT;
> 
>         buf[len] = '\0';
>         if (kstrtoint(buf, 0, &ceiling))
>                 return -EINVAL;
> 
> should be:
>         
>         len = strncpy_from_user(buf, user_buf, len);
>         if (len < 0)
>                 return len;
> 
>         ret = kstrtoint(buf, 0, &ceiling);
>         if (ret)
>                 return ret;
> 
> See, if you use the right API, you don't need this weird
> copy_from_user_nul(). Also notice how nice the original version hides
> possible ERANGE in kstrtoint().

Huh, we actually already have kstrtoint_from_user, so this should be a
one-liner.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
  2026-01-13 18:00   ` Yury Norov
  2026-01-13 18:17     ` Yury Norov
@ 2026-01-16  8:42     ` Fushuai Wang
  1 sibling, 0 replies; 21+ messages in thread
From: Fushuai Wang @ 2026-01-16  8:42 UTC (permalink / raw)
  To: ynorov
  Cc: akpm, aliceryhl, bp, brauner, cyphar, dave.hansen, fushuai.wang,
	hpa, jack, kees, linux-kernel, linux-trace-kernel, luto,
	mathieu.desnoyers, mhiramat, mingo, peterz, rostedt, tglx, vmalik,
	wangfushuai, x86, yury.norov

> I checked the cases you've found, and all them clearly abuse
> copy_from_user(). For example, #2 in tlbflush_write_file():
> 
>         if (copy_from_user(buf, user_buf, len))
>                 return -EFAULT;
> 
>         buf[len] = '\0';
>         if (kstrtoint(buf, 0, &ceiling))
>                 return -EINVAL;
> 
> should be:
>         
>         len = strncpy_from_user(buf, user_buf, len);
>         if (len < 0)
>                 return len;
> 
>         ret = kstrtoint(buf, 0, &ceiling);
>         if (ret)
>                 return ret;
> 
> See, if you use the right API, you don't need this weird
> copy_from_user_nul(). Also notice how nice the original version hides
> possible ERANGE in kstrtoint().
> 
> Patches #3-5 in the series again copy strings with raw non-string API,
> so should be converted to some flavor of strcpy().
> 
> #6 patches lib/kstrtox, which makes little sense because the whole
> purpose of that library is to handle raw pieces of memory as valid
> C strings. One would expect such patterns in library code, and I'd
> prefer having them explicit.
> 
> I find copy_{from,to}_user_nul() useful for objects that must be
> null-terminated, and may have \0 somewhere in the middle. Those are
> not C strings. I suspect this isn't a popular format across the kernel. 
> 
> On the other hand, adding the _nul() version of copy_from_user() would
> make an API abuse like above simpler, which is a bad thing.
> 
> Can you drop copy_from_user_nul() and submit a series that switches
> string manipulations to the dedicated string functions?

OK, I find some misuse of strncpy_from_user() + kstrtoXXX(). I will fix
them.

Regarding patches #3-5, as Steven mentioned, I believe we might need a
strscpy_from_user() for these cases that copy a non-NUL-terminated string
from userspace?

---
Regards,
WANG

> Thanks,
> Yury

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2026-01-16  8:43 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-12  7:30 [PATCH v2 0/6] uaccess: Introduce copy_from_user_nul helper Fushuai Wang
2026-01-12  7:30 ` [PATCH v2 1/6] uaccess: Add " Fushuai Wang
2026-01-12  9:23   ` Alice Ryhl
2026-01-12 10:00     ` Fushuai Wang
2026-01-12 11:20       ` Alice Ryhl
2026-01-12 12:22         ` Fushuai Wang
2026-01-12 13:22           ` Borislav Petkov
2026-01-12 15:40             ` Alice Ryhl
2026-01-12 16:28               ` Steven Rostedt
2026-01-12 16:37                 ` Alice Ryhl
2026-01-13 18:00   ` Yury Norov
2026-01-13 18:17     ` Yury Norov
2026-01-16  8:42     ` Fushuai Wang
2026-01-12  7:30 ` [PATCH v2 2/6] x86/tlb: Use copy_from_user_nul() instead of copy_from_user() Fushuai Wang
2026-01-12  7:30 ` [PATCH v2 3/6] tracing: " Fushuai Wang
2026-01-12 15:43   ` Steven Rostedt
2026-01-13 17:05   ` Yury Norov
2026-01-13 18:03     ` Steven Rostedt
2026-01-12  7:30 ` [PATCH v2 4/6] userns: " Fushuai Wang
2026-01-12  7:30 ` [PATCH v2 5/6] time: " Fushuai Wang
2026-01-12  7:30 ` [PATCH v2 6/6] kstrtox: " Fushuai Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox