execve: open the executable file before doing anything else

No point in allocating a new mm, counting arguments and environment
variables etc if we're just going to return ENOENT.

This patch does expose the fact that 'do_filp_open()' that execve() uses
is still unnecessarily expensive in the failure case, because it
allocates the 'struct file *' early, even if the path lookup (which is
heavily optimized) fails.

So that remains an unnecessary cost in the "no such executable" case,
but it's a separate issue.  Regardless, I do not want to do _both_ a
filename_lookup() and a later do_filp_open() like the origin patch by
Josh Triplett did in [1].

Reported-by: Josh Triplett <josh@joshtriplett.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Link: https://lore.kernel.org/lkml/5c7333ea4bec2fad1b47a8fa2db7c31e4ffc4f14.1663334978.git.josh@joshtriplett.org/ [1]
Link: https://lore.kernel.org/lkml/202209161637.9EDAF6B18@keescook/
Link: https://lore.kernel.org/lkml/CAHk-=wgznerM-xs+x+krDfE7eVBiy_HOam35rbsFMMOwvYuEKQ@mail.gmail.com/
Link: https://lore.kernel.org/lkml/CAHk-=whf9qLO8ipps4QhmS0BkM8mtWJhvnuDSdtw5gFjhzvKNA@mail.gmail.com/
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
Linus Torvalds 2024-01-08 16:43:04 -08:00
parent e5075d8ec5
commit 978ffcbf00

View File

@ -1508,12 +1508,24 @@ static void free_bprm(struct linux_binprm *bprm)
kfree(bprm);
}
static struct linux_binprm *alloc_bprm(int fd, struct filename *filename)
static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int flags)
{
struct linux_binprm *bprm = kzalloc(sizeof(*bprm), GFP_KERNEL);
struct linux_binprm *bprm;
struct file *file;
int retval = -ENOMEM;
if (!bprm)
goto out;
file = do_open_execat(fd, filename, flags);
if (IS_ERR(file))
return ERR_CAST(file);
bprm = kzalloc(sizeof(*bprm), GFP_KERNEL);
if (!bprm) {
allow_write_access(file);
fput(file);
return ERR_PTR(-ENOMEM);
}
bprm->file = file;
if (fd == AT_FDCWD || filename->name[0] == '/') {
bprm->filename = filename->name;
@ -1526,18 +1538,28 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename)
if (!bprm->fdpath)
goto out_free;
/*
* Record that a name derived from an O_CLOEXEC fd will be
* inaccessible after exec. This allows the code in exec to
* choose to fail when the executable is not mmaped into the
* interpreter and an open file descriptor is not passed to
* the interpreter. This makes for a better user experience
* than having the interpreter start and then immediately fail
* when it finds the executable is inaccessible.
*/
if (get_close_on_exec(fd))
bprm->interp_flags |= BINPRM_FLAGS_PATH_INACCESSIBLE;
bprm->filename = bprm->fdpath;
}
bprm->interp = bprm->filename;
retval = bprm_mm_init(bprm);
if (retval)
goto out_free;
return bprm;
if (!retval)
return bprm;
out_free:
free_bprm(bprm);
out:
return ERR_PTR(retval);
}
@ -1807,10 +1829,8 @@ static int exec_binprm(struct linux_binprm *bprm)
/*
* sys_execve() executes a new program.
*/
static int bprm_execve(struct linux_binprm *bprm,
int fd, struct filename *filename, int flags)
static int bprm_execve(struct linux_binprm *bprm)
{
struct file *file;
int retval;
retval = prepare_bprm_creds(bprm);
@ -1826,26 +1846,8 @@ static int bprm_execve(struct linux_binprm *bprm,
current->in_execve = 1;
sched_mm_cid_before_execve(current);
file = do_open_execat(fd, filename, flags);
retval = PTR_ERR(file);
if (IS_ERR(file))
goto out_unmark;
sched_exec();
bprm->file = file;
/*
* Record that a name derived from an O_CLOEXEC fd will be
* inaccessible after exec. This allows the code in exec to
* choose to fail when the executable is not mmaped into the
* interpreter and an open file descriptor is not passed to
* the interpreter. This makes for a better user experience
* than having the interpreter start and then immediately fail
* when it finds the executable is inaccessible.
*/
if (bprm->fdpath && get_close_on_exec(fd))
bprm->interp_flags |= BINPRM_FLAGS_PATH_INACCESSIBLE;
/* Set the unchanging part of bprm->cred */
retval = security_bprm_creds_for_exec(bprm);
if (retval)
@ -1875,7 +1877,6 @@ out:
if (bprm->point_of_no_return && !fatal_signal_pending(current))
force_fatal_sig(SIGSEGV);
out_unmark:
sched_mm_cid_after_execve(current);
current->fs->in_exec = 0;
current->in_execve = 0;
@ -1910,7 +1911,7 @@ static int do_execveat_common(int fd, struct filename *filename,
* further execve() calls fail. */
current->flags &= ~PF_NPROC_EXCEEDED;
bprm = alloc_bprm(fd, filename);
bprm = alloc_bprm(fd, filename, flags);
if (IS_ERR(bprm)) {
retval = PTR_ERR(bprm);
goto out_ret;
@ -1959,7 +1960,7 @@ static int do_execveat_common(int fd, struct filename *filename,
bprm->argc = 1;
}
retval = bprm_execve(bprm, fd, filename, flags);
retval = bprm_execve(bprm);
out_free:
free_bprm(bprm);
@ -1984,7 +1985,7 @@ int kernel_execve(const char *kernel_filename,
if (IS_ERR(filename))
return PTR_ERR(filename);
bprm = alloc_bprm(fd, filename);
bprm = alloc_bprm(fd, filename, 0);
if (IS_ERR(bprm)) {
retval = PTR_ERR(bprm);
goto out_ret;
@ -2019,7 +2020,7 @@ int kernel_execve(const char *kernel_filename,
if (retval < 0)
goto out_free;
retval = bprm_execve(bprm, fd, filename, 0);
retval = bprm_execve(bprm);
out_free:
free_bprm(bprm);
out_ret: