* [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