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 C35A5C54E93 for ; Wed, 28 Aug 2024 10:15:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2B9456B008C; Wed, 28 Aug 2024 06:15:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2672D6B0092; Wed, 28 Aug 2024 06:15:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1093F6B0093; Wed, 28 Aug 2024 06:15:43 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id E8A386B008C for ; Wed, 28 Aug 2024 06:15:42 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 93A511C6C86 for ; Wed, 28 Aug 2024 10:15:42 +0000 (UTC) X-FDA: 82501247724.09.162CEDF Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf04.hostedemail.com (Postfix) with ESMTP id F37CE40028 for ; Wed, 28 Aug 2024 10:15:40 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="n474//dd"; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf04.hostedemail.com: domain of alx@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=alx@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724840069; a=rsa-sha256; cv=none; b=HxoaBX2PdyX3PLUItb2YBX5rAuqg0agCOuVx++gLtIXx5Wp3ZPzuRzgxPox5DqJN/6cPNR 9g9GON9XvmRAfTKMkIELFsZwNRZ4Lmukgn7p7wicIavNn9dUwj2K+gUnhL6Awdj2zBlXNt 93UA429Gm/k8re4hTH+kC5zzhRbRdzY= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="n474//dd"; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf04.hostedemail.com: domain of alx@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=alx@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724840069; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=xXmuxOeVAAqeZWKC+p2y3NlTAL+Ub1RObenNGoZKkOc=; b=h5RJTP9VoIa4hzMyZXVoKkiU5+WCxDdC2RaLcxVlesXuf9uXvVzowPQgHizC5cXNVFzcoP b7FNKzPxf50oE96c2dSXca2PYFpu9t7F7+UTHLWkh8LtO4d8uH48Y8kdWFhA4frfOc4E1Z MQEZkrJZ0wajOaQP3tp7HbPfACaToPg= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 5CB73A433E1; Wed, 28 Aug 2024 10:15:33 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A5E4DC98EC0; Wed, 28 Aug 2024 10:15:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1724840140; bh=5xuHNpJ8s4HOyC/ZMbuo393EqBOuuHgBE8NpAhMS1xo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=n474//ddYkvBDbFi4PGLE0FTy+iiAA4s4wyNdxcHGJALrL1fthrhN3uI+zqdovRS/ 144KFSBIez7oTLYMwkU1OpzSMrIJ/beg9u2IV2kfsLQ3HbVrfcjTYwUOYc0zNYGCUW sJPUqdSKe/vNZkKLA9B0zTCgnE8dCgdnbOaVHYsUF8xA9psYiMn54rrFhKkNIi3oCl dvtmwiVj6Pzfi2AhPqiPDt2mnx4XVarJ+wsJOyfm+g/Kx8+3f9FhCTGEuLGnAERvcX Jg4rftmJ9JMgCrLnqOr0dbza144I/TB8JbkMRSm3BmoN9HM3dOV8VyCx9rbu7NESVa DUUS4wBFCSJWg== Date: Wed, 28 Aug 2024 12:15:33 +0200 From: Alejandro Colomar To: Yafang Shao Cc: akpm@linux-foundation.org, torvalds@linux-foundation.org, justinstitt@google.com, ebiederm@xmission.com, alexei.starovoitov@gmail.com, rostedt@goodmis.org, catalin.marinas@arm.com, penguin-kernel@i-love.sakura.ne.jp, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, audit@vger.kernel.org, linux-security-module@vger.kernel.org, selinux@vger.kernel.org, bpf@vger.kernel.org, netdev@vger.kernel.org, dri-devel@lists.freedesktop.org, Alexander Viro , Christian Brauner , Jan Kara , Kees Cook , Matus Jokay , "Serge E. Hallyn" Subject: Re: [PATCH v8 1/8] Get rid of __get_task_comm() Message-ID: References: <20240828030321.20688-1-laoar.shao@gmail.com> <20240828030321.20688-2-laoar.shao@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="74qvuoge5bg2djof" Content-Disposition: inline In-Reply-To: <20240828030321.20688-2-laoar.shao@gmail.com> X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: F37CE40028 X-Stat-Signature: 4bmq5zcz9xo1m57izk9og89gq7dizt4n X-Rspam-User: X-HE-Tag: 1724840140-352464 X-HE-Meta: U2FsdGVkX1+db7nZJOrfLFYNUMrqyiDJyTiFM+MK8V/80+drwKhnjBF2jCda9TxI3T4d3RyZ4ncFye1QQoNd953oev0aY85fW2B95RcfTNJftcQxOI/S6tr7CNO1rfw/M0u6I/iOcgftPwIP/RASFh4j4RvY/YDqAWtVqZxv4isVATdB27A003nN+o2TGcJZte9zZ3czaEDeZE5q7nJO8gzhi9N5LU8caZnY73vUbMuMozrATxJp81OCQJW3oLqgWNcLZGaq8WZASof+baoXzcYhez00N2yDxN8LkCjyD61fNXL6HQIatbFB4Zw43hgMh7sbUJLbYncOo2Qo+ZLWkWdFCwnGbX2AtcyWutZyCr0Hjza70Nsh9DWzwMv4/xfQDwiGSLmWjnbzC05d4j7hETsRxnLYK3Z9RkgDDx2PJp6FR+PIKWRpSxAftwCPQsBIRD3X1SZy5bfMPyJf6iK6Kd2BptBAa4GVHfSroPO4EMXNXwM3HPSbmqBlEn+Z3s5hVEUCIzqzV2YjStRcUXElBbp/9eyjhBPAXZC8wlaG6SCvIDsdT3u71eAk0bcjIUSjLInAhjhogVaXchGOdZt5kEehDLGPO+1h9SRmfqIfu/vXcBRsESP6jmWMppRTJfgduRNMFLvxPZxr1oQINMgMf3obwieTS9N8ruo/2wREM7VBxUa7ypxPDNRsw6zgu6c9CcEOIL1vUY+A6XmaodlUd08IQqP+n9TYvw1wQSLuiLF0/ZvUTJbX7QWODD2jmgf12lh9tdBU/u7V+41ixv1oITCOO/3oAxopar9Z+5SSwugmjl5J1Zleo8IwwVW3RTZ1vlrOyOudPbB7CZh+DmdbF9lzSkxUIY1KCSbZo1Iz4l8zKuAU6E7QlAlfl+7xuwr0QvhMGTRsh4ADSA2j5OLI8o475JXdc+JC6c5I/lOIbLzq9fbgMu/c0W/jr8YS4KrHggWbnRL/QDEpimD81DA +XP4UCob yY5xBGra/4/ht0x/xm/oi1MYkTRSsHEUcBubtD30+YsL1mT7rZz2jn3svamdGhVQZGqnH2Tic3NAlDB2RUnY4SSOOtI9sqCJDphWULJlnowAS5wFPm+CAFRTKdqMTvPVOgTf6A+u/7qagOwaxYPzsso0lWFMbCkAEtdd/OViCkUUdeCHISqmRrpSSQ1T15faa0bVAQIIATPQSyZXGG1UghGc9sknOdrcC89MExx3OUk8ugYNtFUB/mSMtMgfDciZ7Me92HBHsT8smUqrJRwwKzdyrjG+5ZwaNWkUKSOuAfsJ433+tFBXxjBNDsC3JsaPHskl8aFiqcwxxWZk4uk7rSyTuoMSYs4viPLuFH6RiJ0oqTpDYfArZ3ahOzBCzRLGl01nIDTTornw1bhbnScaggM5KEafDGYzVr+dv23V11En7+bSx3KoHHG0D7WRaDVXZJXtqRIT6SkAvqmwJto2o1SWdrB0tpp0xj8lp7mruA35wTsx6naphp+0iviJQ/omXT2Wnaa2fDq+dqUorsV8FszpvIR2/VIABl7y/B3Kfe9FOKpu3U3qoamBt7/zci7IkjV9w53NoYDX/QT6lwJdrJ84Hckwwa+fCs9S4yjfXZZdNYBSVaIwUJRB1SEMVxMXhqM6Mig2d7ksUSQ53Cxq/2szZ0RHWN90JD4NbswYkr0V8VvF3UYbveBO4TzLGWbANK614HjlNOcsSz7DinhHYt1rtZWXSuZ8zbQ1CxGCkMlxWjIoh0KqmGejhKT9eaNXJ9kyeJHS1J2KBnwJMIJ3FqVOyKAvE738M4G+NSeLGz8Ceh8P8hw349tNQBrv2gbAGTZpwrct0RJTn02+2Gat0kxrNxJwxx+vfDNzY5gV/CdPCIO5uQ8Zal37e4dwyoGn2a7x+xoJ4gl6TZJum0CFwbk6qoA== 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: --74qvuoge5bg2djof Content-Type: text/plain; protected-headers=v1; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable From: Alejandro Colomar To: Yafang Shao Cc: akpm@linux-foundation.org, torvalds@linux-foundation.org, justinstitt@google.com, ebiederm@xmission.com, alexei.starovoitov@gmail.com, rostedt@goodmis.org, catalin.marinas@arm.com, penguin-kernel@i-love.sakura.ne.jp, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, audit@vger.kernel.org, linux-security-module@vger.kernel.org, selinux@vger.kernel.org, bpf@vger.kernel.org, netdev@vger.kernel.org, dri-devel@lists.freedesktop.org, Alexander Viro , Christian Brauner , Jan Kara , Kees Cook , Matus Jokay , "Serge E. Hallyn" Subject: Re: [PATCH v8 1/8] Get rid of __get_task_comm() References: <20240828030321.20688-1-laoar.shao@gmail.com> <20240828030321.20688-2-laoar.shao@gmail.com> MIME-Version: 1.0 In-Reply-To: <20240828030321.20688-2-laoar.shao@gmail.com> Hi Yafang, On Wed, Aug 28, 2024 at 11:03:14AM GMT, Yafang Shao wrote: > We want to eliminate the use of __get_task_comm() for the following > reasons: >=20 > - The task_lock() is unnecessary > Quoted from Linus [0]: > : Since user space can randomly change their names anyway, using locking > : was always wrong for readers (for writers it probably does make sense > : to have some lock - although practically speaking nobody cares there > : either, but at least for a writer some kind of race could have > : long-term mixed results >=20 > - The BUILD_BUG_ON() doesn't add any value > The only requirement is to ensure that the destination buffer is a valid > array. >=20 > - Zeroing is not necessary in current use cases > To avoid confusion, we should remove it. Moreover, not zeroing could > potentially make it easier to uncover bugs. If the caller needs a > zero-padded task name, it should be explicitly handled at the call site. >=20 > Suggested-by: Linus Torvalds > Link: https://lore.kernel.org/all/CAHk-=3DwivfrF0_zvf+oj6=3D=3DSh=3D-npJo= oP8chLPEfaFV0oNYTTBA@mail.gmail.com [0] > Link: https://lore.kernel.org/all/CAHk-=3DwhWtUC-AjmGJveAETKOMeMFSTwKwu99= v7+b6AyHMmaDFA@mail.gmail.com/ > Suggested-by: Alejandro Colomar > Link: https://lore.kernel.org/all/2jxak5v6dfxlpbxhpm3ey7oup4g2lnr3ueurfbo= sf5wdo65dk4@srb3hsk72zwq > Signed-off-by: Yafang Shao > Cc: Alexander Viro > Cc: Christian Brauner > Cc: Jan Kara > Cc: Eric Biederman > Cc: Kees Cook > Cc: Alexei Starovoitov > Cc: Matus Jokay > Cc: Alejandro Colomar > Cc: "Serge E. Hallyn" > --- > fs/exec.c | 10 ---------- > fs/proc/array.c | 2 +- > include/linux/sched.h | 32 ++++++++++++++++++++++++++------ > kernel/kthread.c | 2 +- > 4 files changed, 28 insertions(+), 18 deletions(-) >=20 [...] > diff --git a/include/linux/sched.h b/include/linux/sched.h > index f8d150343d42..c40b95a79d80 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h [...] > @@ -1914,10 +1917,27 @@ static inline void set_task_comm(struct task_stru= ct *tsk, const char *from) > __set_task_comm(tsk, from, false); > } > =20 > -extern char *__get_task_comm(char *to, size_t len, struct task_struct *t= sk); > +/* [...] > + * - ARRAY_SIZE() can help ensure that @buf is indeed an array. > + */ > #define get_task_comm(buf, tsk) ({ \ > - BUILD_BUG_ON(sizeof(buf) !=3D TASK_COMM_LEN); \ > - __get_task_comm(buf, sizeof(buf), tsk); \ > + strscpy(buf, (tsk)->comm, ARRAY_SIZE(buf)); \ I see that there's a two-argument macro #define strscpy(dst, src) sized_strscpy(dst, src, sizeof(dst)) which is used in patch 2/8 diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 6f0d6fb6523f..e4ef5e57dde9 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -2730,7 +2730,7 @@ void __audit_ptrace(struct task_struct *t) context->target_uid =3D task_uid(t); context->target_sessionid =3D audit_get_sessionid(t); security_task_getsecid_obj(t, &context->target_sid); - memcpy(context->target_comm, t->comm, TASK_COMM_LEN); + strscpy(context->target_comm, t->comm); } /** I propose modifying that macro to use ARRAY_SIZE() instead of sizeof(), and then calling that macro here too. That would not only make sure that this is an array, but make sure that every call to that macro is an array. An if there are macros for similar string functions that reduce the argument with a usual sizeof(), the same thing could be done to those too. Have a lovley day! Alex > + buf; \ > }) > =20 > #ifdef CONFIG_SMP > diff --git a/kernel/kthread.c b/kernel/kthread.c > index f7be976ff88a..7d001d033cf9 100644 > --- a/kernel/kthread.c > +++ b/kernel/kthread.c > @@ -101,7 +101,7 @@ void get_kthread_comm(char *buf, size_t buf_size, str= uct task_struct *tsk) > struct kthread *kthread =3D to_kthread(tsk); > =20 > if (!kthread || !kthread->full_name) { > - __get_task_comm(buf, buf_size, tsk); > + strscpy(buf, tsk->comm, buf_size); > return; > } > =20 > --=20 > 2.43.5 >=20 --=20 --74qvuoge5bg2djof Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEE6jqH8KTroDDkXfJAnowa+77/2zIFAmbO+MUACgkQnowa+77/ 2zKy2RAAnnnQ7wIvUyQxf0H9XEIY8X2+ncAYPevk+tiPOW5bQccWRYr4HTCgFzGw ROoKl9PQMjlmswMdkJxLRBd3r2r4nwpDtvORpRfVBbyJwcEbrh1+L8l834QWQbKA sf7ADLROAhIWViIeyaFhFYw/LyBh8fwArPgzPbS1V1YbpXQeh50zpTaZG5CnTFzC OwQ02fhV2rZABw7Vc5d/ZSk0sk4ZfkfqrGKADzX2fhVYkcXYta4QU5oIYmtVlZ1K GyNHVHWcF27pDYQ/it3bKIugmz3RpyJ0uf7vYwcOwfnCeypEQquaEHAxOOuCdjGk SdObiDbJmE5L3FmMcRxHacE2bX5vSlubD9oNFhIA8nJVNN//lvEcG2f7uLUNtT9a rb5+wxYYuIoe5CY/hAm0g4R5RTen4WJdnnGuawk703XeRp7eZKvOfJHgwx+mLiln CB3EGWz6g8ieUhnYcmn3ZRH0YyStGxY/xVgWldQSJkcLJSQ/ppGk+BqeDwShUPnD lPvOwIzfDehLUj+he0PiELV787pJWEbrXHdUumbLryBlBEAnwgkz93KfZtEYjGst g/63uDKyaxh6XYNtKbmYyDC0uN3jf9JKdfNFtA73mlOaeV4EYP7DLUc9m7t9Wuq6 QXB8wxZoun63oEyvF1QFKG1QEU0q9LRvy3h/isMlJmcehTcqI3Y= =NyZ7 -----END PGP SIGNATURE----- --74qvuoge5bg2djof--