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 F3782CD128A for ; Tue, 9 Apr 2024 21:29:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 74B6A6B0087; Tue, 9 Apr 2024 17:29:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6FA996B0088; Tue, 9 Apr 2024 17:29:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5C2556B0089; Tue, 9 Apr 2024 17:29:03 -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 3ED146B0087 for ; Tue, 9 Apr 2024 17:29:03 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 0E3461602C1 for ; Tue, 9 Apr 2024 21:29:03 +0000 (UTC) X-FDA: 81991283766.22.6F39701 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) by imf12.hostedemail.com (Postfix) with ESMTP id 2FE674000B for ; Tue, 9 Apr 2024 21:29:00 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=eVl62Xxa; spf=pass (imf12.hostedemail.com: domain of keescook@chromium.org designates 209.85.214.177 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712698141; a=rsa-sha256; cv=none; b=orX5zMUvuS/yGubyNQz6h8jqCSdf2q49wWQZhTuR6FruyN8ZHRDN9BvNXO9IFtxux6tWmP AdqISVSBa/nLU04QcG84JaN7fPyqf/WFnAWmiLv34MbVaV+6xD7euG+pNCVUoQgVsiU2jA YCdeEW8QzDsSph5AsJ41BS+JbH3DGr0= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=eVl62Xxa; spf=pass (imf12.hostedemail.com: domain of keescook@chromium.org designates 209.85.214.177 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1712698141; 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=I1EckUdhv2S0vsLHXrSpPAoBKrsnqtVxE+FzPaDR/vQ=; b=VaFnPS5jYFeFaTjwdP43f27QY8lm8mkeZ8s05w5jvCl5Y5mixtHQvYwtawUvCSp41UPXQY kK11j/+S9G0EEX67a2K0ImI0Jab0ZYxNXbKplCaHZhHtO300Hifg5L5s1EXWKXEruhZrYb Ednre0vxWRDr0nClpCNGwbtCa9gWAes= Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-1e3f6f03594so18847335ad.0 for ; Tue, 09 Apr 2024 14:29:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1712698140; x=1713302940; 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=I1EckUdhv2S0vsLHXrSpPAoBKrsnqtVxE+FzPaDR/vQ=; b=eVl62XxaTjpYFEwoLfsnCTEt74Q07Ix5szQcdtCYeORD1/7IwPEALyU/Af+aXXCDS1 +/0x9Jq7xXP+rpfZ28UVnab/8scHQGgDRVueZ/RRnX0uMy67xePvS+gLYcmiaFdtaRld Y5weUiOCezqKWyuYWQnBtUmfan8XjGY0+aWhw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712698140; x=1713302940; 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=I1EckUdhv2S0vsLHXrSpPAoBKrsnqtVxE+FzPaDR/vQ=; b=Lm6peWxKZGbZjRyz3QCvI4WwDStLY6kvHlxmQrh18CmkwoPeDJzSQh+L2jc9gy2qFo T6R7uqq5j4RzMXKMBgdc2xrpjcp2h4riMsOCYFQRCRCEKmfIuBlhq1JhIJpfIpGsZmAj eErblaE/21xrY+7rPr8u6n6qIvdB59HyMFRFAgvfU9ozQS9ULPsLm0/dz6X0CcrzDf6L ouuUEPSxIJRxfGcWKzcpRZ2aNZ4HuB3c3CmCRQA7JkmkcPYAx4RGpHXja2Av4Nh9BaFP m3afS94+EF5M4dDyRfCa4rRtx7dka+cRs5Y2Sbedgm0CGwgpQvB3Xxg/htQgFPza021Z oD1w== X-Forwarded-Encrypted: i=1; AJvYcCVQhKwy7Cc4QdEavpYn/GBSrRHjKi9Bd5VnysWvIKhIK6/yyGHogEAf2ZYrCADK9aLkvudpCu9GyWLPdV1r++Zqduk= X-Gm-Message-State: AOJu0Yy4e6Uo9n2HzzZakb4nX4mi5e+cNcBAtM6eL8zUH3ts5qgP3/lF 7Cv8dsgKBB/VrmcZGk2F237YTANAGgcQEYlZ3CLmyCJAdtlWY76bfMcbxw+4cA== X-Google-Smtp-Source: AGHT+IGp1vHyVN9Of3ONnBmhGBURnZ2ozCtASZrjhYok6bINm0BBVqQzRg5525pvfv09CAPY6RG2ng== X-Received: by 2002:a17:902:a5cb:b0:1e2:adad:75f4 with SMTP id t11-20020a170902a5cb00b001e2adad75f4mr907671plq.28.1712698139910; Tue, 09 Apr 2024 14:28:59 -0700 (PDT) Received: from www.outflux.net ([198.0.35.241]) by smtp.gmail.com with ESMTPSA id p18-20020a170902e35200b001e2461c52c6sm9401985plc.149.2024.04.09.14.28.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Apr 2024 14:28:59 -0700 (PDT) Date: Tue, 9 Apr 2024 14:28:58 -0700 From: Kees Cook To: Marco Elver Cc: Steven Rostedt , Eric Biederman , Alexander Viro , Christian Brauner , Jan Kara , Masami Hiramatsu , Mathieu Desnoyers , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, Dmitry Vyukov Subject: Re: [PATCH] tracing: Add new_exec tracepoint Message-ID: <202404091428.B95127B72@keescook> References: <20240408090205.3714934-1-elver@google.com> <202404090840.E09789B66@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 2FE674000B X-Stat-Signature: 34krk3hfa8xm3966kwftr6d7ndozhsdx X-Rspam-User: X-HE-Tag: 1712698140-867951 X-HE-Meta: U2FsdGVkX1+Xz4mrhFsHf0HXTw6z31DKvI2aM1wpNyM0I/skNbayV32b7TfgtAZdzkiaAk5qeCljdKrmTxHoVAeYpndOKquH18AL72trDYfwF/yCefBfHX+tm9uAWETNTEau49isfqGuelYOgLwCLW+Ogae/N/8gguEy0Et/Xsohyc12WJbQYE1tnf7LVYxfS80M0vE0dMlQVgG1fyIQ9esFH1QeP9wcIv9r5LfTFB5Vx1Mc5JsLujJJHc1iXSB8nE0fvbXp9+Eu6oK7DXcXyELocOUa5ClHa0liIj85QAMKdrEx4gAtIJlw2DjUt+4OV0udppptFf7nqMAGbSbZ24/W7B99ry/a8wFaOFViglYlZX3DhPU6iwPBliVhx73y5BVPUrCGtqzcPiG40f9ykm+woD2heNqfFSsdc7gKDpE2WQEjcc6SuK5ZoNKErru8xvWjBrv6PV/P70PDmy1EoyOWsjDptqXxUtZfyS+wYKggUWwnAk07ZqcZA69yoGnlc9Mwfgpeei3nSjWcnk5up9l7OgDoinLOL1Y5hBNAp0Cyc6r5L+KUNNwdjsqguLsZFrsu0/Dru4TTMcnDQwjpUxyVLXGcQ88mKeLxUl7I99Ywo5dGyJLqQRPswNw0gCvfijE912v2lugiP3FCB45yk5WbsC5oB8X5UrYuNoIXlRlVEjIUxFwPGM67vx3xRKn5KdlWzx53cwQ1zSIxa+dzE0hjpVnksakhF1jhpzz0WOg/3P+8HmLVf5wtcfKW01+5HX1TvUHok6oFSbg53OoMbUNSkZekDh/qSCje+oibM8/r1+tiotVS+LeHHKUm5IZpcmD+SRBuTHXMwEgAg1001rhZPG3/S43Iz21qB0vR7GWKTxfNaiVs2EYfu6V6EgbbrzlUwl/BQsFYLDXUCKXyMpqs/UdDMfZr0kanWg358W6SFNCo9njPwSxp2lTdrrohtSu3jg/gU9TenigMYbH iv8Rb2YR nQ/ObpkSazK9mCxNZqwqGa4+YebQy9HgnuFLnNvCS+1vuvq0pDwr/eo3LfJciju1S8fq87sANQVmj3Pi/k4JTnM1A8+k6M89P5NePSWO5lrDuH9d8C36fc3oloFi/skvrGrD0deQGychGsp1AryaxRWRreNKJVmnxJr7ooxUDEYwYipki5FFf+CDiKPiiJef6luF77OOBX/CJouU6uVGk1T+eFr7WgnfJm+Hei7Nv/j5ytD0cgnPK57PUUitKB0Ap89RVXGFTKNE3iDio150XLLjQABZyY33EG4vp9I40xBhn08NcNTQ6QQj75qJqkdtxZ9COhx4cjeHEsrcHUh8cvfWP/ExlSg2W/ibcLA4kPP/4WhE1GGlGgBuw4DuzdMB6eMdxFYYwC/LsmNigjs4N4i+HDQ== X-Bogosity: Ham, tests=bogofilter, spamicity=0.041297, 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 Tue, Apr 09, 2024 at 08:25:45PM +0200, Marco Elver wrote: > On Tue, Apr 09, 2024 at 08:46AM -0700, Kees Cook wrote: > [...] > > > + trace_new_exec(current, bprm); > > > + > > > > All other steps in this function have explicit comments about > > what/why/etc. Please add some kind of comment describing why the > > tracepoint is where it is, etc. > > I beefed up the tracepoint documentation, and wrote a little paragraph > above where it's called to reinforce what we want. > > [...] > > What about binfmt_misc, and binfmt_script? You may want bprm->interp > > too? > > Good points. I'll make the below changes for v2: > > diff --git a/fs/exec.c b/fs/exec.c > index ab778ae1fc06..472b9f7b40e8 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1268,6 +1268,12 @@ int begin_new_exec(struct linux_binprm * bprm) > if (retval) > return retval; > > + /* > + * This tracepoint marks the point before flushing the old exec where > + * the current task is still unchanged, but errors are fatal (point of > + * no return). The later "sched_process_exec" tracepoint is called after > + * the current task has successfully switched to the new exec. > + */ > trace_new_exec(current, bprm); > > /* > diff --git a/include/trace/events/task.h b/include/trace/events/task.h > index 8853dc44783d..623d9af777c1 100644 > --- a/include/trace/events/task.h > +++ b/include/trace/events/task.h > @@ -61,8 +61,11 @@ TRACE_EVENT(task_rename, > * @task: pointer to the current task > * @bprm: pointer to linux_binprm used for new exec > * > - * Called before flushing the old exec, but at the point of no return during > - * switching to the new exec. > + * Called before flushing the old exec, where @task is still unchanged, but at > + * the point of no return during switching to the new exec. At the point it is > + * called the exec will either succeed, or on failure terminate the task. Also > + * see the "sched_process_exec" tracepoint, which is called right after @task > + * has successfully switched to the new exec. > */ > TRACE_EVENT(new_exec, > > @@ -71,19 +74,22 @@ TRACE_EVENT(new_exec, > TP_ARGS(task, bprm), > > TP_STRUCT__entry( > + __string( interp, bprm->interp ) > __string( filename, bprm->filename ) > __field( pid_t, pid ) > __string( comm, task->comm ) > ), > > TP_fast_assign( > + __assign_str(interp, bprm->interp); > __assign_str(filename, bprm->filename); > __entry->pid = task->pid; > __assign_str(comm, task->comm); > ), > > - TP_printk("filename=%s pid=%d comm=%s", > - __get_str(filename), __entry->pid, __get_str(comm)) > + TP_printk("interp=%s filename=%s pid=%d comm=%s", > + __get_str(interp), __get_str(filename), > + __entry->pid, __get_str(comm)) > ); > > #endif Looks good; I await v2, and Steven's Ack. :) -- Kees Cook