All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Josh Triplett <josh@joshtriplett.org>
Cc: Eric Biederman <ebiederm@xmission.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fs/exec.c: Add fast path for ENOENT on PATH search before allocating mm
Date: Tue, 7 Nov 2023 12:30:37 -0800	[thread overview]
Message-ID: <202311071228.27D22C00@keescook> (raw)
In-Reply-To: <5c7333ea4bec2fad1b47a8fa2db7c31e4ffc4f14.1663334978.git.josh@joshtriplett.org>

On Fri, Sep 16, 2022 at 02:41:30PM +0100, Josh Triplett wrote:
> Currently, execve allocates an mm and parses argv and envp before
> checking if the path exists. However, the common case of a $PATH search
> may have several failed calls to exec before a single success. Do a
> filename lookup for the purposes of returning ENOENT before doing more
> expensive operations.
> 
> This does not create a TOCTTOU race, because this can only happen if the
> file didn't exist at some point during the exec call, and that point is
> permitted to be when we did our lookup.
> 
> To measure performance, I ran 2000 fork and execvpe calls with a
> seven-element PATH in which the file was found in the seventh directory
> (representative of the common case as /usr/bin is the seventh directory
> on my $PATH), as well as 2000 fork and execve calls with an absolute
> path to an existing binary. I recorded the minimum time for each, to
> eliminate noise from context switches and similar.
> 
> Without fast-path:
> fork/execvpe: 49876ns
> fork/execve:  32773ns
> 
> With fast-path:
> fork/execvpe: 36890ns
> fork/execve:  32069ns
> 
> The cost of the additional lookup seems to be in the noise for a
> successful exec, but it provides a 26% improvement for the path search
> case by speeding up the six failed execs.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>

*thread necromancy*

I'll snag this patch after -rc1 is out. Based on the research we both
did in the rest of this thread, this original patch is a clear win.
Let's get it into linux-next and see if anything else falls out of it.

I did, however, scratch my head over the 0-day report:
https://lore.kernel.org/lkml/202209221401.90061e56-yujie.liu@intel.com/

But I can't see why this patch could trigger those problems...

Thanks!

-Kees

-- 
Kees Cook

  parent reply	other threads:[~2023-11-07 20:30 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-16 13:41 [PATCH] fs/exec.c: Add fast path for ENOENT on PATH search before allocating mm Josh Triplett
2022-09-16 14:38 ` Kees Cook
2022-09-16 20:13   ` Josh Triplett
2022-09-17  0:11     ` Kees Cook
2022-09-17  0:50       ` Josh Triplett
2022-09-19 20:02         ` Kees Cook
2022-10-01 16:01           ` Josh Triplett
2022-09-19 14:34       ` Peter Zijlstra
2022-09-22  7:27 ` [fs/exec.c] 0a276ae2d2: BUG:workqueue_lockup-pool kernel test robot
2022-09-22  7:27   ` kernel test robot
2023-11-07 20:30 ` Kees Cook [this message]
2023-11-07 20:51   ` [PATCH] fs/exec.c: Add fast path for ENOENT on PATH search before allocating mm Mateusz Guzik
2023-11-07 21:23     ` Mateusz Guzik
2023-11-07 22:50       ` Kees Cook
2023-11-07 23:08         ` Mateusz Guzik
2023-11-07 23:39           ` Kees Cook
2023-11-08  0:03             ` Mateusz Guzik
2023-11-08 19:25               ` Kees Cook
2023-11-08 19:31               ` Kees Cook
2023-11-08 19:35                 ` Mateusz Guzik
2023-11-09  0:17                   ` Eric W. Biederman
2023-11-09 12:21                     ` Mateusz Guzik
2023-11-10  5:26                       ` Eric W. Biederman
2023-11-07 20:37 ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202311071228.27D22C00@keescook \
    --to=keescook@chromium.org \
    --cc=ebiederm@xmission.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.