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 24813C52D7B for ; Tue, 13 Aug 2024 22:19:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7A7F76B0082; Tue, 13 Aug 2024 18:19:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 759116B0083; Tue, 13 Aug 2024 18:19:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 620406B0085; Tue, 13 Aug 2024 18:19:32 -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 3E89B6B0082 for ; Tue, 13 Aug 2024 18:19:32 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 9C0ABA0AF7 for ; Tue, 13 Aug 2024 22:19:31 +0000 (UTC) X-FDA: 82448639742.24.71DFC17 Received: from mail-io1-f48.google.com (mail-io1-f48.google.com [209.85.166.48]) by imf15.hostedemail.com (Postfix) with ESMTP id CDEBBA000B for ; Tue, 13 Aug 2024 22:19:29 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=FigWvuAQ; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf15.hostedemail.com: domain of justinstitt@google.com designates 209.85.166.48 as permitted sender) smtp.mailfrom=justinstitt@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723587490; 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=+QN8X+bBf4e4pg8ECVJmLV7aoGPPE4M9h9y5SfDnco4=; b=qnUgotwATo4RZQY0ZKhmF8e4KYeogcZ11EcHvE3c0DeBNk62wswHpncoEi0Z4FKQdTHK5T 49L94YdefbjYIU0+Y1GeDI7IzqvL2h3O/pia/N54wG9XEgQJ7lcZSiz13ob5DdlRVJL1ta Yb+NxdkqKhli8vvF7Aq+LjnMhHfMMiQ= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723587490; a=rsa-sha256; cv=none; b=hOplww3C2Hz7uz/vo8fW/zSX3YNX6g5f5E/8C9dTkHnEYE4g4308lIYxK2AI2qXDgZUq+a huK4yEbK4raa9szilxlWTUlsgOYZaE5A4scZ6jtanTsDEVdehx+GCqgwRsCIdBRgnLXw8S 6ap39pOJIjsQGWWVMtbyWuLS4Kl+tzY= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=FigWvuAQ; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf15.hostedemail.com: domain of justinstitt@google.com designates 209.85.166.48 as permitted sender) smtp.mailfrom=justinstitt@google.com Received: by mail-io1-f48.google.com with SMTP id ca18e2360f4ac-81f9339e534so212665939f.3 for ; Tue, 13 Aug 2024 15:19:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1723587569; x=1724192369; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=+QN8X+bBf4e4pg8ECVJmLV7aoGPPE4M9h9y5SfDnco4=; b=FigWvuAQTSBg/uQPTyQr/cZFR286LGuNlgJD8Jw8Zf/56sO3DDpgOm3hxChJt3pkSy Dm9mW9BPwdVoo6bzDjMFjY7vOD7JlcB2JvpFPbTsrSrP9oy9wxRuwZ15FMeCVYlo/A5x CN0DIg2RkN2orYXzeKmPCghhBcfE8rByagCiJg87GTL5M0lJQ8u5BoRNvvhlEHNQdkms X+dCkTAVrAu/McdOjv0TRK882MGpiwdbnPFwou1gt6kx56BuIKCZO1R6ln+0S4QlC3gc 3WKfFBx5vDQu9UD20sGY/aht/Szl7jhIThAkCkXD2CQHdRmDVSp6YajI4VIHmb5lwp8p qncg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723587569; x=1724192369; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=+QN8X+bBf4e4pg8ECVJmLV7aoGPPE4M9h9y5SfDnco4=; b=pV3wb7F13nrVDCiFVuP46BV6Q9s7pv6z+s3adQYCvMXB7w4ZyelfKVy6EjxrWlQgci 8lPBqfPA4ovGZzHoR8J+pzgFAjZdK2SFxNcfwrjvvtgznUqKtdEF6wehmnA/WYHmvhcs NMba/gFBqnE3r2MFQgnh/hzDBpdcH3s9vxl9yYQ3dMMCa9abDvwcItHJ8Xbfnp2iDOYM UzLW/R8CLQ6NlsJSIrBYnCcwG7s9LQA/xcSlLdba+qmckOMOlsEVFdNUl5F6kyl6eCwL +KQDrvPYA9WHZX2u+2JadOvBRNap1GvmxQRTSRyCfz95cjTgoNwSHXP2/69dpiQEq7qB x0fw== X-Forwarded-Encrypted: i=1; AJvYcCUVVJmfHsiCUHcaW8+Pzoj9jNSGae8bRgDlS7XKmOSLk8eyfyFxlusSVZkD7LuAIeW7S8nrU4mTFA==@kvack.org X-Gm-Message-State: AOJu0YxcnQtiuygxGNatI7cbZXiiwKYPdSpeDSvSXZFLRPv1nSjVccIj G3Y1i2E8Hv9rVMxQx32kWr7bPstNzMezONyhTYvq4L9lpi+0i2M41TQG7h69Sg== X-Google-Smtp-Source: AGHT+IH+Vc3rvDiFEg5ElrB7bMI7N4hpSdsoFZYx8k23AbxObZpcHIjRmC/uICTdWR81lxtCEKl/zQ== X-Received: by 2002:a05:6602:27c1:b0:824:d58c:ec9 with SMTP id ca18e2360f4ac-824dad3f48bmr165323439f.10.1723587568538; Tue, 13 Aug 2024 15:19:28 -0700 (PDT) Received: from google.com (194.225.68.34.bc.googleusercontent.com. [34.68.225.194]) by smtp.gmail.com with ESMTPSA id 8926c6da1cb9f-4ca769f9d7dsm2773101173.118.2024.08.13.15.19.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Aug 2024 15:19:28 -0700 (PDT) Date: Tue, 13 Aug 2024 15:19:25 -0700 From: Justin Stitt To: Yafang Shao Cc: akpm@linux-foundation.org, torvalds@linux-foundation.org, 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, Masami Hiramatsu , Mathieu Desnoyers Subject: Re: [PATCH v6 7/9] tracing: Replace strncpy() with strscpy() Message-ID: References: <20240812022933.69850-1-laoar.shao@gmail.com> <20240812022933.69850-8-laoar.shao@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240812022933.69850-8-laoar.shao@gmail.com> X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: CDEBBA000B X-Stat-Signature: kww5foyyup4d35pgt1rkqbgiw4myxxyz X-Rspam-User: X-HE-Tag: 1723587569-572313 X-HE-Meta: U2FsdGVkX19ZhgsU8aGUIByH6EYotbbSRbxUlEIHMlMSJPPMAjNvJK9M1Vz5/3MXqHarWH/WuySh0Si8esX7NWQI1nvp4urBZHtHmppSLpDEN5aX6h51IotDnNzjc27/V5QcbXac/EWxOfhiGkcjladEHH2T6weuM+UWWPFk71EGSFJ3ymsw74WTyj9ovn7X/Sr3zd6iQokvBBhM2RdhlgmLFimAiU3l2wpYD8Q85o6WRNV5cWMuHVPqVPDq27GZtvVbCcGsjjvMIuPZa2jP2X23Zy+VYjnRvERGyDoe6CBnpYq3/tV2LQqAnsnTYhSpcgwrw0rSLQJPnRhnrbwAIKV/58sefg5FrqbfpHZ8r2q9qoZp0KPjcyzW6b1faGW+MvEe0rWyJqkslaQndO2GRwTRRxqKKetKLVBDNxbnw3y16qk+dpL3T3MMIsUih3YWSUeDEWSMerRCQDz7VV251A2kjBfp9dIPQ68RnT1ozdkiok32dIJvg/8iD12Pw5dzGGgnKXtQGHjKYax7sEatIGlnIm7it9BHKr8sBZVN0BniPHpkuC1Gj8LG/dpkzNaLG6eOAvI0hVMcTRx86kY+U8xCmddJk0961L92bxzJuxY3eQJEUZEIYomVg/FwYdVXZJwmUUFwoD+xOagEgwtfzD5qbnlhcSTdE/a0HjgJOLv4lb+H1aHqTrZ2Heb1/Sd1iMEQKMnGyNLUhmGtljIqDWDLF0lhdxbOHSXpYs2sxOYKX7g5lyc7ul33snaX28aIT5vZqdCOi3lmhOohgWzQszmKCnMVXDeocDHrlr+Gfd5AMsFLW/JYx6bqd3qGfJZYnCLks2OEowo107DRkjmv34ytwcE3iVOwnlcfpdbu9DmpP6ggJcyO5dnleNVkySDm0MDxiTIP9ySSV/93HfKl2nDZlpBjyerfqC1epkMP9zZUmFfE3wUc+9SQoOM8UqGQrx/BjWmLX4KUuZPSEC/ LXSlIHnI LBuvwumxZRcpwOHZYJWBuI3xtw2Txmr61ET4Hv2gA1Lw6mw374m3AEhHo4l/Ao0fdmPuEs/ExAE+Wi6Fp2MTHHmnmbdufIYY6IwltaqNf9fEXm5iHDP2qs4NVWyn26qj6E7wFLhcwIfJKgF5M8IKqiiGgUhCq8cLgqoWcIQ+OdUWnt/XJgC4zVc4hqshlmnKPbnbYObTD/lFnNYj2FYz/9Ieux69e3JC8lQCpHuS49WzZTa58QO5wp+vj6wjDA5S0HgOIT52JyU4HMafHJFmGdTcULkXTPG4Jcd04GDQ/JsgXwUadZjmB7+MUAHvMPXvW/4I3gyWY8v1MPppVfGz2gTzQVqgggHr73hLR9BWK6iAXVvNhbzLzkC9kKRPE8aGpnZspakIC2QrJ6RRRPn1JLTxlm243sf42YQuWINl6ahRaBnmA1kGKKQnpQdkGFABbGfUarP1K6GM9dTdSXVi1ZQlL9ydDi3FWv7vz/bNvZrzQFNdxex77OXe2qx4EY8u7oWeSVtNpOd21ju5vlr5ZFndx4S7Fx6Uy/0TgRA4+Z95qAyNbT/F1P7GB+hOOo45yPQ9L5m/4J2GjzxM= 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: Hi, On Mon, Aug 12, 2024 at 10:29:31AM GMT, Yafang Shao wrote: > Using strscpy() to read the task comm ensures that the name is > always NUL-terminated, regardless of the source string. This approach also > facilitates future extensions to the task comm. Thanks for sending patches replacing str{n}cpy's! I believe there's at least two more instances of strncpy in trace.c as well as in trace_events_hist.c (for a grand total of 6 instances in the files you've touched in this specific patch). It'd be great if you could replace those instances in this patch as well :>) This would help greatly with [1]. > > Signed-off-by: Yafang Shao > Acked-by: Masami Hiramatsu (Google) > Cc: Steven Rostedt > Cc: Mathieu Desnoyers > --- > kernel/trace/trace.c | 2 +- > kernel/trace/trace_events_hist.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index 578a49ff5c32..1b2577f9d734 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -1907,7 +1907,7 @@ __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu) > max_data->critical_start = data->critical_start; > max_data->critical_end = data->critical_end; > > - strncpy(max_data->comm, tsk->comm, TASK_COMM_LEN); > + strscpy(max_data->comm, tsk->comm, TASK_COMM_LEN); If max_data->comm wants to be NUL-terminated then this is the right replacement. Without knowing how the trace stack works at all, it's hard for me to tell if that is the case. There's a length-supplied format specifier for which this comm field is used with; Either this is just another safeguard against spilling over the buffer or this field really doesn't care about NUL-termination. | seq_printf(m, "# | task: %.16s-%d " | "(uid:%d nice:%ld policy:%ld rt_prio:%ld)\n", | data->comm, data->pid, In the event this field doesn't need to be NUL-terminated then we are introducing an off-by-one error where we are copying one less useful byte with strscpy -- Linus pointed out earlier [2] that these things all just want to be c-strings so this is probably the right change :>) > max_data->pid = tsk->pid; > /* > * If tsk == current, then use current_uid(), as that does not use > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c > index 6ece1308d36a..4cd24c25ce05 100644 > --- a/kernel/trace/trace_events_hist.c > +++ b/kernel/trace/trace_events_hist.c > @@ -1599,7 +1599,7 @@ static inline void save_comm(char *comm, struct task_struct *task) > return; > } > > - strncpy(comm, task->comm, TASK_COMM_LEN); > + strscpy(comm, task->comm, TASK_COMM_LEN); > } > > static void hist_elt_data_free(struct hist_elt_data *elt_data) > -- > 2.43.5 > Link: https://github.com/KSPP/linux/issues/90 [1] Link: https://lore.kernel.org/all/CAHk-=whWtUC-AjmGJveAETKOMeMFSTwKwu99v7+b6AyHMmaDFA@mail.gmail.com/ [2] Thanks Justin