Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 11 Aug 2020 13:59:48 -0500
From: ebiederm@...ssion.com (Eric W. Biederman)
To: Mickaël Salaün <mic@...ikod.net>
Cc: linux-kernel@...r.kernel.org,  Aleksa Sarai <cyphar@...har.com>,  Alexei
 Starovoitov <ast@...nel.org>,  Al Viro <viro@...iv.linux.org.uk>,  Andrew
 Morton <akpm@...ux-foundation.org>,  Andy Lutomirski <luto@...nel.org>,
  Christian Brauner <christian.brauner@...ntu.com>,  Christian Heimes
 <christian@...hon.org>,  Daniel Borkmann <daniel@...earbox.net>,  Deven
 Bowers <deven.desai@...ux.microsoft.com>,  Dmitry Vyukov
 <dvyukov@...gle.com>,  Eric Biggers <ebiggers@...nel.org>,  Eric Chiang
 <ericchiang@...gle.com>,  Florian Weimer <fweimer@...hat.com>,  James
 Morris <jmorris@...ei.org>,  Jan Kara <jack@...e.cz>,  Jann Horn
 <jannh@...gle.com>,  Jonathan Corbet <corbet@....net>,  Kees Cook
 <keescook@...omium.org>,  Lakshmi Ramasubramanian
 <nramas@...ux.microsoft.com>,  Matthew Garrett <mjg59@...gle.com>,
  Matthew Wilcox <willy@...radead.org>,  Michael Kerrisk
 <mtk.manpages@...il.com>,  Mimi Zohar <zohar@...ux.ibm.com>,  Philippe
 Trébuchet <philippe.trebuchet@....gouv.fr>,  Scott Shell
 <scottsh@...rosoft.com>,  Sean Christopherson
 <sean.j.christopherson@...el.com>,  Shuah Khan <shuah@...nel.org>,  Steve
 Dower <steve.dower@...hon.org>,  Steve Grubb <sgrubb@...hat.com>,  Tetsuo
 Handa <penguin-kernel@...ove.SAKURA.ne.jp>,  Thibaut Sautereau
 <thibaut.sautereau@...p-os.org>,  Vincent Strubel
 <vincent.strubel@....gouv.fr>,  kernel-hardening@...ts.openwall.com,
  linux-api@...r.kernel.org,  linux-integrity@...r.kernel.org,
  linux-security-module@...r.kernel.org,  linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v7 1/7] exec: Change uselib(2) IS_SREG() failure to EACCES

Mickaël Salaün <mic@...ikod.net> writes:

> From: Kees Cook <keescook@...omium.org>
>
> Change uselib(2)' S_ISREG() error return to EACCES instead of EINVAL so
> the behavior matches execve(2), and the seemingly documented value.
> The "not a regular file" failure mode of execve(2) is explicitly
> documented[1], but it is not mentioned in uselib(2)[2] which does,
> however, say that open(2) and mmap(2) errors may apply. The documentation
> for open(2) does not include a "not a regular file" error[3], but mmap(2)
> does[4], and it is EACCES.

Do you have enough visibility into uselib to be certain this will change
will not cause regressions?

My sense of uselib is that it would be easier to remove the system call
entirely (I think it's last use was in libc5) than to validate that a
change like this won't cause problems for the users of uselib.

For the kernel what is important are real world users and the manpages
are only important as far as they suggest what the real world users do.

Eric


> [1] http://man7.org/linux/man-pages/man2/execve.2.html#ERRORS
> [2] http://man7.org/linux/man-pages/man2/uselib.2.html#ERRORS
> [3] http://man7.org/linux/man-pages/man2/open.2.html#ERRORS
> [4] http://man7.org/linux/man-pages/man2/mmap.2.html#ERRORS
>
> Signed-off-by: Mickaël Salaün <mic@...ikod.net>
> Signed-off-by: Kees Cook <keescook@...omium.org>
> Acked-by: Christian Brauner <christian.brauner@...ntu.com>
> Link: https://lore.kernel.org/r/20200605160013.3954297-2-keescook@chromium.org
> ---
>  fs/exec.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index e6e8a9a70327..d7c937044d10 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -141,11 +141,10 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
>  	if (IS_ERR(file))
>  		goto out;
>  
> -	error = -EINVAL;
> +	error = -EACCES;
>  	if (!S_ISREG(file_inode(file)->i_mode))
>  		goto exit;
>  
> -	error = -EACCES;
>  	if (path_noexec(&file->f_path))
>  		goto exit;

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.