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 95DB3C369C9 for ; Thu, 17 Apr 2025 20:48:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1E9102800CE; Thu, 17 Apr 2025 16:48:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 19C2E2800C8; Thu, 17 Apr 2025 16:48:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 061942800CE; Thu, 17 Apr 2025 16:48:01 -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 D9B852800C8 for ; Thu, 17 Apr 2025 16:48:01 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 3ADB2B05D2 for ; Thu, 17 Apr 2025 20:48:03 +0000 (UTC) X-FDA: 83344722846.20.1D07350 Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com [209.85.210.169]) by imf06.hostedemail.com (Postfix) with ESMTP id 46230180004 for ; Thu, 17 Apr 2025 20:48:01 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=D1TizaAM; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf06.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.210.169 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1744922881; a=rsa-sha256; cv=none; b=Zw0ILzUJh3IFJVz9nmX2whGYYHAZbJQuC+2ZtN2IerVF0CTvIaXRaZHu7SX3AH9cu4ac8+ U+LEKyIN2sqJKVMmUNj5mOSLghsJYd2azj8PjNx87y68eG+dXwjJyeWXJsbjOvLUOqHcKk QiFj8lRfM0guG3sxJo7NXyzCGD5sQAg= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=D1TizaAM; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf06.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.210.169 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=1744922881; 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=DsyUznqqCfi4i2SQdmivlu5NHQdFlcXIhsGSIh7PHa8=; b=if6TwTPSFekCZAT84IdE3XgRUCIbJwyH3/K1IoKtLLRUyElrm5TMKI/OI2n3suVYSBBjsb 4O9CltvVk0nNlDJj4rLRAUoPKIe5wlxp1fBuF7IvJ08lmhY5HKxj/wYm1fagW/06fJQDuD W1uYt/zy4EOpROM7lYXqcVk572TzjaE= Received: by mail-pf1-f169.google.com with SMTP id d2e1a72fcca58-736b350a22cso1123130b3a.1 for ; Thu, 17 Apr 2025 13:48:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1744922880; x=1745527680; 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=DsyUznqqCfi4i2SQdmivlu5NHQdFlcXIhsGSIh7PHa8=; b=D1TizaAMZ1ttmHXbVx0A0RYLasEIlZK4uGZtHwzaPsHgNq8aQSjeZ2JMCA27CH7kll YmEcfAWpQmihXnFBPfaXIroI1JxZ3aPII8oxvSUtpiD2LtEvHu+uglAfly3ZltlcxgHe us7EUBGICmgueyuUSqRhw0rWmLIv/c/asOE7J+EExz4XXLbcJzUa8ttYM98tlIRvgTm5 +Ie+TWqO+2gHas+cD7z6u3A3+j092/tyDLGun+2pU47tkYYNh307yHjxS4RykIldm+Li E3q/D5Agq2YyfdBkdk275DlY7964CfaS0xb5R5s7tK+9k2Lvban3n2LUQyK/hMm6hSgb 2Kww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1744922880; x=1745527680; 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=DsyUznqqCfi4i2SQdmivlu5NHQdFlcXIhsGSIh7PHa8=; b=Xd+dsiP/c65b44XNXL3B+218/NLoCJumPL+Em4zZF5LZoZ8D+OOsQ5oCbNGInZZUkP 5Fk2F0qP9+vgm13wWGjksYrTfh+QWQHhkzOZ2gMP8bnHjqV6qPa8hIQXA7fjzkKJAxCO /f45u7b8gQcerq7vHiqY+OBTS9q4o72QCLKDxwMQEFoPW55+i+YLdzHnANOisBFr8Dr9 WU9+KNNK2fIH9vZSdtjznDHNbu5eJo04rg7qj0Edd4Whz3XBWSD8Heo156kxymA5oD9j D/NAagM23VhIbHLfCzlA+iQmGnEYmBAuRxr9aoMcF1MucFKMF047fI2hnomjneOE+5xG wuLg== X-Forwarded-Encrypted: i=1; AJvYcCVz5Y9gl1O3icW4z6CTNyOGG2BRhKyB3v/wTX2NjxjIXs7ZPOrD/Yj66hgcy7hHzvTgFhtGDK68fQ==@kvack.org X-Gm-Message-State: AOJu0YyAVCnro0JfSNVLAmj8DMIlfYLjbneipBXlvmeEv//pgrGMLMkf OavHAvy/o04Ppn3IuUBL9Q+xhu4/D0PvjydEdn6XnJ1uxJQvYG9PaX99vCLLSJfd2aaS/4HHcF5 aQA7NYWSGXOkCSzJ7Ql/RrMfy/Ew= X-Gm-Gg: ASbGncvqVp3bq3J9UjThXz7ciDJPmdRh4HfZFvcGLX/2QrSCMl9E91zuT4NSOH3qdh9 IpqLuD2KqhquCrMHlM1buXmW0E0KRokjfWxBsZxIbFga1F0M9RV8Oq2se7qMBU/ms+LIFSKxZBp 99sMk/KvV5sbQY8UFi0ZScYicLd7qp6gYwiUcAkQ== X-Google-Smtp-Source: AGHT+IGkf09SLgRPWC20TV11LMw4EqhtPcAEX96o2A9jKGoNu1a05Gc9kqv8Ppmu44QBbwoG/+CIiEwBFzlP40dlJN4= X-Received: by 2002:a05:6a00:ac0b:b0:736:a82a:58ad with SMTP id d2e1a72fcca58-73dc156a999mr310377b3a.15.1744922880183; Thu, 17 Apr 2025 13:48:00 -0700 (PDT) MIME-Version: 1.0 References: <20250417152808.722409-1-mykyta.yatsenko5@gmail.com> In-Reply-To: From: Andrii Nakryiko Date: Thu, 17 Apr 2025 13:47:48 -0700 X-Gm-Features: ATxdqUEfmguRvvXaFJojHSceTsB5sS3hyibIXzxDMwO9_tjc8wqpOgu1m_hMy4Y 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-Queue-Id: 46230180004 X-Rspam-User: X-Rspamd-Server: rspam07 X-Stat-Signature: 84swkb57c3pqiaubdwzcexgok5s7mqx4 X-HE-Tag: 1744922881-834591 X-HE-Meta: U2FsdGVkX189gDBHrF4fRONBXp4hZucbbs5JiV8qjO9U3CrJMbhSTWPWilMiu8ALbXTpTC7Hi1bqkixKV7MsCJo+cfKK1RuWuR3Pv2d6d7D73WQBlxzqpehILjxpikEhdY0MVx4VvDTgXOhcOaPp33LDCxpUDfkg0hbJagN/ZzCZ4uOv6DWM5l0y1yP9LhXx2LCUSVG/BmCMzpVMFpujijJiCAjuPKuls/RdtWUaf/Wdykc/UipzI/fsnJLoCVDZdkjyWHcfGMXS98/breC8SP2WkqLteuLhnp3ivEIGwfbhAFB8aSGLyp6A7eqMTnsM87Kseg2YlhnVRrUOuOhelPqgiwjgu/zYjNW3KY/0kvmvfeRmDEa2dprakLJTAsFgvPTqetE37kcd+7ENKR73xjYtd30XhcTn2puBFWGKtkD+hjkqJ7tHloTVhceQQ3000j70Vy7Vj4H734OlsyxWc2+hlUUMWRzcxYgT/x/TX8I3pzAex/o3ofPnqp/S35bsEo7g9Yp1q0m0UvhFN9s5j7ySYcAogUyxh410HMPl/ImGyB8Pq6yi8+c4eEjV7WBQQl8QL9om7vWLag+YAoWL/cuGluMvSgCHoQpWhBqrXv2nJCPceFg2HRzYxSvajSJi+sTLctoMtj+baArCj28TLyTKg8JQUmMzxeHbVkcrSTjgJSP50OiD79FD/J8N4eymE43+ipPI3mYjY7CvAU8c/YO/zBfNzNz3/1wnk+PmqJ6UeexESQ83ixKCyVV+QY0n6te1AeeDrLgsKvlt6LQhxDrZeJrozkgNB5wnAZW5u+YMgxAJDc1vLILvbyNPv5/eCHWWvInPgFDgd2w7WPIrW/y8un/d9hg3dmeH1P9XdTdve6DqmfOAgYG6jfTy5fqUWgPE464uazHfWqjr3tWtPMwHOmiEVJr+jUYYVdI5POP/+Z6r2z2tzkUb0HfEvFX78EooafnxDbBDKNzF32e mhgwJGSI EffGsmsosZoFgOyufFVILpzThA+iUWgsdR2Or3beJcntZe9N/u2MDxt3KxEOA3kCW+gYYItHJJ3P2LZ2oOv5Yz4I8BrFKNsb8Py/qAOXrmWEZbxHF4co7PjwLT+ckt6JOMyfUL/xtGcVhpHRmTFacQkCtHA8tcEbrn7WG0FcPX1dzNVkS3x/s+h38ObxpJ16mOoAzuZd+syUMhr9lA/ztQkGaFsR2fR0GPrEIBwNv6mqlyzj2p2oClzBakPlbH0Yev0eRehMlp+tX3saV0l+Ub0LV9+GD4E7jcpjbO+AMAkZL9oUboGw3kfBrB88urMX4YkWWxmN50dnpgT2O0eqDi3tU9o04tOlU95McgGbsSUNFLwDgl/7bb13AYGB78A1FkD1xWVUrBVwnmGErwktaaZdDK9BOg4TqUj+eiPDVdRQS2gdUeEniJs+cz7J1rVHwLnbJl+Ig4kgPOv9HiSVfyYGFvqBx1REcaMRv8cMf5a9GxYLsZpoBvz9hff65V86lYiWsVri6HqQ1+Kdu+dzM6yiOGHqoMT3+oJNq1pcB12AdxxiJNX9IC1MfvWOYOWBmrGlX 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 1:44=E2=80=AFPM Andrii Nakryiko wrote: > > 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_ev= ents_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? Actually, you are not even touching strncpy_from_kernel_nofault(), so yeah, definitely let's not do this (at least not in this patch). > > > 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 voi= d __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 > >