forked from mirrors/linux
		
	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:
		
							parent
							
								
									e5075d8ec5
								
							
						
					
					
						commit
						978ffcbf00
					
				
					 1 changed files with 35 additions and 34 deletions
				
			
		
							
								
								
									
										69
									
								
								fs/exec.c
									
									
									
									
									
								
							
							
						
						
									
										69
									
								
								fs/exec.c
									
									
									
									
									
								
							| 
						 | 
				
			
			@ -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 @@ static int bprm_execve(struct linux_binprm *bprm,
 | 
			
		|||
	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:
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue