From patchwork Sat Nov 30 04:49:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 13889109 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 8E04AD735F1 for ; Sat, 30 Nov 2024 04:49:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E43EC6B0082; Fri, 29 Nov 2024 23:49:23 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DF3EA6B0085; Fri, 29 Nov 2024 23:49:23 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CE1F56B0088; Fri, 29 Nov 2024 23:49:23 -0500 (EST) 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 B19766B0082 for ; Fri, 29 Nov 2024 23:49:23 -0500 (EST) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 3CF4C1602B9 for ; Sat, 30 Nov 2024 04:49:23 +0000 (UTC) X-FDA: 82841532312.21.DC25BA5 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf07.hostedemail.com (Postfix) with ESMTP id 628644000B for ; Sat, 30 Nov 2024 04:49:12 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="GI1wQ/YJ"; spf=pass (imf07.hostedemail.com: domain of kees@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=kees@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1732942155; 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-transfer-encoding:content-transfer-encoding: in-reply-to:references:dkim-signature; bh=J7CXwK9Ke19TZQ925knBI3YJcO7ZLposMmGznoCWVg4=; b=pjsif7OdeUOn6PBTLKetTBBjWVfTOjvvvfyxtXePMhfYJBTiYyGYrizHquT8xvtrLfvHTK Kx9A8lq2dhDwmJ7keHHMeCt6pbVtU/OgmSD5nygCbmf4OLD3bd1ahvkCbcDIgvJ7svRPWw diYpT64JrlOau/MqOdvPw9KSQ1p6v3s= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="GI1wQ/YJ"; spf=pass (imf07.hostedemail.com: domain of kees@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=kees@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1732942155; a=rsa-sha256; cv=none; b=itGpXcQxEbUTRNHWMxxsnI0pDQrnY8NZSMIM9lTBIwXJjlGY4lesmzHp5xVWOpsKlBv9eG d37EaJdWAhfivYZLkh+2Q8a+pLqqRihsJmwniV7+Fgx4QYGfXjmoZGo0yOj/94OsNgbl87 ewi4rzSmVkK8zWxWkg2mMhOQUxNMe6E= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id EB0D3A40324; Sat, 30 Nov 2024 04:47:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B319DC4CECC; Sat, 30 Nov 2024 04:49:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1732942159; bh=7a+G0bORdE2giUr3BfncHUbUhxIoNcU4LB5xeDPWjC0=; h=From:To:Cc:Subject:Date:From; b=GI1wQ/YJnVYzohfA+pkUBuCpZ7wmxmedH3w63YwPYzewyxYdEl0OgB6j0OpKX69kP gHCQDqAsGzSlc7cSAnQx4/T++rPZOCA6VXS8vFoTxin2qWiLmfxBWIB4M0ggiKzcwG EALQDbzN0Gk3lZ8JTYwgfGYA/+FefzNwcO4MTFw1dSOceBPmUVLOHrPmalQ62C9l6i QUe4V2BmgIriWzUlbuc2osu6jamacQ4TlSkrNsWVvBZHm8vWf3GWqF5SwZl9GJzF2u b+fAt858jkaIDXPzS7VoWh+9H/2aWNuInIscckjflwohp4CUaA3Sjm72pUc6hot48/ W3imNrwytJ5sA== From: Kees Cook To: Eric Biederman Cc: Kees Cook , Linus Torvalds , Alexander Viro , Christian Brauner , Jan Kara , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Valentin Schneider , Jens Axboe , Pavel Begunkov , Andrew Morton , Chen Yu , Shuah Khan , =?utf-8?q?Micka=C3=ABl_Sala=C3=BCn?= , linux-kernel@vger.kernel.org, io-uring@vger.kernel.org, linux-hardening@vger.kernel.org Subject: [PATCH] exec: Make sure task->comm is always NUL-terminated Date: Fri, 29 Nov 2024 20:49:14 -0800 Message-Id: <20241130044909.work.541-kees@kernel.org> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=4500; i=kees@kernel.org; h=from:subject:message-id; bh=7a+G0bORdE2giUr3BfncHUbUhxIoNcU4LB5xeDPWjC0=; b=owGbwMvMwCVmps19z/KJym7G02pJDOleM738HBLK3jN2iE/6eH1DywlDV//zrlOz/B20zoSrF TcWRqh2lLIwiHExyIopsgTZuce5eLxtD3efqwgzh5UJZAgDF6cATKSmnZFhhnD59S3JD5jX3D1Q mbmj6tqU1GW6Zs+WWgddMvkW5bPjByPDewHbf9LBK0Ic3Q08T3n7tfyLWvl1wUbxiKnnNR5G8O7 iAAA= X-Developer-Key: i=kees@kernel.org; a=openpgp; fpr=A5C3F68F229DD60F723E6E138972F4DFDC6DC026 X-Rspamd-Server: rspam05 X-Stat-Signature: aazngt9w4fj3f5ktcra8qdqfi8mixqae X-Rspamd-Queue-Id: 628644000B X-Rspam-User: X-HE-Tag: 1732942152-867090 X-HE-Meta: U2FsdGVkX1/VZyA8BIExTmA+Xkta3aeqIyiF+f+7CEuQ5A8MUpZ4SumWFwdHCFSOkLQ0U0rpdmNaRuAEcCZc3WLixEzLuHsb9wYYGmG8WGdIll8nwpGIa065tq/iES6b+2tmfUr7zlhrrL/VggR9Q89X4L2rpt2CeejmkQaxdTB+YrG9wV4Nbn4lVL/3Mh2d/S6qB1ICPgtaWYj2DzG7um+zDM7bR2qssQPxXPCLcTO7ppXmv2LAqcsUPX3DrLHf1gwjxactqiJ+s49N9eVKZMhcNB4N7C+t0IlCfO21iUg1KZcM6KAfZcFVSP0xA1XpP+kCgx7k1mxAMrH6v6fWEVl4RxXeT/NWF3ygnIRvocZsbL7s/JBCaWJXya5bL7gIy8sPCgyxQnJaaZU9dcknhnytNADN8N6mpLGG+H7l3PY5SCcbk9OpF/l1dPZF5EENhV7Lcm1K5qxyI7C6UE+SFO/CDWE3XKbuskjyIU1swLW+kmeZC8vDy32VSeTpnjaSR0d9wdwhxMFxUmlrbjPkUYqo+LYxqPhHA7OcJgBmca5XqGDbKD3bb3vbmwT/G3se7UJNgPn8ikaSS0spmX76trh09yXqzTVJW/ZT4ElnbwucKSDc7AkLbJ3K+BA4L10JdjK+VSeP0PCOhySW/jiVH6suwSd8Fr5Ry7IglfrSoW9TcNyZOvi2I4ecskWBVW4wAzbIoCZuwNghSBh6TwjuakwVpFO7PWy2suKPfMRSpQePdvLGh8BHCjg9iu90OJfFjKj8YnmcyNc9tf6tIUgFk5R8zWJTTrOOOBuuHWm8IaiAmAior3+ghC1zfQIV5LZsQtxE3PN/NSiltAuNL5KJHtNV3zEXtN8P1YmwxBLXN04yVf03wFlHwCFwXEsf9UJ/muMyR9z9Al7To+VcLGV7KxkxSRW4L1lbzpT7qFSn1ccJ6V4nvIXTALdvDDPdTECfuudNmp2ju6OGncNajvL pTy8rmnk FhA7vWDGE5+ftVZVate5s+JbyRXmOyYevZ6bDd7mwsKGlnOmaUYxUgaSY+cTiywA7Rxxpq4aeQLAg5wm9VXcEfcxAhZcu4Tk6hb34I/XyZfZW8mj/UEj9FdKPWVSY9BvX97XQwUJZbZYIPfgSwei06OzQu3Oh7hKNJrOT10+HwRvYobx7iRYsq3JPwh5P0EV45Y3A5WHp3cf59Uet3QT24V0AIk76/7PnsFVMelV0SDpn0Z9KFQ9/H8Vaa0srvZT6MRCY8QEi0HSqacgmueW0IxzREAMwCn02nPZ766zINPpKE5xdsqZgRSca90gTQYyudsXSZls2VHI8smM+QhrPtG4ycVTSaUzmtA9WZu60U3rb7hiu4StXxXHppiFgLLtblVie3jyLNK1xxTSXWp5OfBdJQgxEwQ5z0RkiEFhNu/0W4ib7yJ2rWukaNRjPoM5sdIxbd4xUdyzIctLzPNCiSQz03JH6ku8TEru44JzGu6FZ1emrvP/59UxFDURo8Ak/sVfThcke+2XBU/t6oNyMi1N6pJ8lkJ3ZZc+z 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: Using strscpy() meant that the final character in task->comm may be non-NUL for a moment before the "string too long" truncation happens. Instead of adding a new use of the ambiguous strncpy(), we'd want to use memtostr_pad() which enforces being able to check at compile time that sizes are sensible, but this requires being able to see string buffer lengths. Instead of trying to inline __set_task_comm() (which needs to call trace and perf functions), just open-code it. But to make sure we're always safe, add compile-time checking like we already do for get_task_comm(). Suggested-by: Linus Torvalds Suggested-by: "Eric W. Biederman" Signed-off-by: Kees Cook --- Cc: Eric Biederman Cc: Alexander Viro Cc: Christian Brauner Cc: Jan Kara Cc: linux-mm@kvack.org Cc: linux-fsdevel@vger.kernel.org Here's what I'd prefer to use to clean up set_task_comm(). I merged Linus and Eric's suggestions and open-coded memtostr_pad(). --- fs/exec.c | 12 ++++++------ include/linux/sched.h | 9 ++++----- io_uring/io-wq.c | 2 +- io_uring/sqpoll.c | 2 +- kernel/kthread.c | 3 ++- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index e0435b31a811..5f16500ac325 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1200,16 +1200,16 @@ char *__get_task_comm(char *buf, size_t buf_size, struct task_struct *tsk) EXPORT_SYMBOL_GPL(__get_task_comm); /* - * These functions flushes out all traces of the currently running executable - * so that a new one can be started + * This is unlocked -- the string will always be NUL-terminated, but + * may show overlapping contents if racing concurrent reads. */ - void __set_task_comm(struct task_struct *tsk, const char *buf, bool exec) { - task_lock(tsk); + size_t len = min(strlen(buf), sizeof(tsk->comm) - 1); + trace_task_rename(tsk, buf); - strscpy_pad(tsk->comm, buf, sizeof(tsk->comm)); - task_unlock(tsk); + memcpy(tsk->comm, buf, len); + memset(&tsk->comm[len], 0, sizeof(tsk->comm) - len); perf_event_comm(tsk, exec); } diff --git a/include/linux/sched.h b/include/linux/sched.h index e6ee4258169a..ac9f429ddc17 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1932,11 +1932,10 @@ static inline void kick_process(struct task_struct *tsk) { } #endif extern void __set_task_comm(struct task_struct *tsk, const char *from, bool exec); - -static inline void set_task_comm(struct task_struct *tsk, const char *from) -{ - __set_task_comm(tsk, from, false); -} +#define set_task_comm(tsk, from) ({ \ + BUILD_BUG_ON(sizeof(from) != TASK_COMM_LEN); \ + __set_task_comm(tsk, from, false); \ +}) extern char *__get_task_comm(char *to, size_t len, struct task_struct *tsk); #define get_task_comm(buf, tsk) ({ \ diff --git a/io_uring/io-wq.c b/io_uring/io-wq.c index a38f36b68060..5d0928f37471 100644 --- a/io_uring/io-wq.c +++ b/io_uring/io-wq.c @@ -634,7 +634,7 @@ static int io_wq_worker(void *data) struct io_wq_acct *acct = io_wq_get_acct(worker); struct io_wq *wq = worker->wq; bool exit_mask = false, last_timeout = false; - char buf[TASK_COMM_LEN]; + char buf[TASK_COMM_LEN] = {}; set_mask_bits(&worker->flags, 0, BIT(IO_WORKER_F_UP) | BIT(IO_WORKER_F_RUNNING)); diff --git a/io_uring/sqpoll.c b/io_uring/sqpoll.c index a26593979887..90011f06c7fb 100644 --- a/io_uring/sqpoll.c +++ b/io_uring/sqpoll.c @@ -271,7 +271,7 @@ static int io_sq_thread(void *data) struct io_ring_ctx *ctx; struct rusage start; unsigned long timeout = 0; - char buf[TASK_COMM_LEN]; + char buf[TASK_COMM_LEN] = {}; DEFINE_WAIT(wait); /* offload context creation failed, just exit */ diff --git a/kernel/kthread.c b/kernel/kthread.c index db4ceb0f503c..162d55811744 100644 --- a/kernel/kthread.c +++ b/kernel/kthread.c @@ -736,10 +736,11 @@ EXPORT_SYMBOL(kthread_stop_put); int kthreadd(void *unused) { + static const char comm[TASK_COMM_LEN] = "kthreadd"; struct task_struct *tsk = current; /* Setup a clean context for our children to inherit. */ - set_task_comm(tsk, "kthreadd"); + set_task_comm(tsk, comm); ignore_signals(tsk); set_cpus_allowed_ptr(tsk, housekeeping_cpumask(HK_TYPE_KTHREAD)); set_mems_allowed(node_states[N_MEMORY]);