LKML Archive on lore.kernel.org
 help / color / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: "Mickaël Salaün" <mic@digikod.net>
Cc: linux-kernel@vger.kernel.org, "Aleksa Sarai" <cyphar@cyphar.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Christian Brauner" <christian.brauner@ubuntu.com>,
	"Christian Heimes" <christian@python.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Deven Bowers" <deven.desai@linux.microsoft.com>,
	"Dmitry Vyukov" <dvyukov@google.com>,
	"Eric Biggers" <ebiggers@kernel.org>,
	"Eric Chiang" <ericchiang@google.com>,
	"Florian Weimer" <fweimer@redhat.com>,
	"James Morris" <jmorris@namei.org>, "Jan Kara" <jack@suse.cz>,
	"Jann Horn" <jannh@google.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Kees Cook" <keescook@chromium.org>,
	"Lakshmi Ramasubramanian" <nramas@linux.microsoft.com>,
	"Matthew Garrett" <mjg59@google.com>,
	"Matthew Wilcox" <willy@infradead.org>,
	"Michael Kerrisk" <mtk.manpages@gmail.com>,
	"Mimi Zohar" <zohar@linux.ibm.com>,
	"Philippe Trébuchet" <philippe.trebuchet@ssi.gouv.fr>,
	"Scott Shell" <scottsh@microsoft.com>,
	"Sean Christopherson" <sean.j.christopherson@intel.com>,
	"Shuah Khan" <shuah@kernel.org>,
	"Steve Dower" <steve.dower@python.org>,
	"Steve Grubb" <sgrubb@redhat.com>,
	"Tetsuo Handa" <penguin-kernel@I-love.SAKURA.ne.jp>,
	"Thibaut Sautereau" <thibaut.sautereau@clip-os.org>,
	"Vincent Strubel" <vincent.strubel@ssi.gouv.fr>,
	kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v7 1/7] exec: Change uselib(2) IS_SREG() failure to EACCES
Date: Tue, 11 Aug 2020 13:59:48 -0500
Message-ID: <87eeodnh3v.fsf@x220.int.ebiederm.org> (raw)
In-Reply-To: <20200723171227.446711-2-mic@digikod.net> (=?utf-8?Q?=22Micka?= =?utf-8?Q?=C3=ABl_Sala=C3=BCn=22's?= message of "Thu, 23 Jul 2020 19:12:21 +0200")

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

> From: Kees Cook <keescook@chromium.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@digikod.net>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Acked-by: Christian Brauner <christian.brauner@ubuntu.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;

  reply index

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-23 17:12 [PATCH v7 0/7] Add support for O_MAYEXEC Mickaël Salaün
2020-07-23 17:12 ` [PATCH v7 1/7] exec: Change uselib(2) IS_SREG() failure to EACCES Mickaël Salaün
2020-08-11 18:59   ` Eric W. Biederman [this message]
2020-08-11 19:14     ` Eric W. Biederman
2020-07-23 17:12 ` [PATCH v7 2/7] exec: Move S_ISREG() check earlier Mickaël Salaün
2020-08-11 19:27   ` Eric W. Biederman
2020-07-23 17:12 ` [PATCH v7 3/7] exec: Move path_noexec() " Mickaël Salaün
2020-08-11 19:36   ` Eric W. Biederman
2020-08-13 15:31     ` Mickaël Salaün
2020-07-23 17:12 ` [PATCH v7 4/7] fs: Introduce O_MAYEXEC flag for openat2(2) Mickaël Salaün
2020-07-24 19:03   ` Kees Cook
2020-07-27  4:21   ` Al Viro
2020-07-27  5:27     ` Florian Weimer
2020-07-27 19:46       ` Mickaël Salaün
2020-08-11 19:51   ` Eric W. Biederman
2020-08-13 14:36     ` Mickaël Salaün
2020-07-23 17:12 ` [PATCH v7 5/7] fs,doc: Enable to enforce noexec mounts or file exec through O_MAYEXEC Mickaël Salaün
2020-07-24 19:03   ` Kees Cook
2020-08-11 19:58   ` Eric W. Biederman
2020-08-13 14:49     ` Mickaël Salaün
2020-07-23 17:12 ` [PATCH v7 6/7] selftest/openat2: Add tests for O_MAYEXEC enforcing Mickaël Salaün
2020-07-24 19:03   ` Kees Cook
2020-07-23 17:12 ` [PATCH v7 7/7] ima: add policy support for the new file open MAY_OPENEXEC flag Mickaël Salaün
2020-07-24 19:04   ` Kees Cook
2020-07-24 11:20 ` [PATCH v7 0/7] Add support for O_MAYEXEC Thibaut Sautereau
2020-07-24 19:06 ` Kees Cook
2020-07-25 11:15   ` Christian Brauner
2020-08-10 20:11   ` Mickaël Salaün
2020-08-10 20:21     ` Al Viro
2020-08-10 22:09       ` David Laight
2020-08-10 22:28         ` Al Viro
2020-08-10 22:47           ` Mickaël Salaün
2020-08-11  8:09             ` David Laight
2020-08-11  8:50               ` Mickaël Salaün
2020-08-10 22:43       ` Mickaël Salaün
2020-08-10 23:03         ` Jann Horn
2020-08-11  8:48           ` Mickaël Salaün
2020-08-11 13:56             ` Mimi Zohar
2020-08-11 14:02               ` Matthew Wilcox
2020-08-11 14:30                 ` Mimi Zohar
2020-08-11 17:18             ` Deven Bowers
2020-08-10 23:05         ` Al Viro
2020-08-11  8:49           ` Mickaël Salaün

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=87eeodnh3v.fsf@x220.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=ast@kernel.org \
    --cc=christian.brauner@ubuntu.com \
    --cc=christian@python.org \
    --cc=corbet@lwn.net \
    --cc=cyphar@cyphar.com \
    --cc=daniel@iogearbox.net \
    --cc=deven.desai@linux.microsoft.com \
    --cc=dvyukov@google.com \
    --cc=ebiggers@kernel.org \
    --cc=ericchiang@google.com \
    --cc=fweimer@redhat.com \
    --cc=jack@suse.cz \
    --cc=jannh@google.com \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mic@digikod.net \
    --cc=mjg59@google.com \
    --cc=mtk.manpages@gmail.com \
    --cc=nramas@linux.microsoft.com \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=philippe.trebuchet@ssi.gouv.fr \
    --cc=scottsh@microsoft.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=sgrubb@redhat.com \
    --cc=shuah@kernel.org \
    --cc=steve.dower@python.org \
    --cc=thibaut.sautereau@clip-os.org \
    --cc=vincent.strubel@ssi.gouv.fr \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    --cc=zohar@linux.ibm.com \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git