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

[Alexander Mikhalitsyn]

Am Fr., 4. Sept. 2026 um 17:56 Uhr schrieb Seoah Myeong
<astronaut0703@xxxxxxxxx>:
>
> Hi,

Hi Seoah,

>
> 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.

no, this is not oversight. We are very limited on our ways to report an error
when user-space received socket control messages. One of the ways
could be fail recvmsg() completely, but
there might be more than SCM. In some special cases we raise flags
like MSG_CTRUNC.
In case of SCM_PIDFD we decided to return errno as pidfd and userspace
*must* check for this (see for example [1]).

Link: https://github.com/systemd/systemd/blob/0b5c330d156e91c21d68f246f92790bf212cbd98/src/shared/notify-recv.c#L156
[1]

P.S. Is this LLM generated stuff?

Kind regards,
Alex

>
> 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