From: Al Viro <viro@zeniv.linux.org.uk>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Christian Brauner <brauner@kernel.org>,
Christoph Hellwig <hch@lst.de>,
Andrii Nakryiko <andrii.nakryiko@gmail.com>,
Andrii Nakryiko <andrii@kernel.org>, bpf <bpf@vger.kernel.org>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Martin KaFai Lau <martin.lau@kernel.org>,
Aleksa Sarai <cyphar@cyphar.com>,
Lennart Poettering <lennart@poettering.net>,
Linux-Fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: fd == 0 means AT_FDCWD BPF_OBJ_GET commands
Date: Sat, 26 Aug 2023 05:27:50 +0100 [thread overview]
Message-ID: <20230826042750.GP3390869@ZenIV> (raw)
In-Reply-To: <20230519044433.2chdcze3qg2eho77@MacBook-Pro-8.local>
On Thu, May 18, 2023 at 09:44:33PM -0700, Alexei Starovoitov wrote:
> That footgun was removed from folly in 2021, but we still see this issue from time to time.
> My point that the kernel can help here.
> Since folks don't like sysctl to control FD assignment how about something like this:
>
> diff --git a/fs/file.c b/fs/file.c
> index 7893ea161d77..896e79433f61 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -554,9 +554,15 @@ static int alloc_fd(unsigned start, unsigned end, unsigned flags)
> return error;
> }
>
> +__weak noinline u32 get_start_fd(void)
> +{
> + return 0;
> +}
> +/* mark it as BPF_MODIFY_RETURN to let bpf progs adjust return value */
> +
> int __get_unused_fd_flags(unsigned flags, unsigned long nofile)
> {
> - return alloc_fd(0, nofile, flags);
> + return alloc_fd(get_start_fd(), nofile, flags);
> }
>
> Then we can enforce fd >= 3 for a certain container or for a particular app.
[an extremely belated reply - had been net.dead since mid-May, just got to
that thread]
As far as I'm concerned, the main conclusion is that BPF handling of file
descriptors needs a fairly hostile code review, regarding the interactions
with dup2(), shared descriptor tables, SCM_RIGHTS, etc. Rationale:
demonstrated utter lack of clue about the nature of file descriptors,
along with a weird mental model of how they are used, complete with
"if they are used not in the way we expect, let's shove a hook
somewhere to enforce The Right Way(tm)". Will do...
next prev parent reply other threads:[~2023-08-26 4:27 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-16 0:13 [PATCH bpf-next 0/3] Add O_PATH-based BPF_OBJ_PIN and BPF_OBJ_GET support Andrii Nakryiko
2023-05-16 0:13 ` [PATCH bpf-next 1/3] bpf: support O_PATH FDs in BPF_OBJ_PIN and BPF_OBJ_GET commands Andrii Nakryiko
2023-05-16 8:52 ` Jiri Olsa
2023-05-16 18:02 ` Andrii Nakryiko
2023-05-16 9:07 ` Christian Brauner
2023-05-16 18:02 ` Andrii Nakryiko
2023-05-17 9:11 ` fd == 0 means AT_FDCWD " Christian Brauner
2023-05-17 12:05 ` Christoph Hellwig
2023-05-17 16:17 ` Alexei Starovoitov
2023-05-17 21:48 ` Alexei Starovoitov
2023-05-18 8:38 ` Christian Brauner
2023-05-18 14:30 ` Theodore Ts'o
2023-05-18 16:25 ` Alexei Starovoitov
2023-05-18 16:33 ` Matthew Wilcox
2023-05-18 17:22 ` Christian Brauner
2023-05-18 17:20 ` Christian Brauner
2023-05-18 17:33 ` Linus Torvalds
2023-05-18 18:21 ` Christian Brauner
2023-05-18 18:26 ` Alexei Starovoitov
2023-05-18 18:57 ` Linus Torvalds
2023-05-19 4:44 ` Alexei Starovoitov
2023-05-19 8:13 ` Christian Brauner
2023-05-19 14:27 ` Theodore Ts'o
2023-05-19 17:51 ` Linus Torvalds
2023-05-23 7:49 ` Lennart Poettering
2023-05-23 17:25 ` Andrii Nakryiko
2023-08-26 4:27 ` Al Viro [this message]
2023-05-18 21:56 ` Andrii Nakryiko
2023-05-16 0:13 ` [PATCH bpf-next 2/3] libbpf: add opts-based bpf_obj_pin() API and add support for path_fd Andrii Nakryiko
2023-05-16 0:13 ` [PATCH bpf-next 3/3] selftests/bpf: add path_fd-based BPF_OBJ_PIN and BPF_OBJ_GET tests Andrii Nakryiko
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=20230826042750.GP3390869@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brauner@kernel.org \
--cc=cyphar@cyphar.com \
--cc=daniel@iogearbox.net \
--cc=hch@lst.de \
--cc=lennart@poettering.net \
--cc=linux-fsdevel@vger.kernel.org \
--cc=martin.lau@kernel.org \
--cc=torvalds@linux-foundation.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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.