BPF List
 help / color / mirror / Atom feed
From: Yafang Shao <laoar.shao@gmail.com>
To: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	kafai@fb.com, songliubraving@fb.com, yhs@fb.com,
	john.fastabend@gmail.com, kpsingh@kernel.org
Cc: netdev@vger.kernel.org, bpf@vger.kernel.org,
	Yafang Shao <laoar.shao@gmail.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: [PATCH] bpf: Refuse to mount bpffs on the same mount point multiple times
Date: Wed, 23 Feb 2022 13:18:33 +0000	[thread overview]
Message-ID: <20220223131833.51991-1-laoar.shao@gmail.com> (raw)

We monitored an unexpected behavoir that bpffs is mounted on a same mount
point lots of times on some of our production envrionments. For example,
$ mount -t bpf
bpffs /sys/fs/bpf bpf rw,relatime 0 0
bpffs /sys/fs/bpf bpf rw,relatime 0 0
bpffs /sys/fs/bpf bpf rw,relatime 0 0
bpffs /sys/fs/bpf bpf rw,relatime 0 0
...

That was casued by a buggy user script which didn't check the mount
point correctly before mounting bpffs. But it also drives us to think more
about if it is okay to allow mounting bpffs on the same mount point
multiple times. After investigation we get the conclusion that it is bad
to allow that behavior, because it can cause unexpected issues, for
example it can break bpftool, which depends on the mount point to get
the pinned files.

Below is the test case wrt bpftool.
First, let's mount bpffs on /var/run/ltcp/bpf multiple times.
$ mount -t bpf
bpffs on /run/ltcp/bpf type bpf (rw,relatime)
bpffs on /run/ltcp/bpf type bpf (rw,relatime)
bpffs on /run/ltcp/bpf type bpf (rw,relatime)

After pinning some bpf progs on this mount point, let's check the pinned
files with bpftool,
$ bpftool prog list -f
87: sock_ops  name bpf_sockmap  tag a04f5eef06a7f555  gpl
        loaded_at 2022-02-23T16:27:38+0800  uid 0
        xlated 16B  jited 15B  memlock 4096B
        pinned /run/ltcp/bpf/bpf_sockmap
        pinned /run/ltcp/bpf/bpf_sockmap
        pinned /run/ltcp/bpf/bpf_sockmap
        btf_id 243
89: sk_msg  name bpf_redir_proxy  tag 57cd311f2e27366b  gpl
        loaded_at 2022-02-23T16:27:38+0800  uid 0
        xlated 16B  jited 18B  memlock 4096B
        pinned /run/ltcp/bpf/bpf_redir_proxy
        pinned /run/ltcp/bpf/bpf_redir_proxy
        pinned /run/ltcp/bpf/bpf_redir_proxy
        btf_id 244

The same pinned file will be showed multiple times.
Finnally after mounting bpffs on the same mount point again, we can't
get the pinnned files via bpftool,
$ bpftool prog list -f
87: sock_ops  name bpf_sockmap  tag a04f5eef06a7f555  gpl
        loaded_at 2022-02-23T16:27:38+0800  uid 0
        xlated 16B  jited 15B  memlock 4096B
        btf_id 243
89: sk_msg  name bpf_redir_proxy  tag 57cd311f2e27366b  gpl
        loaded_at 2022-02-23T16:27:38+0800  uid 0
        xlated 16B  jited 18B  memlock 4096B
        btf_id 244

We should better refuse to mount bpffs on the same mount point. Before
making this change, I also checked why it is allowed in the first place.
The related commits are commit e27f4a942a0e
("bpf: Use mount_nodev not mount_ns to mount the bpf filesystem") and
commit b2197755b263 ("bpf: add support for persistent maps/progs").
Unfortunately they didn't explain why it is allowed. But there should be
no use case which requires to mount bpffs on a same mount point multiple
times, so let's just refuse it.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
 kernel/bpf/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index 4f841e16779e..58374db9376f 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -763,7 +763,7 @@ static int bpf_fill_super(struct super_block *sb, struct fs_context *fc)
 
 static int bpf_get_tree(struct fs_context *fc)
 {
-	return get_tree_nodev(fc, bpf_fill_super);
+	return get_tree_single(fc, bpf_fill_super);
 }
 
 static void bpf_free_fc(struct fs_context *fc)
-- 
2.17.1


             reply	other threads:[~2022-02-23 13:18 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-23 13:18 Yafang Shao [this message]
2022-02-23 15:36 ` [PATCH] bpf: Refuse to mount bpffs on the same mount point multiple times Yonghong Song
2022-02-23 16:17   ` Yafang Shao
2022-02-23 16:53     ` Yonghong Song

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=20220223131833.51991-1-laoar.shao@gmail.com \
    --to=laoar.shao@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=ebiederm@xmission.com \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kpsingh@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox