From: Christian Brauner <brauner@kernel.org>
To: Lennart Poettering <mzxreary@0pointer.de>
Cc: Kuniyuki Iwashima <kuniyu@amazon.com>,
bluca@debian.org, alexander@mihalicyn.com,
daan.j.demeyer@gmail.com, daniel@iogearbox.net,
davem@davemloft.net, david@readahead.eu, edumazet@google.com,
horms@kernel.org, jack@suse.cz, jannh@google.com,
kuba@kernel.org, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, me@yhndnzj.com,
netdev@vger.kernel.org, oleg@redhat.com, pabeni@redhat.com,
viro@zeniv.linux.org.uk, zbyszek@in.waw.pl
Subject: Re: [PATCH v6 4/9] coredump: add coredump socket
Date: Tue, 13 May 2025 14:08:07 +0200 [thread overview]
Message-ID: <20250513-agitation-tastatur-327692d0caf0@brauner> (raw)
In-Reply-To: <aCMJI-2goig2VBDX@gardel-login>
On Tue, May 13, 2025 at 10:56:03AM +0200, Lennart Poettering wrote:
> On Mo, 12.05.25 19:14, Kuniyuki Iwashima (kuniyu@amazon.com) wrote:
>
> > > > Note this version does not use prefix. Now it requires users to
> > > > just pass the socket cookie via core_pattern so that the kernel
> > > > can verify the peer.
> > >
> > > Exactly - this means the pattern cannot be static in a sysctl.d early
> > > on boot anymore, and has to be set dynamically by <something>.
> >
> > You missed the socket has to be created dynamically by <something>.
>
> systemd implements socket activation: the generic code in PID 1 can
> bind a socket, and then generically forks off a process (or instances
> of processes for connection-based sockets) once traffic is seen on
> that socket. On a typical, current systemd system, PID 1 does this for
> ~40 sockets by default. The code to bind AF_UNIX or AF_INET/AF_INET6
> sockets is entirely generic.
>
> Currently, in the existing systemd codebase coredumping is implemented
> via socket activation: the core_pattern handler binary quickly hands
> off the coredump fds to an AF_UNIX socket bound that way, and the
> service behind that does the heavy lifting. Our hope is that with
> Christian's work we can make the kernel deliver the coredumps directly
> to the socket PID1 generically binds, getting rid of one middle man.
>
> By requiring userspace to echo the SO_COOKIE value into the
> core_pattern sysctl in a special formatting, you define a bespoke
> protocol: it's not just enough to bind a socket (for which the generic
> code in PID1 is good enough), and to write a fixed
> string into a sysctl (for which the generic code in the current
> /etc/sysctl.d/ manager, i.e. systemd-sysctl, works fine). But you
> suddenly are asking from userspace, that some specific tool runs at
> early boot, extracts the socket cookie from PID1 somehow, and writes
> that into sysctl. We'd have to come up with a new tool for that, we
> can no longer use generic tools. And that's the part that Luca doesn't
> like.
>
> To a large degree I agree with Luca about this. I would much prefer
> Christian's earlier proposal (i.e. to simply define some prefix of
> AF_UNIX abstract namespace addresses as requiring privs to bind),
> because that would enable us to do generic handling in userspace: the
> existing socket binding logic in PID 1, and the existing sysctl.d
> handling in the systemd suite would be good enough to set up
> everything for the coredump handling.
>
> That said, I'd take what we can get. If enforcing privs on some
> abstract namespace socket address prefix is not acceptable, then we
> can probably make the SO_COOKIE proposal work (Luca: we'd just hook
> some small tool into ExecStartPost= of the .socket unit, and make PID1
> pass the cookie in some env var or so to it; the tool would then just
> echo that env var into the sysctl with the fixed prefix). In my eyes,
> it's not ideal though: it would mean the sysctl data on every instance
> of the system system image would necessarily deviate (because the
> socket cookie is going to be different), which mgmt tools won't like
> (as you cannot compare sysctl state anymore), and we'd have a weak
> conflict of ownership: right now most sysctl settings are managed by
> /etc/sysctl.d/, but the core_pattern suddenly wouldn't be
> anymore. This will create conflicts because suddenly two components
> write to the thing, and will start fighting.
>
> Hence: I'd *much* prefer Christian's original approach as it does not
> have these issues. But I'll take what I can get, we can make the
> cookie thing work, but it's much uglier.
>
> I am not sure I understand why enforcing privs on some abstract
> namespace socke address prefix is such an unacceptable idea though.
I prefer the prefix approach as well. It's clean, simple and is safe by
itself and elegant. And it fits into the generic socket activation and
system administration models. I mainly show-cased the cookie model as an
elaborate workaround. It can be done but it's ugly and more difficult to
use.
I do have one more idea how to solve this problem cleanly using regular
socket paths that hopefully pleases everyone.
next prev parent reply other threads:[~2025-05-13 12:08 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-12 8:55 [PATCH v6 0/9] coredump: add coredump socket Christian Brauner
2025-05-12 8:55 ` [PATCH v6 1/9] coredump: massage format_corname() Christian Brauner
2025-05-12 8:55 ` [PATCH v6 2/9] coredump: massage do_coredump() Christian Brauner
2025-05-12 8:55 ` [PATCH v6 3/9] coredump: reflow dump helpers a little Christian Brauner
2025-05-12 8:55 ` [PATCH v6 4/9] coredump: add coredump socket Christian Brauner
2025-05-12 10:58 ` Luca Boccassi
2025-05-13 0:17 ` Kuniyuki Iwashima
2025-05-13 1:09 ` Luca Boccassi
2025-05-13 2:14 ` Kuniyuki Iwashima
2025-05-13 8:56 ` Lennart Poettering
2025-05-13 12:08 ` Christian Brauner [this message]
2025-05-13 0:06 ` Kuniyuki Iwashima
2025-05-12 8:55 ` [PATCH v6 5/9] pidfs, coredump: add PIDFD_INFO_COREDUMP Christian Brauner
2025-05-12 8:55 ` [PATCH v6 6/9] coredump: show supported coredump modes Christian Brauner
2025-05-12 8:55 ` [PATCH v6 7/9] coredump: validate socket name as it is written Christian Brauner
2025-05-12 8:55 ` [PATCH v6 8/9] selftests/pidfd: add PIDFD_INFO_COREDUMP infrastructure Christian Brauner
2025-05-12 8:55 ` [PATCH v6 9/9] selftests/coredump: add tests for AF_UNIX coredumps Christian Brauner
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=20250513-agitation-tastatur-327692d0caf0@brauner \
--to=brauner@kernel.org \
--cc=alexander@mihalicyn.com \
--cc=bluca@debian.org \
--cc=daan.j.demeyer@gmail.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=david@readahead.eu \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=jack@suse.cz \
--cc=jannh@google.com \
--cc=kuba@kernel.org \
--cc=kuniyu@amazon.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=me@yhndnzj.com \
--cc=mzxreary@0pointer.de \
--cc=netdev@vger.kernel.org \
--cc=oleg@redhat.com \
--cc=pabeni@redhat.com \
--cc=viro@zeniv.linux.org.uk \
--cc=zbyszek@in.waw.pl \
/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