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 6B900D767FB for ; Thu, 31 Oct 2024 20:27:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F27B76B0082; Thu, 31 Oct 2024 16:27:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id ED7BF6B0083; Thu, 31 Oct 2024 16:27:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D77456B0085; Thu, 31 Oct 2024 16:27: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 B25F46B0082 for ; Thu, 31 Oct 2024 16:27:34 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 5CB40A113D for ; Thu, 31 Oct 2024 20:27:34 +0000 (UTC) X-FDA: 82735032534.28.C53C5E2 Received: from mail-vk1-f175.google.com (mail-vk1-f175.google.com [209.85.221.175]) by imf01.hostedemail.com (Postfix) with ESMTP id 15ABE40007 for ; Thu, 31 Oct 2024 20:27:10 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=EArvlwpt; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf01.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.175 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730406290; a=rsa-sha256; cv=none; b=RIfGpMd8py8jLEebKG6tUmxRoZblhZOOb5ls0xdXoR94zZCLeDyvdJCmiJJ3GeFcEtI9z2 kXKMg4OW5yV+YClTzf9TdgMWmAj/shqxaDxu4+0GeWceECIqtvH2kXHvyS/+16fA0lvvUv elfRF6cvQadMcNnky/7LrK94+bqYYE4= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=EArvlwpt; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf01.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.175 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1730406290; 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=amvcyG4D30V+zPACcGkMCnMlW1VYfRruH76cCv8fQ/Y=; b=GmkjaNkfg5DIHAE+wytwfv86iVW+5pjDipTMb8zGSJOND9Re15kcalwJm0T7//vroqbRv8 ooqsOx5c6nwRnXS+/ltV6xyH5Qaidtq3aDbA7VIzSjQoOelyNiJvQcjpGkT5rx6uhRB/qg bXHcI8fFmmCooI/8MiwGhhVCN2O/aCs= Received: by mail-vk1-f175.google.com with SMTP id 71dfb90a1353d-50d85d7db19so405612e0c.2 for ; Thu, 31 Oct 2024 13:27:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730406451; x=1731011251; 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=amvcyG4D30V+zPACcGkMCnMlW1VYfRruH76cCv8fQ/Y=; b=EArvlwptu0ZDC1vw0BFK0EMDon7SBWWiLPBOdpPgdDw09Y0n5lWPZS7cKrm2/J/VCC bWQnr8RdmboOZKXyMXTUo5WuzVZ4KhKAutvXd++5yIjN2utC1zL4AYyvtlWOUjm2r/mO KxULTWB1YZVg2QRClUkibaMWp+RluvYXE9cyatjmIekSCjtLIj8W2qOOIqZaSTa7TyV3 /pBrk2Q1QuJSL0dcyBA3VUIU9+IvR4Lq3MvMs7asYRJE4bQ6AtUrBbmVriMpXt1cEbqe YCGUL9CO7KuYELrnTf2kjtIP/oTcSmvW+ADXMwbJD8QXnvwuFypF8jC+IGUaDmmyD6Iw F4Ig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730406451; x=1731011251; 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=amvcyG4D30V+zPACcGkMCnMlW1VYfRruH76cCv8fQ/Y=; b=odKpEzqJ2vejLJ0TB81cmsagPDB9qRnp2yQ8tg9SujKcVQc8+NxKuaBmFF6g9VgIm9 MN5VCRMXBBG70Oe7JddZbS/onSnYYtLBVWIhOb2snbsgV4xYS4QIYmojNaAms1HYagIo RJYYnrwweUaYsF9/o0zUnEl0Pgd9CbCvp0IZFO0P62AALeT1rtJHHGURqKbGaZvJbwoW vgSnC5cPezuxGUJ2ULxUueaFuy/5mrYriYEwxKI7k+FL5KzaV5LqMTeQ//3L369Jw2wT e6mJznMXA8TFaZg6ZoqZ7lIdkA7AJ3Sz24faVtG/5Fl7j9teAVVbqG2/m06dlY+O0HSy KcFA== X-Forwarded-Encrypted: i=1; AJvYcCUFEZ5wZ2XZy3gcvVQDoM/AyFMoqa8GLllDEyRsClktRVvzOiTq1NxltjnauLJoL71o/cnQZxN79w==@kvack.org X-Gm-Message-State: AOJu0YxRDPFLDUMrHr9h7KHAQRmWZAXwCOGjnPjhsmJZPDl3jB8VFL7i me0WliZetZ/zlNjsA93OmvL8rfVkI+kOLsm27ywzHdOrjMONZvipHMYqxbqTJ1yFsERc0x4Nqbt zQnfmhyb08SUz80zVSgXuTlhj+q8= X-Google-Smtp-Source: AGHT+IEjP5n4EaL/lfyR2Oq4IthrHgwE9IctYNm9yJ0eUDY9obh+WLPxcfQ1PXLfkX2tfzwznjrPSroNST9yVLXbGUA= X-Received: by 2002:a05:6122:31aa:b0:50d:5be4:c39d with SMTP id 71dfb90a1353d-5106ad573c6mr5815502e0c.0.1730406451519; Thu, 31 Oct 2024 13:27:31 -0700 (PDT) MIME-Version: 1.0 References: <20241030130308.1066299-1-mcanal@igalia.com> <20241030130308.1066299-5-mcanal@igalia.com> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Fri, 1 Nov 2024 09:27:20 +1300 Message-ID: Subject: Re: [PATCH v3 4/4] mm: huge_memory: Use strscpy() instead of strcpy() To: =?UTF-8?B?TWHDrXJhIENhbmFs?= Cc: Jonathan Corbet , Andrew Morton , Hugh Dickins , David Hildenbrand , Ryan Roberts , Baolin Wang , Lance Yang , linux-mm@kvack.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-dev@igalia.com, kees@kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: wip9mn9zeiioacuu4e8frr3nakx37gz4 X-Rspamd-Queue-Id: 15ABE40007 X-Rspamd-Server: rspam08 X-Rspam-User: X-HE-Tag: 1730406430-729455 X-HE-Meta: U2FsdGVkX1/gOO9K8/lgT7JFaoxKOGGPjb+/sLa3JXpulnnjViwxaFLYENoxclxav01Bdx/D7tVo5d9KBYKdhO884Y68YkPw2dir1SUoSLp5vX/wYd5Kt0whxEA6DJBL5P4wTAvQkjmLrobL6YPjRrqgUvJiwn9g6TJimZZXYUWm8kHqiC1Bvn9XPfC65M/XzbDqIKuYrYvEWAboK/50KNOGgL4Ej1mXxiDrQCTELkSN8MlCnDY5vjS3bYs324OZPVn06TbCUMNb3LuvyU3uUiLsO15ZA05Jm2RfTnlkWdYDcUo7ySw3c1EWGQflkUsCd83/+LlvF3bTcHjJE2NgZYqnpiAKh08beWjh4DY55KcfsYoZwc/kMVvlEdKap7ez+bBY10HIb/SC2Zf6XoZashh7tD3hz8UU/jWiPAb8KTFsITNRNub1PC61ZgB35GVJrSQUESXeLhyldgPH4PEi3Um25uWLMJmlk3xlAGqtFbNmJf7RFhc+18vCx9U+0hxp5z3BMw0010iRIJzpYNPTi1QHAd+4+Th7TAJ/UqUr5Bri1A5ei5ZZ557q8glQ6gAxubbyF2FWM01sVm7XAka+2M3ZKms657g9oUi6Sd6pPFOnuX0JEomsuULzSd0FIffWH6kJXjMgFvSD4V8Lb7neliViloCYnnv8qxv19Dss7Nv2yic5byeUmNfGdYoCjndEVMKyuDE75HXTwn15/DQMClf+Cqy7B3mQqYskEzO5eeFSETQwuEwSu26au/+55RPhT/fPoO1iGWMUF/aSzx+Gd7duqcn2Q0ztROO8LL40XmZSZQ/HvnsY+7p1nrAk64VUYHF5G02kMCZZSuRlugz/6HAeZyT+YzpxLzZ2xfIhjDe9QsPf7Ldfor7eTKoaIh0lE2AIIDFNe0wKFn9Xg8B1lq4g7nRwqAt/82m5UvJXqeXV2JDFLYfoQJh37NW9vKDtFn6nfij1ygrZRRFXNIJ 6SiyetUd 375iXeq8yE7oxHTX5OFGcCt28RoMxhKbBgJqA3LkAOUbFs4nQGKxlsusA8ncSPf6uaajcBsdj0EWpAqAq+9GlUrLTlvTE1qp1c8H3HGeg/5qbxK4kaNFqZig3LO8TY3m12NOKstoTUZngBI6rb1kLP7FqfsvuNS+JnxQz5yFCfZReL7qudlmsTCwxYpppY6zZFPkPcOxEB79dkref2hb+/v1l0JHfRVZunCh4x9mMmxU+wLZyu8KF4h2gBChgSlV7+JfH1n/sF5Q+hvFnHVCXEmUEEQWleg7JDtQy6/vu7TjlepgDl/cbhwgTHk05WiqXNYXxaxnn0c3/HWHGRtCXcwKE6XMc88R0zMiY0UR8NYuD+4BfLgMjQdJQZzeAXHfbk4KX+r0YxCkHS5sr4jXOF6vP0gGYfAqvwK6SelglsbZ8SgK2XohvW0PRNqQAbGRzk+MRyDlLpuJDvtGLWXg8753fXdboEaYy3pbAuCF/LDzOXRE= 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 Fri, Nov 1, 2024 at 1:12=E2=80=AFAM Ma=C3=ADra Canal = wrote: > > +cc Kees Cook > > Hi Barry, > > On 31/10/24 09:01, Barry Song wrote: > > On Thu, Oct 31, 2024 at 6:55=E2=80=AFPM Ma=C3=ADra Canal wrote: > >> > >> Hi Barry, > >> > >> On 30/10/24 20:07, Barry Song wrote: > >>> On Thu, Oct 31, 2024 at 2:03=E2=80=AFAM Ma=C3=ADra Canal wrote: > >>>> > >>>> Replace strcpy() with strscpy() in mm/huge_memory.c > >>>> > >>>> strcpy() has been deprecated because it is generally unsafe, so help= to > >>>> eliminate it from the kernel source. > >>>> > >>>> Link: https://github.com/KSPP/linux/issues/88 > >>>> Signed-off-by: Ma=C3=ADra Canal > >>>> --- > >>>> mm/huge_memory.c | 4 ++-- > >>>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c > >>>> index f92068864469..8f41a694433c 100644 > >>>> --- a/mm/huge_memory.c > >>>> +++ b/mm/huge_memory.c > >>>> @@ -989,7 +989,7 @@ static int __init setup_thp_anon(char *str) > >>>> > >>>> if (!str || strlen(str) + 1 > PAGE_SIZE) > >>>> goto err; > >>>> - strcpy(str_dup, str); > >>>> + strscpy(str_dup, str); > >>> > >>> What is the difference between strcpy and strscpy without a size para= meter? > >>> > >>> we have already a check and goto err. strcpy() is entirely safe. > >>> if (!str || strlen(str) + 1 > PAGE_SIZE) > >>> goto err; > >>> > >>> My understanding is that we don't need this patch. > >> > >> strcpy() is a deprecated interface [1]. From the GitHub issue I linked > >> in the commit description, Kees states: "A lot of kernel code is still > >> using strcpy(). While the CONFIG_FORTIFY_SOURCE wrapper macros tend to > >> make its use mostly safe, it would be nice to eliminate the function > >> from the kernel entirely." > > > > I don't see any value added here since strscpy() has no size parameter = and > > As `str_dup` is a sized buffer, we don't need to specify the size > parameter. > > > we have checked strlen(str) + 1 > PAGE_SIZE to avoid parsing any pointl= ess > > bootcmd longer than PAGE_SIZE. > > From my point of view, this is an extra layer of safety. A developer > could change `str_dup` to SZ_4K and leave PAGE_SIZE in the check. This > could pass by in a review and we would have a check that allows strings > bigger than the destination buffer in systems using 16KB pages. > > But see that `split_huge_pages_write` uses `strcpy` without any checks. > > I can remove the internal check from `setup_thp_anon` if you feel it > would be more suitable. My point is that we shouldn't remove the strlen(str) + 1 > PAGE_SIZE check, as it could lead to situations like this: For thp_anon=3DAB, where len(A) =3D PAGE_SIZE and B is illegal, we would mistakenly interpret the string as valid by ignoring B after trimming it. Therefore, if you remove the check, you could instead do: len =3D strscpy(str_dup, src); if (len >=3D sizeof(str_dup)) err; I have no objection to replacing strcpy with strscpy, so feel free to go ahead :-) > > Best Regards, > - Ma=C3=ADra > > > > But I have no objection to this patch if other people like it. > > > >> > >> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#str= cpy > >> > >> Best Regards, > >> - Ma=C3=ADra > >> > >>> > >>>> > >>>> always =3D huge_anon_orders_always; > >>>> madvise =3D huge_anon_orders_madvise; > >>>> @@ -4175,7 +4175,7 @@ static ssize_t split_huge_pages_write(struct f= ile *file, const char __user *buf, > >>>> > >>>> tok =3D strsep(&buf, ","); > >>>> if (tok) { > >>>> - strcpy(file_path, tok); > >>>> + strscpy(file_path, tok); > >>>> } else { > >>>> ret =3D -EINVAL; > >>>> goto out; > >>>> -- > >>>> 2.46.2 > >>>> > >>> Thanks barry