public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Lennart Poettering <mzxreary@0pointer.de>
To: Kuniyuki Iwashima <kuniyu@amazon.com>
Cc: bluca@debian.org, alexander@mihalicyn.com, brauner@kernel.org,
	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 10:56:03 +0200	[thread overview]
Message-ID: <aCMJI-2goig2VBDX@gardel-login> (raw)
In-Reply-To: <20250513021626.86287-1-kuniyu@amazon.com>

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.

Lennart

--
Lennart Poettering, Berlin

  reply	other threads:[~2025-05-13  8:56 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 [this message]
2025-05-13 12:08             ` Christian Brauner
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=aCMJI-2goig2VBDX@gardel-login \
    --to=mzxreary@0pointer.de \
    --cc=alexander@mihalicyn.com \
    --cc=bluca@debian.org \
    --cc=brauner@kernel.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=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