Re: [PATCH net] netlink: render SCM_CREDENTIALS pid in the receiver's pid namespace

[Maoyi Xie]

Please drop this patch.

Sorry for the time this cost you both. I sent it without a selftest and
without a use case, and that was the wrong order.

Eric Dumazet wrote:
> Please include a dedicated selftest in tools/testing/selftests/net/
> exercising SO_PASSCRED / SCM_CREDENTIALS
> over netlink across PID namespaces to demonstrate the issue and
> validate the proposed changes.

I wrote it as tools/testing/selftests/net/netlink_scm_pidns.c and have
not posted it, since I am asking you to drop the patch. cross_pidns
fails on 66817a979426 and passes with the patch. reader_in_child_ns goes
the other way. It passes on 66817a979426 and fails with the patch
applied. With the patch, an unprivileged sender in the parent pid
namespace makes a NETLINK_USERSOCK reader in a child namespace see pid
0. No userspace sender can produce 0 today, since find_get_pid(0) is
NULL. That is why I am asking you to drop the patch.

Alexander Mikhalitsyn wrote:
> 1. We need to start from use-case here, because netlink sockets are
> quite special in many ways.

What I measured on an unpatched kernel is that a task with no
capabilities, after unshare(CLONE_NEWUSER) and unshare(CLONE_NEWPID),
chooses the number a NETLINK_USERSOCK reader in a sibling pid namespace
is told, by picking its own position in the namespace it created. I have
not found a program that reads that number on NETLINK_USERSOCK and acts
on it. avahi does on NETLINK_ROUTE, where it accepts a message only when
the pid is 0, and an unprivileged sender that shares its netns gets
-EPERM there.

Is the sender-chosen number worth changing at all?

Alexander Mikhalitsyn wrote:
> 2. If you start tracking struct pid (which is a good idea
> theoretically, if we ignore 1.), then we should do it a bit smarter
> and
>     make use pidfs API [pidfs_register_pid() function] like we do it
> in unix_maybe_add_creds() to make sure that we properly support
>     dead pidfds too.

With the patch dropped nothing in netlink stores a struct pid, so I have
not added pidfs_register_pid(). If the answer is yes, I will start from
your point 2.

I will not post again unless one of you says it is worth changing.

Thanks.