[BUG] net/scm: scm_pidfd_recv() reports pidfd_prepare() failure to userspace as a file descriptor

[Seoah Myeong]

Hi,

scm_pidfd_recv() does not check the return value of pidfd_prepare().
When pidfd_prepare() fails, its negative errno is passed to put_cmsg()
and delivered to userspace in the SCM_PIDFD control message, in the
field that is specified to hold a file descriptor.  No flag is set to
tell the receiver that anything went wrong.

Affected trees
==============

  mainline (421066905cbc)  net/core/scm.c:489, call at :510
  v7.2-rc5                 net/core/scm.c:465, call at :486
  v6.12.107 (LTS)          include/net/scm.h:136, call at :157

The code is identical in all three; only the location differs, because
38b95d588f8f moved the function out of the header after v6.12.

Analysis
========

The current code in mainline (421066905cbc), net/core/scm.c:

  489  static void scm_pidfd_recv(struct msghdr *msg, struct scm_cookie *scm)
  490  {
  491          struct file *pidfd_file = NULL;
  492          int len, pidfd;
  493
  494          /* put_cmsg() doesn't return an error if CMSG is truncated,
  495           * that's why we need to opencode these checks here.
  496           */
  497          if (msg->msg_flags & MSG_CMSG_COMPAT)
  498                  len = sizeof(struct compat_cmsghdr) + sizeof(int);
  499          else
  500                  len = sizeof(struct cmsghdr) + sizeof(int);
  501
  502          if (msg->msg_controllen < len) {
  503                  msg->msg_flags |= MSG_CTRUNC;
  504                  return;
  505          }
  506
  507          if (!scm->pid)
  508                  return;
  509
  510          pidfd = pidfd_prepare(scm->pid, PIDFD_STALE, &pidfd_file);
  511
  512          if (put_cmsg(msg, SOL_SOCKET, SCM_PIDFD, sizeof(int), &pidfd)) {
  513                  if (pidfd_file) {
  514                          put_unused_fd(pidfd);
  515                          fput(pidfd_file);
  516                  }
  517
  518                  return;
  519          }
  520
  521          if (pidfd_file)
  522                  fd_install(pidfd, pidfd_file);
  523  }

pidfd_prepare() returns a file descriptor on success or a negative
errno on failure.  At line 510 the return value is stored in pidfd, and
at line 512 it is copied into the cmsg without being checked.

On the failure path pidfd_file remains NULL, so the cleanup at 513-516
and the install at 521-522 are both skipped.  Nothing is leaked and no
descriptor is installed; the only consequence is the value that reaches
userspace.

This looks like an oversight rather than intended behaviour:

  - The comment at 494-496 shows the function deliberately opencodes
    error checks that put_cmsg() cannot report, so error reporting in
    this function was considered.

  - The function has already been fixed twice for error-path problems:
    603fc57ab70c ("af_unix: Skip SCM_PIDFD if scm->pid is NULL.") and
    718e6b51298e ("af_unix: Fix msg_controllen test in scm_pidfd_recv()
    for MSG_CMSG_COMPAT.").

  - c679d17d3f2d ("af_unix: enable handing out pidfds for reaped tasks
    in SCM_PIDFD") modified this very call, changing the flags argument
    from 0 to PIDFD_STALE, without adding a check.

Reproduction
============

The attached program creates an AF_UNIX SOCK_STREAM socket pair with
SO_PASSPIDFD enabled on the receiver, lowers RLIMIT_NOFILE so that
pidfd_prepare() fails with -EMFILE, and then calls recvmsg().

  $ ./poc
  pidfd_val = -24        (-EMFILE)
  close(-24)  -> EBADF
  fstat(-24)  -> EBADF
  msg_flags: MSG_CTRUNC not set

3000 of 3000 iterations reproduce this as an unprivileged user
(uid 65534, CapEff = CapPrm = 0).  A root control run reproduces
500 of 500 identically, so this does not depend on privilege.

Testing was done on v6.12.107, where the code is in include/net/scm.h.
I have not run the reproducer on mainline, but the code path is
character-for-character the same apart from the PIDFD_STALE flag.

Observed vs expected
====================

Observed: the SCM_PIDFD cmsg carries a negative errno in the int field
that is specified to carry a file descriptor, with no MSG_CTRUNC or
other indication of failure.

Expected: either the cmsg is not emitted, or the failure is signalled
in a way the receiver can detect.

Impact
======

This is a correctness bug, not a security issue.  To be explicit about
what I checked and did not find:

  - No file descriptor or struct file leak.  pidfd_file stays NULL on
    the failure path, and /proc/sys/fs/file-nr went from 384 to 352
    across the run, i.e. it decreased.

  - No memory corruption.  The run was performed with KASAN and lockdep
    enabled, and neither reported anything.  lockdep was verified to be
    active throughout: its lock-class and dependency-chain counts
    increased during the run and the validator was never turned off.

  - No privilege boundary is crossed.

The practical effect is that a receiver which follows the SCM_PIDFD
contract will store or use a value that is not a descriptor.  In the
common case this surfaces later as an EBADF from an unrelated call
site, which is hard to attribute back to the recvmsg() that produced
it.

Fix direction
=============

I have not sent a patch because the right behaviour looks like a
maintainer decision.  Since 38b95d588f8f the function is a static void
in net/core/scm.c with a single caller in the same file, so changing
its signature to propagate an error is much more contained than it
would have been while it was a header inline.  But choosing between
suppressing the cmsg, setting MSG_CTRUNC, and reporting the error to
the caller has ABI implications for existing receivers, and that is
not my call.

I am happy to test any patch, and to send one myself if you tell me
which behaviour you prefer.

Environment
===========

  Kernel:   v6.12.107, x86_64, QEMU
  Config:   KASAN=y, PROVE_LOCKING=y, DEBUG_LIST=y
  Repro:    3000/3000 unprivileged, 500/500 as root

Reproducer
==========

[PASTE poc.c INLINE HERE]

Thanks,
Seoah Myeong