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 72BBAC3ABBE for ; Thu, 8 May 2025 08:22:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7DE336B000A; Thu, 8 May 2025 04:22:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 78D7F6B0082; Thu, 8 May 2025 04:22:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 67C486B0085; Thu, 8 May 2025 04:22:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 4AC7C6B000A for ; Thu, 8 May 2025 04:22:17 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 43566C1013 for ; Thu, 8 May 2025 08:22:18 +0000 (UTC) X-FDA: 83419048356.04.4279E87 Received: from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56]) by imf28.hostedemail.com (Postfix) with ESMTP id 691C9C000B for ; Thu, 8 May 2025 08:22:16 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=rTr5kv4J; dmarc=pass (policy=none) header.from=igalia.com; spf=pass (imf28.hostedemail.com: domain of bhsharma@igalia.com designates 213.97.179.56 as permitted sender) smtp.mailfrom=bhsharma@igalia.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1746692536; a=rsa-sha256; cv=none; b=N16maY+mwLwZXx2HTcJr7C8PufqxbusDGkaMRHwDMWcv5knldPtUtAvOjtx5e1RPirAyxT Xpn0/WaxICMYoP3u/7sbvtNnT7lanw5wgumEoMcEvkX518lNbyyiy5HQxJTNp+W39OYTaG zu8kJdu9ap73u+phJryl6K5qe0dFC5g= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=rTr5kv4J; dmarc=pass (policy=none) header.from=igalia.com; spf=pass (imf28.hostedemail.com: domain of bhsharma@igalia.com designates 213.97.179.56 as permitted sender) smtp.mailfrom=bhsharma@igalia.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1746692536; 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=CyA4fdewJCdynAxXbxpGy2pyASTRxTov0Q5FFYV+koE=; b=vrqgXU4phUfPeAPq9av+n1mCeyV2CQ9mD5PfXIzsZn8fFR93NIBuhlQgUNpI9blEwhwTY+ 7H48AQvZILyqs+gd4ZjZKAAfBshY4vQOJXFJ9HZ1goRmuL0j24fIEoeGeBMAKC+FHnKXlg iGHV6/fUaOgIJnUDudhuLFelZW9lkPM= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:References: Cc:To:From:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=CyA4fdewJCdynAxXbxpGy2pyASTRxTov0Q5FFYV+koE=; b=rTr5kv4JM8lEQaje0SpZ81EMpa nRCOclH0h/mS3Slv643F5s7Zh1JSj1cHELrJHIxEBWvURQEG7RzjkcbEh181emB5FXRBKRG4ya3NK 6j0yG8NI4d4wVCP996ls4W3PBCWOLw2QNJXb8lXRY8/zYxCBQ/L9XjWZHJAGiwj7GBND0BYpSwthn da7zvwhLD0EKJ0X255mQ+INCtR1FSzu/WJNnv59pDCHVNMfn9SlJsDoUXYWnqHdz6NwW1Rcf8Mz77 vaICt2hUCVUPyuthiCK4DTM8CclIrFCkN8OospG9kIyIVlj9I46f72ZwLzymXToqc+BBBfBvYox8h 21bbeKPw==; Received: from [223.233.71.203] (helo=[192.168.1.12]) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128) (Exim) id 1uCwS4-0056VN-LH; Thu, 08 May 2025 10:22:08 +0200 Message-ID: <3fd1dd03-ce1c-37e5-98aa-a91ab5d210b3@igalia.com> Date: Thu, 8 May 2025 13:52:01 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: Re: [PATCH v3 2/3] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation Content-Language: en-US From: Bhupesh Sharma To: Petr Mladek Cc: akpm@linux-foundation.org, kernel-dev@igalia.com, linux-kernel@vger.kernel.org, bpf@vger.kernel.org, linux-perf-users@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, oliver.sang@intel.com, lkp@intel.com, laoar.shao@gmail.com, rostedt@goodmis.org, mathieu.desnoyers@efficios.com, arnaldo.melo@gmail.com, alexei.starovoitov@gmail.com, andrii.nakryiko@gmail.com, mirq-linux@rere.qmqm.pl, peterz@infradead.org, willy@infradead.org, david@redhat.com, viro@zeniv.linux.org.uk, keescook@chromium.org, ebiederm@xmission.com, brauner@kernel.org, jack@suse.cz, mingo@redhat.com, juri.lelli@redhat.com, bsegall@google.com, mgorman@suse.de, vschneid@redhat.com References: <20250507110444.963779-1-bhupesh@igalia.com> <20250507110444.963779-3-bhupesh@igalia.com> <4af48ad5-1aa7-46d0-bfca-7779294e355c@igalia.com> In-Reply-To: <4af48ad5-1aa7-46d0-bfca-7779294e355c@igalia.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 691C9C000B X-Rspam-User: X-Rspamd-Server: rspam07 X-Stat-Signature: baaih4dkyamgeiyu7fwabs9e7zu86tdd X-HE-Tag: 1746692536-329125 X-HE-Meta: U2FsdGVkX18svZL+Ax5aLgimy3fzWnwJLns8378JVs6qvABZOn3HVPU0stydidDfUdL72ZBrCaQBfOb/xE88/I20lfoAtgGJjJP1f1bbO36oBqZEHtNYjicRczCf00Xb5aNPdhIZ4UacdgbQ17SeYcbB2O1W7//L/kLuAkJrGFscXYm4IJf7wQdK43G0aZcrAyehs0qNjzSz46PsDi3WJRfDQMp/QKuOw+IlT90KO7cCJdtDwEOiamrN6B92xhNyjSIYA8Ob0wZvRJa/h8ZmCtGOW55gAJ9MhsqsJrp3JX2WDxWiiBWcgaPOSis7w2WCwydw+Fl+gpc1Bnn9xp5AQaJuFBSqEwrZspievqI5OtwWlDYew/UR+dDo38Dn/M5ke+hbJG7cD10GNGSGE38Tkj3dxJhkSPeMHYaD72kuOv1eSfBxUwjgmXWrJsDnkPK1x8Ao+oCmqqIVcknZ93xsSHsZ5S11fI/nS/TtwQbKg13Vm7ZDHr4TgBThBsKuhyEzwrUzlHfgZONvOZFjwWeuazIrUEN0XS7KQOlwsjDK0+sxnthGbqqjxv+tjbYpJdw4nJiFlod1R0CRpp5BuYLmwjuU3cfNn4HsPowmb/JnVjDhAX62qCT//mOUOx/O5JLhMTA5sXMjN/RpugqVBiBUvKza3nFeiGteogqsaXRTVcKnfdYqPtVyLhGgtPRJplj3QGl/wR2sgGxbTykto95EY+nazysGBQWkObQOb+OIIZqcw1u5FPWjuBu5rqi1QfSKdy2luJPVhcEsoNRLpAuEmZ5eGJjEg+pAt1NWXLPts4eKMSPmL2NBl9CUhIcq1EfjeAK89RkMpORWLj4e0mmuOjc08jCVMSngmvlsMdAjSqPAKZoGGW+Jy/+/V30X1rUhMJFiAmWPVSbmxoUymiPRYFlIHqZlCXVIJKM1r4wnvOb+7v1G2PH0woRER44rflXQ8UucrAfbwc34luqlC6m I8esCMa0 cm7XFwA2Pm9moWb8jPtoGhDgD63YBeYHvETH0hK/XLFNopEQSSezlHbjF+kVm7uqd+vg5SlNTqq7+K2j5AnUfzsDP8EsE5k0diOMBLh3kuhfti9NjRtg8JGiAdfouVMqG29uLSB+uaaGRa1lEPsisnkN1BRyFryM75Et9tixSNz6r6XvDdVb5hiry69KdAoykWJMzrmnjBY+7pKU/8iNBQ/yJ/CoLt5ZaoTu1/q4qyCeCwCs7bcnh6QwfzbcMNGWnSAxL6HB5ZUNgflxIev1i3WPf5ETNYIULFIEScp9YztcaPsHREzbRY9W3GTVLVL0ZjBSdhrcUpO3nRtUTcakKwVSmKKGTZBKpom4TYYaBivaqsjrDs6ohMeHxTQ== 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 5/8/25 1:47 PM, Bhupesh Sharma wrote: > Hi Petr, > > On 5/7/25 5:59 PM, Petr Mladek wrote: >> On Wed 2025-05-07 16:34:43, Bhupesh wrote: >>> As Linus mentioned in [1], currently we have several memcpy() use-cases >>> which use 'current->comm' to copy the task name over to local copies. >>> For an example: >>> >>>   ... >>>   char comm[TASK_COMM_LEN]; >>>   memcpy(comm, current->comm, TASK_COMM_LEN); >>>   ... >>> >>> These should be modified so that we can later implement approaches >>> to handle the task->comm's 16-byte length limitation (TASK_COMM_LEN) >>> is a more modular way (follow-up patches do the same): >>> >>>   ... >>>   char comm[TASK_COMM_LEN]; >>>   memcpy(comm, current->comm, TASK_COMM_LEN); >>>   comm[TASK_COMM_LEN - 1] = 0; >>>   ... >>> >>> The relevant 'memcpy()' users were identified using the following >>> search >>> pattern: >>>   $ git grep 'memcpy.*->comm\>' >>> >>> [1]. >>> https://lore.kernel.org/all/CAHk-=wjAmmHUg6vho1KjzQi2=psR30+CogFd4aXrThr2gsiS4g@mail.gmail.com/ >>> >>> --- a/include/linux/coredump.h >>> +++ b/include/linux/coredump.h >>> @@ -53,7 +53,8 @@ extern void do_coredump(const kernel_siginfo_t >>> *siginfo); >>>       do {    \ >>>           char comm[TASK_COMM_LEN];    \ >>>           /* This will always be NUL terminated. */ \ >>> -        memcpy(comm, current->comm, sizeof(comm)); \ >>> +        memcpy(comm, current->comm, TASK_COMM_LEN); \ >>> +        comm[TASK_COMM_LEN] = '\0'; \ >> I would expect that we replace this with a helper function/macro >> which would do the right thing. >> >> Why is get_task_comm() not used here, please? >> >>>           printk_ratelimited(Level "coredump: %d(%*pE): " Format >>> "\n",    \ >> Also the name seems to be used for printing a debug information. >> I would expect that we could use the bigger buffer here and print >> the "full" name. Is this planed, please? >> >>>               task_tgid_vnr(current), (int)strlen(comm), comm, >>> ##__VA_ARGS__);    \ >>>       } while (0)    \ >>> diff --git a/include/trace/events/block.h >>> b/include/trace/events/block.h >>> index bd0ea07338eb..94a941ac2034 100644 >>> --- a/include/trace/events/block.h >>> +++ b/include/trace/events/block.h >>> @@ -214,6 +214,7 @@ DECLARE_EVENT_CLASS(block_rq, >>>           blk_fill_rwbs(__entry->rwbs, rq->cmd_flags); >>>           __get_str(cmd)[0] = '\0'; >>>           memcpy(__entry->comm, current->comm, TASK_COMM_LEN); >>> +        __entry->comm[TASK_COMM_LEN - 1] = '\0'; >> Same for all other callers. >> >> That said, I am not sure if the larger buffer is save in all situations. >> >>>       ), >> > > Thanks for the review, I agree on using the helper / wrapper function > to replace this open-coded memcpy + set last entry as '\0'. > > However I see that Steven has already shared a RFC approach (see [1]), > to use __string() instead of fixed lengths for 'task->comm' for > tracing events. > I plan to  rebase my v4 on top of his RFC, which might mean that this > patch would no longer be needed in the v4. > > [1]. > https://lore.kernel.org/linux-trace-kernel/20250507133458.51bafd95@gandalf.local.home/ > Sorry, pressed the send button too quickly :D I instead meant - "I plan to  rebase my v4 on top of Steven's RFC, which might mean that this patch would no longer need to address the trace events, but would still need to handle other places where tsk->comm directly in memcpy() and replace it with 'get_task_comm()' instead". Thanks.