linux-kernel - Re: [PATCH net-next v3] af_unix: pass pidfd flags via SCM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <170c8ac6-688e-4d41-82aa-f392872896e7@yandex.ru>
Date: Thu, 21 Nov 2024 16:37:43 +0300
From: stsp <stsp2@...dex.ru>
To: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
Cc: almasrymina@...gle.com, asml.silence@...il.com, axboe@...nel.dk,
 brauner@...nel.org, cyphar@...har.com, davem@...emloft.net,
 edumazet@...gle.com, gouhao@...ontech.com, horms@...nel.org,
 kees@...nel.org, krisman@...e.de, kuba@...nel.org, kuniyu@...zon.com,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org, mhal@...x.co,
 netdev@...r.kernel.org, oleg@...hat.com, pabeni@...hat.com,
 quic_abchauha@...cinc.com, shuah@...nel.org, tandersen@...flix.com,
 viro@...iv.linux.org.uk, willemb@...gle.com
Subject: Re: [PATCH net-next v3] af_unix: pass pidfd flags via SCM_PIDFD cmsg

21.11.2024 15:57, Alexander Mikhalitsyn пишет:
> Hi Stas!
>
> Hmm, it is a bit unusual that SCM_PIDFD message format is different in case
> when you send it and when you read it.
>
> I mean that when you read it (on the receiver side) you get pidfd file descriptor number,
> while when you write it (on the sender side) you are only allowed to send one integer and this time it's
> a pidfd file descriptor flags. I personally have nothing strictly against that but just found this
> a bit unusual and probably confusing for userspace programmers.
>
> Compare it with SCM_CREDENTIALS, for instance, where we read/write the same structure struct ucred.

SCM_PIDFD_FLAGS can be added instead.
But as you currently can't send SCM_PIDFD,
and won't be able to receive SCM_PIDFD_FLAGS
if it is added, then nothing prevents the reuse
of an existing value. So among the possible
solutions is to have SCM_PIDFD_FLAGS as
an alias to SCM_PIDFD.

Should I do that?

>> +				goto error;
>> +			memcpy(&flags, CMSG_DATA(cmsg), sizeof(flags));
>> +			err = pidfd_validate_flags(flags);
> pidfd_validate_flags allows PIDFD_THREAD, but what's the idea behind this if
> scm->pid is always a thread-group leader? (see maybe_add_creds() function).
>
> Sorry if I misunderstand something just want to ensure that we are on the same page.
The idea is to cover only PIDFD_NONBLOCK
for now, and add new flags after. Of course
if PIDFD_NONBLOCK+future extensions sounds
not very convincing, then I'll have to send those
"new flag" patches together with this one.
As you can see, the supplied test-case only
covers PIDFD_NONBLOCK. You can decide
whether or not it is valuable/acceptable on
its own.
Speaking of PIDFD_THREAD, one can add
tid to creds for this to work. I don't need
such functionality, so instead I can as well
disallow sending this flag until someone
else needs it. But it looks potentially useful,
so adding tid can also be considered and
looks simple enough.
What do you think?