Re: [PATCH net v2] scm: fix negative fds with SO_PASSPIDFD

[Alexander Mikhalitsyn]

Am So., 17. Nov. 2024 um 10:13 Uhr schrieb Stas Sergeev <stsp2@xxxxxxxxx>:
>
> pidfd_prepare() can return negative values as an error codes.
> But scm_pidfd_recv() didn't check for that condition.
> As the result, it is possible to create the race that leads to
> the negative fds. The race happens if the peer process sends
> something to SO_PASSPIDFD-enabled recipient, and quickly exits.
> pidfd_prepare() has this code:
>
>     if (!pid || !pid_has_task(pid, thread ? PIDTYPE_PID : PIDTYPE_TGID))
>             return -EINVAL;
>
> So if you exit quickly enough, you can hit that EINVAL.
> Getting the fd=-22 is very weird, if not exploitable.
>
> This patch adds the missing check and sets MSG_CTRUNC on error.
> Recipient can now detect an error by checking this flag.
>
> Changes in v2: add Fixes tag
>
> Signed-off-by: Stas Sergeev <stsp2@xxxxxxxxx>

Hi Stas,

Actually, it's not a forgotten check. It's intended behavior to pass
through errors from pidfd_prepare() to
the userspace. In my first version [1] of the patch I tried to return
ESRCH instead of EINVAL in your case, but
then during discussions we decided to remove that.

[1] https://lore.kernel.org/all/20230316131526.283569-2-aleksandr.mikhalitsyn@xxxxxxxxxxxxx/

Kind regards,
Alex

>
> Fixes: 5e2ff6704a2 ("scm: add SO_PASSPIDFD and SCM_PIDFD")
>
> CC: Alexander Mikhalitsyn <alexander@xxxxxxxxxxxxx>
> CC: "David S. Miller" <davem@xxxxxxxxxxxxx>
> CC: Eric Dumazet <edumazet@xxxxxxxxxx>
> CC: Jakub Kicinski <kuba@xxxxxxxxxx>
> CC: Paolo Abeni <pabeni@xxxxxxxxxx>
> CC: Simon Horman <horms@xxxxxxxxxx>
> CC: Christian Brauner <brauner@xxxxxxxxxx>
> CC: Kees Cook <kees@xxxxxxxxxx>
> CC: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>
> CC: netdev@xxxxxxxxxxxxxxx
> CC: linux-kernel@xxxxxxxxxxxxxxx
> ---
>  include/net/scm.h | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/include/net/scm.h b/include/net/scm.h
> index 0d35c7c77a74..3ccf8546c506 100644
> --- a/include/net/scm.h
> +++ b/include/net/scm.h
> @@ -155,6 +155,10 @@ static __inline__ void scm_pidfd_recv(struct msghdr *msg, struct scm_cookie *scm
>                 return;
>
>         pidfd = pidfd_prepare(scm->pid, 0, &pidfd_file);
> +       if (pidfd < 0) {
> +               msg->msg_flags |= MSG_CTRUNC;
> +               return;
> +       }
>
>         if (put_cmsg(msg, SOL_SOCKET, SCM_PIDFD, sizeof(int), &pidfd)) {
>                 if (pidfd_file) {
> --
> 2.47.0
>