From: Lennart Poettering <lennart@poettering.net>
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>,
Linux-Fsdevel <linux-fsdevel@vger.kernel.org>,
Al Viro <viro@zeniv.linux.org.uk>
Subject: Re: fd == 0 means AT_FDCWD BPF_OBJ_GET commands
Date: Tue, 23 May 2023 09:49:48 +0200 [thread overview]
Message-ID: <ZGxwHO2MRNK9gYxB@gardel-login> (raw)
In-Reply-To: <20230519044433.2chdcze3qg2eho77@MacBook-Pro-8.local>
On Do, 18.05.23 21:44, Alexei Starovoitov (alexei.starovoitov@gmail.com) wrote:
> > The 0/1/2 file descriptors are not at all special. They are a shell
> > pipeline default, nothing more. They are not the argument your think they
> > are, and you should stop trying to make them an argument.
>
> I'm well aware that any file type is allowed to be in FDs 0,1,2 and
> some user space is using it that way, like old inetd:
> https://github.com/guillemj/inetutils/blob/master/src/inetd.c#L428
> That puts the same socket into 0,1,2 before exec-ing new process.
>
> My point that the kernel has to assist user space instead of
> stubbornly sticking to POSIX and saying all FDs are equal.
>
> Most user space developers know that care should be taken with FDs 0,1,2,
> but it's still easy to make a mistake.
If I look at libbpf, which supposedly gets the fd handling right I
can't find any hint it actually moves the fds it gets from open() to
an fd > 2, though?
i.e. the code that invokes open() calls in the libbpf codebase happily
just accepts an fd < 2, including fd == 0, and this is then later
passed back into the kernel in various bpf() syscall invocations,
which should refuse it, no? So what's going on there?
I did find this though:
<snip>
new_fd = open("/", O_RDONLY | O_CLOEXEC);
if (new_fd < 0) {
err = -errno;
goto err_free_new_name;
}
new_fd = dup3(fd, new_fd, O_CLOEXEC);
if (new_fd < 0) {
err = -errno;
goto err_close_new_fd;
}
</snip>
(This is from libbpf.c, bpf_map__reuse_fd(), i.e. https://github.com/libbpf/libbpf/blob/master/src/libbpf.c)
Not sure what's going on here, what is this about? you allocate an fd
you then immediately replace? Is this done to move the fd away from
fd=0? but that doesn't work that way, in case fd 0 is closed when
entering this function.
Or is this about dup'ping with O_CLOEXEC?
Please be aware that F_DUPFD_CLOEXEC exists, which allows you to
easily move some fd above some treshold, *and* set O_CLOEXEC at the
same time. In the systemd codebase we call this frequently for code
that ends up being loaded in arbitrary processes (glibc NSS modules,
PAM modules), in order to ensure we get out of the fd < 3 territory
quickly.
(btw, if you do care about O_CLOEXEC – which is great – then you also
want to replace a bunch of fopen(…, "r") with fopen(…, "re") in your
codebase)
Lennart
next prev parent reply other threads:[~2023-05-23 8:22 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 [this message]
2023-05-23 17:25 ` Andrii Nakryiko
2023-08-26 4:27 ` Al Viro
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=ZGxwHO2MRNK9gYxB@gardel-login \
--to=lennart@poettering.net \
--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=linux-fsdevel@vger.kernel.org \
--cc=martin.lau@kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/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