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 0F6DAC369C9 for ; Thu, 17 Apr 2025 22:08:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CDA462800D6; Thu, 17 Apr 2025 18:08:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C88EB2800C8; Thu, 17 Apr 2025 18:08:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B4F8D2800D6; Thu, 17 Apr 2025 18:08:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 8C0612800C8 for ; Thu, 17 Apr 2025 18:08:34 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 00355BA5F5 for ; Thu, 17 Apr 2025 22:08:35 +0000 (UTC) X-FDA: 83344925790.10.948CDFF Received: from sea.source.kernel.org (sea.source.kernel.org [172.234.252.31]) by imf28.hostedemail.com (Postfix) with ESMTP id 331D7C0003 for ; Thu, 17 Apr 2025 22:08:33 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=none; spf=pass (imf28.hostedemail.com: domain of "SRS0=IxM/=XD=goodmis.org=rostedt@kernel.org" designates 172.234.252.31 as permitted sender) smtp.mailfrom="SRS0=IxM/=XD=goodmis.org=rostedt@kernel.org"; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1744927714; a=rsa-sha256; cv=none; b=qLRnBlBs7bbIXv0cslf/kJT+FpHIm6KemVWWkgH4+hCc1aMIXfWJcVQlsPxKNbJimJShYf V4cRGYayVFKWqyCS+VmdP1/E7EfUD3eroaFd6/k8C1I591ZaPo01DaGha2WGo7MUYOXCoF jXyz2EjhUQSXA0NfJtTaxX/xkJT3Imo= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=none; spf=pass (imf28.hostedemail.com: domain of "SRS0=IxM/=XD=goodmis.org=rostedt@kernel.org" designates 172.234.252.31 as permitted sender) smtp.mailfrom="SRS0=IxM/=XD=goodmis.org=rostedt@kernel.org"; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1744927714; 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; bh=0atE2Dz2pofik/s1ph5JtO0hoMUEdN8Raqjz1W93gUQ=; b=jr7RCAXUzxfk0MoWCqk+zDdW/N28jccCDJG/bVvg5R5LoqkfhoRTYrGb/SdzIUeQFZZPJu xCrchwk+usBHQAixlUFkGC4vX9f4jVm9+DXDtO8+DbEqvHAumQ1crm4M/LtFUkr8cVp/ZJ CkBANTa+P4qE8fYs8td/RnilAI4Q9aQ= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id CD31C4A165; Thu, 17 Apr 2025 22:08:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C14C6C4CEE4; Thu, 17 Apr 2025 22:08:31 +0000 (UTC) Date: Thu, 17 Apr 2025 18:10:10 -0400 From: Steven Rostedt To: Andrii Nakryiko Cc: Mykyta Yatsenko , 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 Subject: Re: [PATCH mm] maccess: fix strncpy_from_user_nofault empty string handling Message-ID: <20250417181010.3cc5777f@gandalf.local.home> In-Reply-To: References: <20250417152808.722409-1-mykyta.yatsenko5@gmail.com> X-Mailer: Claws Mail 3.20.0git84 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 331D7C0003 X-Stat-Signature: qa5rpitixrwecn4rrriino8s6e6x5n3c X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1744927713-817127 X-HE-Meta: U2FsdGVkX1/0sEbu2o7NIscvlsOM02oYIvNyybO9eqgzIfuMZBop0sS/HSrPOGJ+l/CREsis9XM++CNOXdcc/M/57pYPs20ZUZrCultlOmWwraZG4rp1tyQvKQY0zfTDrK5XT+SWxqMSQ7pOYXZl6XUfwV0lrtPl6SiHMEFmrH4ViiIXjo9HO3cNJFaJimb7WZNkcQhj6qXkN1Z9YUhgw96gFhJ928i9No4u/93HYBSugsfaPyigTflocAWcwR4Q51bdlf8sAo7D5s5Db+fKXdDGJK3PHtYVZ41BhUAmtvGYYjum6NEfIJ8/xEurlyJ98CcunNlIZeddCcmTbqK36l97Bok5u9bG9scp79blZ1Ts7svlyC6GC19VjSXnifNcKZITEHcvMVjZl2v3CeJX5sn19McVbIPt3WUupGl+fh94G5PyLPKGgauscViVxjgayRDKy9Y52GmUrjlaAgTDXzANRZIEr1slUhra3zqfGgLdNCD6I9gRlFPiGopb8ytPP9kTRMrEKd+93susFrFakNeiy0DY8ihp24LQSG+OlLgpSo8i4Y1D5Ya+w8MQ5TLji5AbESFsm1VLbKFzJbWRCeKZhscy3F1UxT2H6g7JFozyaT4m60ztBcLDlTxcJA4Lq4v/aBone1m1exRe2qi+pjeqFZ79L2EIMfZayezmCzmG7H4c1chbjoi91YLHz2XzLNDgBCZE3V7v8H66k0eiRGVIdt77HuFkwf1RK42AqXQnVdiKIJxpTa3MdZVvZpFmGbLGkH10rWFcpfBpo/SXc9YMesd5vLluOff6eutO6iW08SEj2oavn+KkGW5sMCqyJUV4XNTaHMhKiI+3sL/qtywwwPLZ+oehlnx/RBIbahc8b8LkBYg4VOrj1+P+bFVQ5VtWCgR8Zevdm/h8y0hO6DKJB1b2fXj54VwjQNHtjqaOl01Sd0Rp4h3FRFLyp8LHfLmsja8OfJl89JGHkk2 khoolLue fyZfP65+hho4m4o1Wur/DG1tiXhpZKEV0VJ/kikhimFHTlEyu+R082XJeKzz7/Td4R85yb577XQRnE+ufid3JX+SHFAfSH/9oKin8Xazq2z13BvgEib4QnPqUFKqnm4uSeeiSB3aPR4aSl8fE1z0wDKreluaSgtxpRxOlaJ4fowcBfSu4NLW/XMD8loG6mVB+HX3vc2a4u2c4R2fQzyJStrKbL++tftO+xFEpOrHMAoscJHHk0IauaIfJbcGN+MzRgkOzlivd1LY+s1ebG7vhoS2LwO7IWC6zx4gVI2cn7koAe+f68If0RMqekaQwbqF2+fkleS1zqIqy1mlZiiQ1eSJD2XCzDL6hZvw5xrH2wEzTor5hxJy/OFQEkfeyZBAwXdzGQdtBpcit5H4= 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, 17 Apr 2025 13:44:48 -0700 Andrii Nakryiko wrote: > > @@ -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; > > 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? Bah, it is wrong. I don't usually use filtering on strings much, but come to think of it, the last time I tried, it didn't work, but I found another way to get what I was looking for, and didn't look deeper into it. I only care if it faulted or not. I don't care about it just copying zero bytes. It should have been: if (strncpy_from_kernel_nofault(kstr, str, USTRING_BUF_SIZE) < 0) > > > 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)) This is broken too. As this isn't relying on the other change in this patch, I'll just fix it myself. I'm getting a pull request ready anyway. Thanks! -- Steve > > + cnt = strncpy_from_user_nofault(kstr, ustr, USTRING_BUF_SIZE); > > + /* Return null if empty string or error */ > > + if (cnt <= 1) > > return NULL; > > ditto > > >