Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: "Mickaël Salaün" <mic@digikod.net>
Cc: linux-kernel@vger.kernel.org, "Aleksa Sarai" <cyphar@cyphar.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Andy Lutomirski" <luto@amacapital.net>,
	"Arnaldo Carvalho de Melo" <acme@kernel.org>,
	"Casey Schaufler" <casey@schaufler-ca.com>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"David Drysdale" <drysdale@google.com>,
	"David S . Miller" <davem@davemloft.net>,
	"Eric W . Biederman" <ebiederm@xmission.com>,
	"James Morris" <jmorris@namei.org>, "Jann Horn" <jann@thejh.net>,
	"John Johansen" <john.johansen@canonical.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Kees Cook" <keescook@chromium.org>,
	"Michael Kerrisk" <mtk.manpages@gmail.com>,
	"Mickaël Salaün" <mickael.salaun@ssi.gouv.fr>,
	"Paul Moore" <paul@paul-moore.com>,
	"Sargun Dhillon" <sargun@sargun.me>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	"Shuah Khan" <shuah@kernel.org>,
	"Stephen Smalley" <sds@tycho.nsa.gov>,
	"Tejun Heo" <tj@kernel.org>,
	"Tetsuo Handa" <penguin-kernel@i-love.sakura.ne.jp>,
	"Thomas Graf" <tgraf@suug.ch>, "Tycho Andersen" <tycho@tycho.ws>,
	"Will Drewry" <wad@chromium.org>,
	kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	linux-security-module@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH bpf-next v9 05/10] bpf,landlock: Add a new map type: inode
Date: Tue, 25 Jun 2019 23:52:01 +0100
Message-ID: <20190625225201.GJ17978@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20190625215239.11136-6-mic@digikod.net>

On Tue, Jun 25, 2019 at 11:52:34PM +0200, Mickaël Salaün wrote:
> +/* must call iput(inode) after this call */
> +static struct inode *inode_from_fd(int ufd, bool check_access)
> +{
> +	struct inode *ret;
> +	struct fd f;
> +	int deny;
> +
> +	f = fdget(ufd);
> +	if (unlikely(!f.file || !file_inode(f.file))) {
> +		ret = ERR_PTR(-EBADF);
> +		goto put_fd;
> +	}

Just when does one get a NULL file_inode()?  The reason I'm asking is
that arseloads of code would break if one managed to create such
a beast...

Incidentally, that should be return ERR_PTR(-EBADF); fdput() is wrong there.

> +	}
> +	/* check if the FD is tied to a mount point */
> +	/* TODO: add this check when called from an eBPF program too */
> +	if (unlikely(!f.file->f_path.mnt

Again, the same question - when the hell can that happen?  If you are
sitting on an exploitable roothole, do share it...

 || f.file->f_path.mnt->mnt_flags &
> +				MNT_INTERNAL)) {
> +		ret = ERR_PTR(-EINVAL);
> +		goto put_fd;

What does it have to do with mountpoints, anyway?

> +/* called from syscall */
> +static int sys_inode_map_delete_elem(struct bpf_map *map, struct inode *key)
> +{
> +	struct inode_array *array = container_of(map, struct inode_array, map);
> +	struct inode *inode;
> +	int i;
> +
> +	WARN_ON_ONCE(!rcu_read_lock_held());
> +	for (i = 0; i < array->map.max_entries; i++) {
> +		if (array->elems[i].inode == key) {
> +			inode = xchg(&array->elems[i].inode, NULL);
> +			array->nb_entries--;

Umm...  Is that intended to be atomic in any sense?

> +			iput(inode);
> +			return 0;
> +		}
> +	}
> +	return -ENOENT;
> +}
> +
> +/* called from syscall */
> +int bpf_inode_map_delete_elem(struct bpf_map *map, int *key)
> +{
> +	struct inode *inode;
> +	int err;
> +
> +	inode = inode_from_fd(*key, false);
> +	if (IS_ERR(inode))
> +		return PTR_ERR(inode);
> +	err = sys_inode_map_delete_elem(map, inode);
> +	iput(inode);
> +	return err;
> +}

Wait a sec...  So we have those beasties that can have long-term
references to arbitrary inodes stuck in them?  What will happen
if you get umount(2) called while such a thing exists?

  reply index

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-25 21:52 [PATCH bpf-next v9 00/10] Landlock LSM: Toward unprivileged sandboxing Mickaël Salaün
2019-06-25 21:52 ` [PATCH bpf-next v9 01/10] fs,security: Add a new file access type: MAY_CHROOT Mickaël Salaün
2019-06-25 21:52 ` [PATCH bpf-next v9 02/10] bpf: Add eBPF program subtype and is_valid_subtype() verifier Mickaël Salaün
2019-06-25 23:02   ` Alexei Starovoitov
2019-06-26  7:33     ` Mickaël Salaün
2019-06-25 21:52 ` [PATCH bpf-next v9 03/10] bpf,landlock: Define an eBPF program type for Landlock hooks Mickaël Salaün
2019-06-25 21:52 ` [PATCH bpf-next v9 04/10] seccomp,landlock: Enforce Landlock programs per process hierarchy Mickaël Salaün
2019-06-25 21:52 ` [PATCH bpf-next v9 05/10] bpf,landlock: Add a new map type: inode Mickaël Salaün
2019-06-25 22:52   ` Al Viro [this message]
2019-06-27 16:18     ` Mickaël Salaün
2019-06-27 16:56       ` Al Viro
2019-06-28 13:17         ` Mickaël Salaün
2019-06-25 21:52 ` [PATCH bpf-next v9 06/10] landlock: Handle filesystem access control Mickaël Salaün
2019-06-25 21:52 ` [PATCH bpf-next v9 07/10] landlock: Add ptrace restrictions Mickaël Salaün
2019-06-25 21:52 ` [PATCH bpf-next v9 08/10] bpf: Add a Landlock sandbox example Mickaël Salaün
2019-06-25 21:52 ` [PATCH bpf-next v9 09/10] bpf,landlock: Add tests for Landlock Mickaël Salaün
2019-06-25 21:52 ` [PATCH bpf-next v9 10/10] landlock: Add user and kernel documentation " Mickaël Salaün

Reply instructions:

You may reply publically 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=20190625225201.GJ17978@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=acme@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=ast@kernel.org \
    --cc=casey@schaufler-ca.com \
    --cc=corbet@lwn.net \
    --cc=cyphar@cyphar.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=drysdale@google.com \
    --cc=ebiederm@xmission.com \
    --cc=jann@thejh.net \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mic@digikod.net \
    --cc=mickael.salaun@ssi.gouv.fr \
    --cc=mtk.manpages@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=sargun@sargun.me \
    --cc=sds@tycho.nsa.gov \
    --cc=serge@hallyn.com \
    --cc=shuah@kernel.org \
    --cc=tgraf@suug.ch \
    --cc=tj@kernel.org \
    --cc=tycho@tycho.ws \
    --cc=wad@chromium.org \
    /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

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

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

Example config snippet for mirrors

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


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