From: Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>
To: akpm@linux-foundation.org, linux-mm@kvack.org,
rostedt@goodmis.org, mhiramat@kernel.org, andrii@kernel.org,
kernel-team@meta.com, linux-kernel@vger.kernel.org
Cc: Mykyta Yatsenko <yatsenko@meta.com>
Subject: [PATCH mm] maccess: fix strncpy_from_user_nofault empty string handling
Date: Thu, 17 Apr 2025 16:28:08 +0100 [thread overview]
Message-ID: <20250417152808.722409-1-mykyta.yatsenko5@gmail.com> (raw)
From: Mykyta Yatsenko <yatsenko@meta.com>
strncpy_from_user_nofault should return the length of the copied string
including the trailing NUL, but if the argument unsafe_addr points to
an empty string ({'\0'}), the return value is 0.
This happens as strncpy_from_user copies terminal symbol into dst
and returns 0 (as expected), but strncpy_from_user_nofault does not
modify ret as it is not equal to count and not greater than 0, so 0 is
returned, which contradicts the contract.
Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
kernel/trace/trace_events_filter.c | 10 ++++++++--
mm/maccess.c | 2 +-
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 0993dfc1c5c1..86b7e5a4e235 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -800,6 +800,7 @@ static __always_inline char *test_string(char *str)
{
struct ustring_buffer *ubuf;
char *kstr;
+ int cnt;
if (!ustring_per_cpu)
return NULL;
@@ -808,7 +809,9 @@ static __always_inline char *test_string(char *str)
kstr = ubuf->buffer;
/* For safety, do not trust the string pointer */
- if (!strncpy_from_kernel_nofault(kstr, str, USTRING_BUF_SIZE))
+ cnt = strncpy_from_kernel_nofault(kstr, str, USTRING_BUF_SIZE);
+ /* Return null if empty string or error */
+ if (cnt <= 1)
return NULL;
return kstr;
}
@@ -818,6 +821,7 @@ static __always_inline char *test_ustring(char *str)
struct ustring_buffer *ubuf;
char __user *ustr;
char *kstr;
+ int cnt;
if (!ustring_per_cpu)
return NULL;
@@ -827,7 +831,9 @@ static __always_inline char *test_ustring(char *str)
/* user space address? */
ustr = (char __user *)str;
- if (!strncpy_from_user_nofault(kstr, ustr, USTRING_BUF_SIZE))
+ cnt = strncpy_from_user_nofault(kstr, ustr, USTRING_BUF_SIZE);
+ /* Return null if empty string or error */
+ if (cnt <= 1)
return NULL;
return kstr;
diff --git a/mm/maccess.c b/mm/maccess.c
index 8f0906180a94..831b4dd7296c 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -196,7 +196,7 @@ long strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr,
if (ret >= count) {
ret = count;
dst[ret - 1] = '\0';
- } else if (ret > 0) {
+ } else if (ret >= 0) {
ret++;
}
--
2.49.0
next reply other threads:[~2025-04-17 15:28 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-17 15:28 Mykyta Yatsenko [this message]
2025-04-17 20:40 ` Andrew Morton
2025-04-17 21:07 ` Andrii Nakryiko
2025-04-17 20:44 ` Andrii Nakryiko
2025-04-17 20:47 ` Andrii Nakryiko
2025-04-17 22:10 ` Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250417152808.722409-1-mykyta.yatsenko5@gmail.com \
--to=mykyta.yatsenko5@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=andrii@kernel.org \
--cc=kernel-team@meta.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhiramat@kernel.org \
--cc=rostedt@goodmis.org \
--cc=yatsenko@meta.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox