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 86250C54FAA for ; Wed, 28 Aug 2024 12:57:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 188A26B0082; Wed, 28 Aug 2024 08:57:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 139526B0083; Wed, 28 Aug 2024 08:57:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 026FB6B0089; Wed, 28 Aug 2024 08:57:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id D39796B0082 for ; Wed, 28 Aug 2024 08:57:55 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 523B8AA304 for ; Wed, 28 Aug 2024 12:57:55 +0000 (UTC) X-FDA: 82501656510.19.FAA1BBA Received: from mail-qk1-f174.google.com (mail-qk1-f174.google.com [209.85.222.174]) by imf13.hostedemail.com (Postfix) with ESMTP id 6C13F2001A for ; Wed, 28 Aug 2024 12:57:52 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=iTZdgcxK; spf=pass (imf13.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.222.174 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724849803; 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=75zSlLwUuo3vTsXNiB/Bg248lll9xEE0NLqRnWuoevM=; b=N5JUVtNzWWn+3xgCLvenjjftwBUUcRS5BJwhU+RebNzM84Jj6sC5BBfsF/ZrCRmA3KpxSk r/2LNuFg/JnXvGSFNM2m6bRwFzxTfWZciEEzjr2xlEpIXd/ErwzxtkCCuqENYaF2PtZGOG 9YC3O/dBRO6r2G4RRi5Y/atwPZIJ+vQ= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=iTZdgcxK; spf=pass (imf13.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.222.174 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724849803; a=rsa-sha256; cv=none; b=psGCMJzVVMTC7vU+QGTMpbzFxyc+O++ISILJIZvM8SORWMw7Aig6Onb/vcD9WsNp1/1fao rrCy19WveAFaZI6pj0ycDtWcklEuiCK//WrAovZLuFbssVIbmQDqdrMEQPOOHkev39Bd8P rC6q0GzbY68L3dLhXsRMNUw/A/PGESQ= Received: by mail-qk1-f174.google.com with SMTP id af79cd13be357-7a7d7ec7395so167504985a.3 for ; Wed, 28 Aug 2024 05:57:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724849871; x=1725454671; 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=75zSlLwUuo3vTsXNiB/Bg248lll9xEE0NLqRnWuoevM=; b=iTZdgcxKGm/Sl/Yn0STYY6FlIxWFxRHnRxqamydpXj/kwN37VJOFSTdyEpU5d7hhus aBzp+1crjPTMnZzMvThGF6cnhOvYwznhAlzTFuNzW/fN9a9x6K8KxZn10VDmrLT+JOfF CVdIQu5wBRixX5ijp2JRx8AAOlWos/Rt0AI0u4gWgx4SlPDUjHBGzL8PGhLHzN3kjH5c YII2QZ3JkKEijIQS3KZpqMFxPECkA8aKCY/b5y+qWvk7P1B44raIlAERZrf/Ug4dPIXU lIwDvaNo3JGfAumh6f4rVWk3WLqS4QINH2rFHMhlmbGDTHiEKQBjviCgQf/5RmvEmze+ jU3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724849871; x=1725454671; 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=75zSlLwUuo3vTsXNiB/Bg248lll9xEE0NLqRnWuoevM=; b=NVERTcg4tZIZAzMW5mSE6F80kg2Sr3hcGmA1VwNY5JiZ4bPV9MfFQ1BcywgCZloMzM 9qL8vBKxBedqyRGCrgCt2HnI/FhTksqLFweyUIsJ2I7P1HUH4EdTkzDIqP0fM1zmC6rg cnF2LUliSZyu1cTZs9LjyidBrBJbxXGOIk6XaSb8lkcHSmjJrU2KZ69fXkLbIY89liPM 2EqH25fa9Mic+9adKxgkPpV35Mig3vPNsnv+6anug7xc6YR9X7L3o6Dl/kxnJjnI03ao qc6VJH9gH2WxP2evoatAd4hJFroGJfBRNqP9W/o8U7rZC7ZRfAkM4Tgzxm1787NHwMkE 4YCQ== X-Forwarded-Encrypted: i=1; AJvYcCU2zEvL5pq+jM9QMbnUAlu2oNc3WWbZRUE2DNslehQXIQWnXrywhucgNmZfai3fT3lVP8fcnE1yoQ==@kvack.org X-Gm-Message-State: AOJu0YwKYVecBLOGmYTCsbfwBYS0/V641OWQCVnNB2ACmFiXGwz6jZTw xzrLWsAAyVASClDeXB9ABawHIWOcFk8wC8DQAlQhuu3njWZ78aUB60X8L27XGtpXJ6kKRTNEUDK /uIyH/+qLBdRvrSalxa4KnqzelHQ= X-Google-Smtp-Source: AGHT+IGVdDZRdAbiWZ5RYcs75Y+9kyklByrS1cllsDWIp6LEk4Cl8MdoT9mGOnGBew4RhB5tPSnWZccJF6guOMsDPnY= X-Received: by 2002:a05:6214:320c:b0:6c1:5283:e67b with SMTP id 6a1803df08f44-6c16deb306dmr178438206d6.47.1724849871190; Wed, 28 Aug 2024 05:57:51 -0700 (PDT) MIME-Version: 1.0 References: <20240828030321.20688-1-laoar.shao@gmail.com> <20240828030321.20688-2-laoar.shao@gmail.com> In-Reply-To: From: Yafang Shao Date: Wed, 28 Aug 2024 20:57:13 +0800 Message-ID: Subject: Re: [PATCH v8 1/8] Get rid of __get_task_comm() To: Alejandro Colomar 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" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: g81f6bj4eb9hfy5m3hicwbk7qerzm6q3 X-Rspam-User: X-Rspamd-Queue-Id: 6C13F2001A X-Rspamd-Server: rspam02 X-HE-Tag: 1724849872-969816 X-HE-Meta: U2FsdGVkX1/9L++/NntVRSmNZWjCZ0GqNNxoN3jfwpgAZHYaU9BcfCynuZFW5MevP2U4sc5ZmBh7MB4h3nC0XHQC/meyqpPBo0gO+xTW5rDXB1g2IfIyCE7OMpCkRpOA6D3wKwwOZoj5NWUoP8mZlxRViCXfa/5DLe8vf8/+90rovr215wcbdg3FNw1G8uYf55LOm5JQXiA52eT6m+QEZll0vJsM9DscFGjbBq1Vj+p0aEwSxAj7yuSll/A10c8mEEVdVj1dAWlk8siZS6pkY9atgq7fxRh05L008D0q38SmjULck1L1gGFn/7ItjZTRPUxGST69XbH5XGWHVOg4rO+ww3M+nceMINrdrPiuxDnon1HjOtKTleTPL0zxWnMqQ7gaC016qaENDXrSfE3NaJMzdS2IBiSHDF+vQgVZXm6xgtSqpLfk4x5+HdDbY7vu39bJCUunT8VtMl2Ppw/FSu64RtIeM3+IXOH1SSzZ/07McKC7cDIGk1sfeQ1VJxQ865U/t20vFhyY0pMaZ6Smc773m+vrHg7eYojqnCCHUIBTT7BSoPZUhVWtPCOHhBb+o5Jt2/q4L5cH4edgiOWmUzVHZuyWKM28p8rWCeCFUxMk01JKK53ibNn7NHrBTl4/OdEeMSv4cyHD8wnl+MjbeGmbmMZyRXz4unwyZNtJOYPPRivrcAKxftgjXSCaUqzevbmJo0R7cl9uO7XYCvqe2oeopgKs/ulskr2ds8jzgBdasK4y8+6n4cfsel8AXuun7Sb5J5mC3lDbzaS2BTtzaCthLNyNDnaG+nyCUHCbJwmHmkExDNLqN4PW7uQrVwsXLDY6mDMuxDR9TDIdlmOYiVXiqOPhywi3/RV8W1BTrMYBKiR9cFgLyJDNFW/08cnYedNPPd+rRvlfaBy9tNR2Vt5+xRgaD5Hcu9dO7Xl3Bc70Qo0Ta6T9DYFrIJjT2N0h4OFTZ70yL2FzFr3c0AM XEyeyoGe HROokoijW4Na03tcB1ACYUWNG4L3aCIqxcp63dFqtkw/cQn9XVa5VVxtng2o8pg1oog9YcnF+a3TuCImLxitZHWLg2Xo5qNcd3kuZX5m37Nby5fsadC+nXgyYtc+T8PcGSjS0ikpR9UmKSJW8RgN/Ir9RLbcrmemLD8snNmcfDz0S/lEhTU5ASBnQXBOUr3yy9EjYBIPig8sTzdCEDSCSU0aQMSSBl8tQ9GDNQpDXk6Fx8wMkzUS2y2V6lS0bmy6UCVy5OxBHIsV0vPXz2cuPOzc3ZCM5vwPC7LsTjhZH9ss2Kni5kM0JCH4Pd41RfR/8QXO/sO+oKo3VqCofDPrjVXvIJtVhQ/JXxbRU+kHv2a9eThFX8fAWisn3ynHvf9aaB2uP8ALe1CaiCFAtgeM6DwNSW4NyyklqQzl9zZkWAsHXhhI0uduvgfrlHtTGeATB9sNvLuj+t8wWqqlUL4n14KmoTprwlEqY202crKzcmtvNNyf7HWNy842weHQ97OvTmhn4WeOeSUVAlRjh89Bydt5dBb8I3D1SzXcIzvMnHYMfY3tA1pQB1RrckPScyJ/Lk8DE75FZF7L5g+p3cThgQyKug5eXZQP+BWk94bxoWEexLY35Oet31L5cLJQyS5stb5ORWPpoJglo1F9pbVXcaBHmQeUW61zY0aFwMr5+n9CJubbvDjLC3oo5CHQXxRy12O4buy2pAvE2dQZfBYea9X5/1Pn28Wqbi+AfbmBJeDdO3bmptnR1OHBSO4YE2mQjG+9dZNthy5Yfrdpxrp7aSvz+WNV41KZxIuLCQsgGYPHAvgfz2myd/HZCmQ== 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 Wed, Aug 28, 2024 at 6:15=E2=80=AFPM Alejandro Colomar = wrote: > > 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: > > > > - The task_lock() is unnecessary > > Quoted from Linus [0]: > > : Since user space can randomly change their names anyway, using lock= ing > > : was always wrong for readers (for writers it probably does make sen= se > > : to have some lock - although practically speaking nobody cares ther= e > > : either, but at least for a writer some kind of race could have > > : long-term mixed results > > > > - The BUILD_BUG_ON() doesn't add any value > > The only requirement is to ensure that the destination buffer is a va= lid > > array. > > > > - 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 si= te. > > > > Suggested-by: Linus Torvalds > > Link: https://lore.kernel.org/all/CAHk-=3DwivfrF0_zvf+oj6=3D=3DSh=3D-np= JooP8chLPEfaFV0oNYTTBA@mail.gmail.com [0] > > Link: https://lore.kernel.org/all/CAHk-=3DwhWtUC-AjmGJveAETKOMeMFSTwKwu= 99v7+b6AyHMmaDFA@mail.gmail.com/ > > Suggested-by: Alejandro Colomar > > Link: https://lore.kernel.org/all/2jxak5v6dfxlpbxhpm3ey7oup4g2lnr3ueurf= bosf5wdo65dk4@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(-) > > > > [...] > > > 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_st= ruct *tsk, const char *from) > > __set_task_comm(tsk, from, false); > > } > > > > -extern char *__get_task_comm(char *to, size_t len, struct task_struct = *tsk); > > +/* > > [...] > > > + * - 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(ds= t)) This macro is defined in arch/um/include/shared/user.h, which is not used outside the arch/um/ directory. This marco should be addressed. > > which is used in patch 2/8 The strscpy() function used in this series is defined in include/linux/string.h, which already checks whether the input is an array: #define __strscpy0(dst, src, ...) \ sized_strscpy(dst, src, sizeof(dst) + __must_be_array(dst)) #define __strscpy1(dst, src, size) sized_strscpy(dst, src, size) #define __strscpy_pad0(dst, src, ...) \ sized_strscpy_pad(dst, src, sizeof(dst) + __must_be_array(dst)) #define __strscpy_pad1(dst, src, size) sized_strscpy_pad(dst, src, size) > > 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. I have no preference between using ARRAY_SIZE() or sizeof(dst) + __must_be_array(dst). However, for consistency, it might be better to use ARRAY_SIZE(). -- Regards Yafang