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 437BEC54E68 for ; Thu, 21 Mar 2024 16:23:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 871236B007B; Thu, 21 Mar 2024 12:23:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7D3136B0082; Thu, 21 Mar 2024 12:23:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6732D6B0083; Thu, 21 Mar 2024 12:23:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 4FACE6B007B for ; Thu, 21 Mar 2024 12:23:47 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id C7D07A1FD9 for ; Thu, 21 Mar 2024 16:23:46 +0000 (UTC) X-FDA: 81921567252.02.1090397 Received: from out01.mta.xmission.com (out01.mta.xmission.com [166.70.13.231]) by imf01.hostedemail.com (Postfix) with ESMTP id 9E6794001C for ; Thu, 21 Mar 2024 16:23:43 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=xmission.com; spf=pass (imf01.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.231 as permitted sender) smtp.mailfrom=ebiederm@xmission.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711038223; 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; bh=pG2vThmrEqgGbaYjFbZFq3esOJ9SyW0sC1//fVwokTA=; b=hi/0/WTNlfLWG2ypOR4VqNf3m9gNSZsTdnWpIGBnx5cH2Jr6XOIkP9fLDOQJ48tcNljUnz chgchUXpTEtCIqqh3jJ7wukhsID0GG/Wf9I9dzQYNQRz08Z2T1hQKjq3Sywp4z8+l9MKbP pXhoRnHU4/yvifrIuxvtpFGcbGdwz9U= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=xmission.com; spf=pass (imf01.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.231 as permitted sender) smtp.mailfrom=ebiederm@xmission.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711038223; a=rsa-sha256; cv=none; b=qu26F68i1ZkXsna0t4AeGL8xX0gq24u7gULOUmOQ2nBap+dKzNsxEW3reBtmOKO3vZKRPW UGMGV6T9wBaTTUQaVjH2wCHm0grnQcIaAtHaLiQluYTweFqOC1UO96OK91U25RPnDifxdH 0YhKiXYICdJPFPpOuLoEkzb9o2EkGzM= Received: from in02.mta.xmission.com ([166.70.13.52]:50958) by out01.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1rnLCf-004Jao-FF; Thu, 21 Mar 2024 10:23:41 -0600 Received: from ip68-227-168-167.om.om.cox.net ([68.227.168.167]:60220 helo=email.froward.int.ebiederm.org.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1rnLCe-000QNJ-Dr; Thu, 21 Mar 2024 10:23:41 -0600 From: "Eric W. Biederman" To: Justin Stitt Cc: Alexander Viro , Christian Brauner , Jan Kara , Kees Cook , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org References: <20240321-strncpy-fs-binfmt_elf_fdpic-c-v1-1-fdde26c8989e@google.com> Date: Thu, 21 Mar 2024 11:23:34 -0500 In-Reply-To: <20240321-strncpy-fs-binfmt_elf_fdpic-c-v1-1-fdde26c8989e@google.com> (Justin Stitt's message of "Thu, 21 Mar 2024 00:10:59 +0000") Message-ID: <871q83eepl.fsf@email.froward.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1rnLCe-000QNJ-Dr;;;mid=<871q83eepl.fsf@email.froward.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.168.167;;;frm=ebiederm@xmission.com;;;spf=pass X-XM-AID: U2FsdGVkX1/OG1LEXI8jC5HSB4g9jn0q4xs9g3G9J4M= X-SA-Exim-Connect-IP: 68.227.168.167 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH] binfmt: replace deprecated strncpy with strscpy_pad X-SA-Exim-Version: 4.2.1 (built Sat, 08 Feb 2020 21:53:50 +0000) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) X-Rspamd-Queue-Id: 9E6794001C X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: dpmqntcfik9whpqtapaayen6imiesm8c X-HE-Tag: 1711038223-627286 X-HE-Meta: U2FsdGVkX180pCFy+pxtoDxZQ2E2rB358IfNNJfrxMutYRgv1R9TxA22lZzdBWYIfY/a5oOHN8rvBATqdjxbI0EnfQf48qqaJO4vO4cJHZDXd79asmm9Dyg7EqZ5ZlirQ18TbyUTcgfQi6Uw7bJNf4059DmWrb8bQIwIK2OkZPysM2rDnjhQ4hDtYuIdDHn5gaAxqI3N3ewiikhoVv0vZypKBJYsDtkZ+x9DvPZ8FKbqnB5d7jCxGxQPq0O8eqba9QR84XjAgrb/M6T8WNBF40H8EgwO4kkPvtysnUT+gkfhTh5BlIqnIKz5eR7usi7x2acz146rOUU88lIs5c+1sPypijZcOgWT9R/chPU5TX9ap6XxN7eiKCoC/rn4FKzyZdtm915Tt/7scXZAqO6fBBFtVM+68ZFczh+z8gOSwWAb7O01cvhBQJdphHvn03b/6aY61whEq/k6M9tcNN+VSPEPRMEhrHR6XU1WR8ja5U/DNTuo47I3rth2sY/Zn4j8CBdS002PmdmvoX3IfX+71O9DjgihP1XtKpV3uKq9arHU6Uf5kTHP2p7yjQQjqCb5F1iImVDaCCRW6RufcLmq1hdUTX5Fn+dQddE42Elotqna1GGzijMoFUdjhhcGQCt0ehVn0yQ5EUPFhWC4qXUWSdsP/kFlQZI0I4DHxLxfb0Hskpcyd1hHt5DPvq4jZoQXF6T1JZBbKhHjD/G+t0BsUPCD33S6tWBmnEII5ENQqlbNjvHwXuXs5ozKaANxecv2KXktgXKvXVkpaFuah5rlmwBAazIlnJPQF7xd/2Bj1RRxkbDhnCVJDNWw/+oKGB2DbdX1tMLwm2jHjOLydA6RyuLcsRA7AbgUZj3MNX8gs/FLg9Dqddbep1lktsjTz6KEuBMHe891Dy9FuTeQfwBzRTPPeYey/iz/xCu6MbMEHYBN/gGuzIBctYwlMq9VP8a9lAWnu5wgUKW1/E0zd0m Yjvcjd+t 2eJrFH3t2Y9+1hNtLK/T6QlBevME5EBx2hq7wMd6VQcASQPSwzGZXR8Egxfn4WL+Io4s+HJ0LQA1AWEIWzNo2gHvBKzwXlnNgsDnOjUF4gfxgT5+Fo//ckY2cQHRkP03NcWIK0QQXRAM+my7paJ2eJOXb/DbIrjFn1F03cV0E6oirWwhqsQBeEXgmnOmv4xVKe6DPs2OlbniUq90ocqZfOxCh5YELfD4DWmfoR7YDoLgt1q8i/rQbcV6AJdTDMjN2Lr1DwdKcoyqKwPGXO1T6m5/1BVow2Moqh/Th3hqQhZcJNtLCTM/iFQL2+Q== 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: Justin Stitt writes: > strncpy() is deprecated for use on NUL-terminated destination strings > [1] and as such we should prefer more robust and less ambiguous string > interfaces. > > In every other location psinfo->pr_fname is used, it's with strscpy_pad. > It's clear that this field needs to be NUL-terminated and potentially > NUL-padded as well. > binfmt_elf.c +1545: > | char *__get_task_comm(char *buf, size_t buf_size, struct task_struct *tsk) > | { > | task_lock(tsk); > | /* Always NUL terminated and zero-padded */ > | strscpy_pad(buf, tsk->comm, buf_size); > | task_unlock(tsk); > | return buf; > | } > > Note that this patch relies on the _new_ 2-argument versions of > strscpy() and strscpy_pad() introduced in Commit e6584c3964f2f ("string: > Allow 2-argument strscpy()"). I am perplexed. Why not use get_task_comm fill_psinfo like binfmt_elf does? It seems very silly to copy half the function without locking and then not copy it's locking as well. Given that the more highly tested binfmt_elf uses get_task_comm I can't imagine a reason why binfmt_elf_fdpic can't use it as well. Eric > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Justin Stitt > --- > Note: build-tested only. > > Found with: $ rg "strncpy\(" > --- > fs/binfmt_elf_fdpic.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c > index 1920ed69279b..0365f14f18fc 100644 > --- a/fs/binfmt_elf_fdpic.c > +++ b/fs/binfmt_elf_fdpic.c > @@ -1359,7 +1359,7 @@ static int fill_psinfo(struct elf_prpsinfo *psinfo, struct task_struct *p, > SET_UID(psinfo->pr_uid, from_kuid_munged(cred->user_ns, cred->uid)); > SET_GID(psinfo->pr_gid, from_kgid_munged(cred->user_ns, cred->gid)); > rcu_read_unlock(); > - strncpy(psinfo->pr_fname, p->comm, sizeof(psinfo->pr_fname)); > + strscpy_pad(psinfo->pr_fname, p->comm); > > return 0; > } > > --- > base-commit: a4145ce1e7bc247fd6f2846e8699473448717b37 > change-id: 20240320-strncpy-fs-binfmt_elf_fdpic-c-828286d76310 > > Best regards, > -- > Justin Stitt