#16192

signalfd read: short buffers consume the signal, records are 130 bytes, and only one signal is returned per read

Opensotomayorlucasopened 8d ago

Description

read(2) on a signalfd differs from Linux in three ways:

CaseLinuxgVisor
Buffer smaller than struct signalfd_siginfo (128 bytes)EINVAL, signal stays pendingreturns a truncated record and dequeues the signal, so it is lost
Buffer of 256 bytes, two signals pendingreturns 256 (two records)returns 130 (one record)
Buffer of 128 bytesreturns 128returns 128 (record truncated by 2 bytes)

All three come from pkg/sentry/fsimpl/signalfd/signalfd.go and pkg/abi/linux/signalfd.go:

  • linux.SignalfdSiginfo ends with AddrLSB uint16 followed by _ [48]uint8, so it marshals to 130 bytes. In Linux, everything after ssi_addr_lsb (__pad2, ssi_syscall, ssi_call_addr, ssi_arch, __pad[28]) adds up to 46 bytes, for a total of 128. The padding has been 48 since signalfd support was added (c98e7f0d, 2019).
  • SignalFileDescription.Read dequeues a signal with Sigtimedwait before looking at the buffer size, and then treats ErrEndOfIOSequence as "Partial copy-out ok". That accepts short buffers, which consumes and drops the signal. It also hides the 130-byte size for 128-byte reads, which is why the syscall tests (all of them read sizeof(signalfd_siginfo)) pass.
  • Read dequeues at most one signal. Linux's signalfd_read_iter() loops while there is room for another record. It blocks only for the first one and returns what it has once the queue is empty.

Code path unchanged on master as of 30ace3e2.

Suggested fix:

  1. Change the padding to [46]uint8 so SizeBytes() is 128. A size assertion would help.
  2. In Read, return EINVAL if dst.NumBytes() < SizeBytes() before dequeueing anything, and drop the partial copy-out special case.
  3. Dequeue up to dst.NumBytes() / SizeBytes() signals, blocking only for the first.
  4. Add syscall tests for a short buffer (EINVAL, signal still pending) and for a multi-record read.

Steps to reproduce

// signalfd_short_read.c
#define _GNU_SOURCE
#include <errno.h>
#include <signal.h>
#include <stdio.h>
#include <string.h>
#include <sys/signalfd.h>
#include <unistd.h>

static const char *err(ssize_t r) { return r < 0 ? strerrorname_np(errno) : "-"; }

int main(void) {
  setvbuf(stdout, NULL, _IONBF, 0);
  sigset_t mask, pending;
  sigemptyset(&mask);
  sigaddset(&mask, SIGUSR1);
  sigaddset(&mask, SIGUSR2);
  sigprocmask(SIG_BLOCK, &mask, NULL);
  int fd = signalfd(-1, &mask, SFD_NONBLOCK);

  // 1. Short buffer.
  raise(SIGUSR1);
  char small[4];
  ssize_t r = read(fd, small, sizeof(small));
  sigpending(&pending);
  printf("short read: ret=%zd errno=%s SIGUSR1 still pending=%d\n", r, err(r),
         sigismember(&pending, SIGUSR1));
  struct signalfd_siginfo si[2];
  r = read(fd, si, sizeof(si[0]));
  printf("full read after it: ret=%zd errno=%s\n", r, err(r));

  // 2. Room for two signals with two pending.
  raise(SIGUSR1);
  raise(SIGUSR2);
  r = read(fd, si, sizeof(si));
  printf("two pending, buffer for two: ret=%zd (%zd siginfo)\n", r,
         r > 0 ? r / (ssize_t)sizeof(si[0]) : 0);
  return 0;
}
$ gcc -O2 -static -o signalfd_short_read signalfd_short_read.c
$ docker run --rm --runtime=runc  -v $PWD:/t:ro <any image> /t/signalfd_short_read
short read: ret=-1 errno=EINVAL SIGUSR1 still pending=1
full read after it: ret=128 errno=-
two pending, buffer for two: ret=256 (2 siginfo)

$ docker run --rm --runtime=runsc -v $PWD:/t:ro <any image> /t/signalfd_short_read
short read: ret=4 errno=- SIGUSR1 still pending=0
full read after it: ret=-1 errno=EAGAIN
two pending, buffer for two: ret=130 (1 siginfo)

runsc version

runsc version release-20261012.0
spec: 1.2.1

docker version (if using docker)

29.9.1

uname

Linux 7.0.0-38-generic x86_64

repo state (if built from source)

Not built from source (release binary). Code path checked on master 30ace3e2.

Activity

0

No activity yet.