From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id E8B07C369C9 for ; Thu, 17 Apr 2025 20:45:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B96256B02A4; Thu, 17 Apr 2025 16:45:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B448B6B02A5; Thu, 17 Apr 2025 16:45:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9E3826B02A7; Thu, 17 Apr 2025 16:45:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 7FEBF6B02A4 for ; Thu, 17 Apr 2025 16:45:02 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 99A9C1C8F45 for ; Thu, 17 Apr 2025 20:45:03 +0000 (UTC) X-FDA: 83344715286.28.D60CC26 Received: from mail-pf1-f175.google.com (mail-pf1-f175.google.com [209.85.210.175]) by imf10.hostedemail.com (Postfix) with ESMTP id 945C3C0006 for ; Thu, 17 Apr 2025 20:45:01 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=TZ+UMzaq; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf10.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.210.175 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1744922701; a=rsa-sha256; cv=none; b=P7F6yqqlcOYshQEHFIMisj/8KuXnOh+OE9IyjexlTCwxBS3DaqNvMXAg9Q0pwyXhUadzhR 3903y/dpEOX5sQ/s+9CKP6oeU/Rej1z31oM7H8Kl6dmnUjGRaL7numw66wjdeokCu+ra5Q Ty03QJdN3UY/8XWNZxSli3IV99xPGeg= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=TZ+UMzaq; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf10.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.210.175 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1744922701; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=y1WebkH56zb7G6DhIHiiBnNyOX71aQ4HiSgmHnZkjHA=; b=uudvfQkfz/YiLHsSqvXrMxk93YA3rlhzcsVvU9W7J86F3IHI1r804adAsjypsUEdjPRCX0 FUIMpTHnxbPK+6y5EvbL9nc7wXX9OgUsFATg63X9CCUv/9VyFU8Skhgxhv4+OeBY0ZT0pU eFgmkq4iropo8C1rA28Pgv2bReM0yMg= Received: by mail-pf1-f175.google.com with SMTP id d2e1a72fcca58-73bb647eb23so1157974b3a.0 for ; Thu, 17 Apr 2025 13:45:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1744922700; x=1745527500; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=y1WebkH56zb7G6DhIHiiBnNyOX71aQ4HiSgmHnZkjHA=; b=TZ+UMzaq2g8MNkdTy4UCFqH+kEFKB+bpMPNkflp65CKLrUey9UrTPzjupUonsunrSB oVNLF05jHCgUAs4wXf52gxzC/GF/rog0ScBg9ZT7d9nHevwE3i1BRTumGMHCPDIoTw97 /fJTCvQ0SoRFBQdlY4yLF/2RCtbJqne/sOgcTAOBTGoyh/2LBhoyj9yWEfWsdKIZIbLJ YykpISeihHB6zNJqbuHgY0d+qnplZYlKgphymK2kclQmdnrwsLFThL4vTS3ZmK31q/U1 sKWVRw6OHNKnxtBPGYwtt8JhfFSZK+T0/hdxeud5nJR0n++2kXsF1hCpd+qQg/pw7lZA ZwJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1744922700; x=1745527500; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=y1WebkH56zb7G6DhIHiiBnNyOX71aQ4HiSgmHnZkjHA=; b=VufSUqTNLOQQCG1TjrQThJzifoaxSbdLARNpw7WIaP2tvLm/28KdrviJz4NTv18JVJ 4801hzVSFV75UcDTDxJyTrT2lOJsKcRK/wnnpJCBPOJBkNT9UdRCCBgBtXgJEdoreZ5W oK9SaSUIfhOUgXfEMeQx+3bZ1P8JwO/x+O1kOBTKbm1/g7CybQ8vXuxNaEhbJwMYaeTG Lwn1ja/omlXzVRRbAuobe1OE8D+xsC5eIsu53w7grUftoZQE8ipn0s8hdPog16OHYzyz f2JFHfAKwJ+Mzx98xLtpxJQ8PLhwdsxk744GZpsUP9uTbp6FtJgaX2DLbFXT6iAvXeF6 Pmeg== X-Forwarded-Encrypted: i=1; AJvYcCWsAqOgYPVWz+Uvo6okZldN+inDA50hBTKQldf6MKVhI3a1k2EdMIiGK4dsreRiSpJGA0q3ZdU4ag==@kvack.org X-Gm-Message-State: AOJu0YywWAm/SzYEhVub+XpGw1MHMG13R0LPRFyikKutM6NdCa/sBcTk 35yyrnDH0KIkW/AK23eufA2aSJ1b2U3KfWfeRVLBumnUQ9Kz04d5YM9tElngDdviePU4/YB8ezq gIaxemc2gnN2+3EAhOobxssoEr/MumA== X-Gm-Gg: ASbGnctyfOw0QUMNAU6LoFI6FIdcDOeE3y0+WuqehW85gE+sOqTT/iwuP5q8iAT1Hby z60MmEaEH7hrh+XNRtJVzhiDzlEittyGPPi5q625sGdanjZhIT/bzb1WcPETbEoykEE0UjWsKeF OCUWOsnrAiWNL2T6T3XhIn0Z+51LUW7xdvK8LVqw== X-Google-Smtp-Source: AGHT+IH6P+rw0rH+i/tJDRg48raNeT0ozLCzudndX44Z5cDiOIUS9XvrR4olo4WXstj23esSjt2Szx8eednW22u0uDM= X-Received: by 2002:a05:6a00:928e:b0:736:51ab:7aed with SMTP id d2e1a72fcca58-73dc1582877mr324322b3a.16.1744922700380; Thu, 17 Apr 2025 13:45:00 -0700 (PDT) MIME-Version: 1.0 References: <20250417152808.722409-1-mykyta.yatsenko5@gmail.com> In-Reply-To: <20250417152808.722409-1-mykyta.yatsenko5@gmail.com> From: Andrii Nakryiko Date: Thu, 17 Apr 2025 13:44:48 -0700 X-Gm-Features: ATxdqUF0nWldrdhHiQzfKQVvVttGVxo_-gQ6-QjL6ahNuQWYtCX_lC8NolP6WVg Message-ID: Subject: Re: [PATCH mm] maccess: fix strncpy_from_user_nofault empty string handling To: Mykyta Yatsenko , rostedt@goodmis.org Cc: akpm@linux-foundation.org, linux-mm@kvack.org, mhiramat@kernel.org, andrii@kernel.org, kernel-team@meta.com, linux-kernel@vger.kernel.org, Mykyta Yatsenko Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 945C3C0006 X-Stat-Signature: 5qptorow6ra57gdiuijukhe7j59ohaf4 X-Rspam-User: X-HE-Tag: 1744922701-812364 X-HE-Meta: U2FsdGVkX18lL+D71bRQe3/cPVmzUbB1WXI5CN9+DShNctk6/bSceXcIsdyBzixWGvRtudAzLmswPa76SPK7EWZG6gY1LCdhU0KBjkJN16OKPu/7aH8VacrK+ka0fus/omRoXyeY03Qb7EvL7cImaENXPc7NkjEzBGmTuMOJyJWzWsIyM59JMR6ZbLsnQS33WklusvYxW2VyyCxRlEBK0Wlv1kRfIdcZLJBhUIBAsPXGc2mM4a1P94WqA6AfFtKcEThAyIWdmhL+Q8vL1Hs+wIAhxF5SPeoJWy8kNxjyRlFSUJuvhspLLA3Xcp8BjE/e3HCNyhaqfqWVbLOjgscM9pSf+Kkp9Xd5Q62vGqqnY4NCIE2qkC0yI73Yc+WHBJhqRp5lCepUxu9fWI/P78j87XeUKqIsxUA8wyDoK9l4a1NbZrHSU06jHfMlH6r33Qs8wSJ5gDrdoraXxR5MyB9cgBEcwlf4DOuVJ/6IS9C6iJrT5+0Ie/ZWpR2djjtd4rEMwDnHgRhwXjxziZL198qVk+yP1lbZxfucjfnMJR8zfDcwwFlM6SeTPOcuEpx1N3SJnnt/4PSVy8rMiGAeYhaCwnSgSB6aOGDDqiI+PzS1Ky3dzt1cqcjMwfYSj35v4+UhRNfcx13xLe/tqhXxN+TDFtpwSgoFm0YeKpNB06JCKW2CmvmUpAYKrjyR94PKfCZLyLPA3rtV3cJZZ+Amk6sQhX0FG5gCyAs3Eu5lMY4Vqov1mN5Q8OYll1a6/MBr2Cxi6eMm9+TUULhN+pJMBMzoSIH7csRf30NjZlpXgk+Tb+eorE+2w9kDKgMHALHdrJZPq6cHwdB1jvgm3mYqS1raLYMD1MZlL5Hg2kvSWBJMgfcLZpPYUvOoVg0FYDN4Yb1NqCG8lee3UQnHe360nCsSV0qER24gbiLXkT5xl+jJOpeHdtSTH1h9YXV6EzUYw8MY/WAHMxpT741yiZZWf2H rnf8GN2X aHB4s1pUIFJgJV3oTp1UdLY7VHSs9ZgkAG/jxj3DygMWjqNeJoYll6vffHXY7Z4NZa5PqMqUmGnkBp1zLGFyADV8IOQ4AGwjWZ5a6HKrL9PA6k0V1Q5UoYU3R3L/SomVGLuWQyaQaT3y61T/HlJtaa2EE6bQL0vwLW5U2b0Ma6+jmfQnsOBT1JqEYaPO1WWK0uK13rqknvjT5iozWcVfPqfcIE+Erp1HARVBD083RI1z+gBdBJJYpYYRHesGGuzhLjsZuFBDM5tKfg5iXRLGju2e11wMZYbjNrFs2r5KJ9MPD7KtCu3YEFagL8b6sv2IyzyZXnqHd/+tPkEa2up0nAYWjhoim4FdfMLJ9lqbL9k5qxKX8R097pjVByZBZaJY5jErIJl/OdX+ERpnGh6a5yRsJELF4F6rsvXOQkwW+qymP09zMDlPQzasPRMK9K6/GlK1cAGoWwC4rhMCKxHXb6XLXUqDMyQOa2+o5c7+MZZ4aAARsLdMaeCQwoGX5xtwGQ5hJnwfWWKUYxSj7I63igtLc2jtTKIF/tfOxKe0ec3UN4K6cZCfnDjn46w6Rlo8v2JJC X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, Apr 17, 2025 at 8:28=E2=80=AFAM Mykyta Yatsenko wrote: > > From: Mykyta Yatsenko > > 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 > --- > 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_even= ts_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 =3D ubuf->buffer; > > /* For safety, do not trust the string pointer */ > - if (!strncpy_from_kernel_nofault(kstr, str, USTRING_BUF_SIZE)) > + cnt =3D strncpy_from_kernel_nofault(kstr, str, USTRING_BUF_SIZE); > + /* Return null if empty string or error */ > + if (cnt <=3D 1) > return NULL; I wouldn't touch this part and leave it up to Steven to fix (if he agrees it needs fixing). Current logic seems wrong already, as it won't correctly handle -EFAULT. And, on the other hand, there is nothing wrong or special about empty string, so I don't think it needs special handling. Let's drop these changes in trace_events_filter.c? > 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 =3D (char __user *)str; > - if (!strncpy_from_user_nofault(kstr, ustr, USTRING_BUF_SIZE)) > + cnt =3D strncpy_from_user_nofault(kstr, ustr, USTRING_BUF_SIZE); > + /* Return null if empty string or error */ > + if (cnt <=3D 1) > return NULL; ditto > > 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 >=3D count) { > ret =3D count; > dst[ret - 1] =3D '\0'; > - } else if (ret > 0) { > + } else if (ret >=3D 0) { > ret++; > } > This part looks good and does indeed fix the issue. Good catch! Reviewed-by: Andrii Nakryiko > -- > 2.49.0 >