Skip to content

Resolve scan-build regressions#47

Merged
jserv merged 1 commit into
mainfrom
fix
May 26, 2026
Merged

Resolve scan-build regressions#47
jserv merged 1 commit into
mainfrom
fix

Conversation

@jserv
Copy link
Copy Markdown
Contributor

@jserv jserv commented May 26, 2026

CI scan-build raised eleven warnings against the rosetta merge. Resolving them surfaced a latent utimensat bug: Linux UTIME_NOW (0x3fffffff) and UTIME_OMIT (0x3ffffffe) were passed straight through to macOS, whose sys/stat.h uses -1 and -2 for those sentinels. Guests asking for UTIME_NOW were getting an nsec value > 999999999, which the host utimensat rejects as EINVAL; UTIME_OMIT silently degraded to "set tv_nsec to a billion". Both are now translated explicitly, and both-OMIT is short-circuited before any fd/flag validation since the call is a guaranteed no-op.

src/core/stack.c (build_linux_stack)

  • Always allocate at least one calloc slot for arg_ptrs/env_ptrs so the pointers are never NULL. The empty-loop case is a no-op, and the static analyzer no longer has to correlate (argc > 0) with arg_ptrs being non-NULL.

src/core/rosetta.c (rosetta_finalize)

  • Drop the ownership-transfer bin_host_fd = -1 sentinel. No goto fail path runs past fd_alloc_at(3) on success, so the store was dead. The refreshed comment warns future maintainers that adding any fallible step below the commit point requires explicit ownership handling.

src/syscall/mem.c (sys_mmap success path, mmap_fork_prepare_anon_shared)

  • Drop the track_backing_fd = -1 sentinel after guest_region_add_ex_owned_gpa() takes ownership; same audit, same comment as the rosetta change. Tighten the mmap_fork_prepare_anon_shared API so txn_out must be non-NULL, returning -LINUX_EINVAL up front; the three "if (txn_out) *txn_out = txn" guards in error paths were the leak scan-build flagged. The sole caller in forkipc.c already supplies a non-NULL out-pointer.

src/syscall/fuse.c (fuse_dev_write FUSE_INIT handling)

  • Skip the init_out copy and session teardown when the local reply-buffer malloc earlier failed (req->reply == NULL with reply_len

    0). The previous code fell into the protocol-error else branch and
    overwrote -LINUX_ENOMEM with -LINUX_EPROTO while marking the daemon dead, turning a host OOM into a fake protocol failure. The new local_oom branch preserves -LINUX_ENOMEM and leaves session_state alone.

src/syscall/sidecar.c, fd.c, inotify.c

  • Replace read(fd, buf, n) with readv(fd, &iov, 1) for the three drain / locked-file-read sites that scan-build's unix.BlockInCriticalSection flagged. Each read site is either a regular file under the sidecar fcntl lock or an O_NONBLOCK pipe drain, neither of which can actually block; the checker only knows the read/recv names, so readv with a single iovec is functionally identical and silences the warning.

src/syscall/fs.c (sys_utimensat)

  • Translate Linux UTIME_NOW/UTIME_OMIT to the macOS sentinels and short-circuit when both timestamps are UTIME_OMIT. The original passthrough relied on the (incorrect) comment that the constants matched on macOS; they do not. Reject flags != 0 combined with a NULL path with EINVAL (Linux's do_utimes_fd does the same), and reject utimensat(AT_FDCWD, NULL, ...) with EFAULT before futimens(-2, ...) would return EBADF and diverge from Linux.

src/syscall/fs-stat.c (sys_fstat, sys_newfstatat, sys_statx)

  • Zero-initialize the local struct stat before the host call so partial FUSE / /proc emulator backfills do not expose stack garbage through translate_stat / translate_statx.

src/syscall/abi.h

  • Define LINUX_UTIME_NOW and LINUX_UTIME_OMIT alongside the existing AT_* sentinels so the fs.c translation reads from one source of truth.

Summary by cubic

Fixes scan-build regressions and corrects utimensat behavior on macOS by translating Linux UTIME_NOW/UTIME_OMIT and short-circuiting no-ops. This removes EINVAL/EPROTO false errors and improves safety around FUSE init, mmap fork, and stat handling.

  • Bug Fixes

    • Translate UTIME_NOW/UTIME_OMIT to macOS sentinels, short-circuit both-OMIT, and match Linux validation: reject flags with NULL path (EINVAL) and AT_FDCWD with NULL path (EFAULT).
    • Preserve -ENOMEM on FUSE INIT reply-buffer OOM and mark the daemon dead to wake waiters; propagate daemon error codes correctly instead of reporting EPROTO.
    • Zero-initialize struct stat before host calls to prevent leaking stack garbage into guest results.
  • Refactors

    • Replace read(...) with readv(..., 1) for non-blocking drains and locked-file reads to satisfy clang’s checker without behavior changes.
    • Remove ownership-transfer sentinels in rosetta finalize and mmap success paths; document commit points where failures must not be added.
    • Require non-NULL txn_out in mmap_fork_prepare_anon_shared (return -LINUX_EINVAL otherwise); always allocate at least one slot for argv/envp arrays to keep pointers non-NULL.

Written for commit b6ad3cb. Summary will update on new commits. Review in cubic

cubic-dev-ai[bot]

This comment was marked as resolved.

CI scan-build raised eleven warnings against the rosetta merge. Resolving
them surfaced a latent utimensat bug: Linux UTIME_NOW (0x3fffffff) and
UTIME_OMIT (0x3ffffffe) were passed straight through to macOS, whose
sys/stat.h uses -1 and -2 for those sentinels. Guests asking for
UTIME_NOW were getting an nsec value > 999999999, which the host
utimensat rejects as EINVAL; UTIME_OMIT silently degraded to "set tv_nsec
to a billion". Both are now translated explicitly, and both-OMIT is
short-circuited before any fd/flag validation since the call is a
guaranteed no-op.

src/core/stack.c (build_linux_stack)
- Always allocate at least one calloc slot for arg_ptrs/env_ptrs so the
  pointers are never NULL. The empty-loop case is a no-op, and the
  static analyzer no longer has to correlate (argc > 0) with arg_ptrs
  being non-NULL.

src/core/rosetta.c (rosetta_finalize)
- Drop the ownership-transfer bin_host_fd = -1 sentinel. No goto fail
  path runs past fd_alloc_at(3) on success, so the store was dead. The
  refreshed comment warns future maintainers that adding any fallible
  step below the commit point requires explicit ownership handling.

src/syscall/mem.c (sys_mmap success path, mmap_fork_prepare_anon_shared)
- Drop the track_backing_fd = -1 sentinel after
  guest_region_add_ex_owned_gpa() takes ownership; same audit, same
  comment as the rosetta change. Tighten the
  mmap_fork_prepare_anon_shared API so txn_out must be non-NULL,
  returning -LINUX_EINVAL up front; the three "if (txn_out) *txn_out =
  txn" guards in error paths were the leak scan-build flagged. The sole
  caller in forkipc.c already supplies a non-NULL out-pointer.

src/syscall/fuse.c (fuse_dev_write FUSE_INIT handling)
- Detect the local reply-buffer OOM (req->reply == NULL with reply_len
  > 0) up front and take a dedicated branch. The previous code fell
  into the protocol-error else branch and overwrote -LINUX_ENOMEM with
  -LINUX_EPROTO while marking the daemon dead, turning a host OOM into
  a fake protocol failure. The new local_oom branch preserves
  -LINUX_ENOMEM as the request error so the originator sees the root
  cause, and still marks session->daemon_dead so init waiters wake out
  of fuse_wait_for_init_locked with -LINUX_ENOTCONN; otherwise the
  init_cond broadcast below would let them re-block on the still-false
  init_done flag.

src/syscall/sidecar.c, fd.c, inotify.c
- Replace read(fd, buf, n) with readv(fd, &iov, 1) for the three drain
  / locked-file-read sites that scan-build's unix.BlockInCriticalSection
  flagged. Each read site is either a regular file under the sidecar
  fcntl lock or an O_NONBLOCK pipe drain, neither of which can actually
  block; the checker only knows the read/recv names, so readv with a
  single iovec is functionally identical and silences the warning.

src/syscall/fs.c (sys_utimensat)
- Translate Linux UTIME_NOW/UTIME_OMIT to the macOS sentinels and
  short-circuit when both timestamps are UTIME_OMIT. The original
  passthrough relied on the (incorrect) comment that the constants
  matched on macOS; they do not. Reject flags != 0 combined with a NULL
  path with EINVAL (Linux's do_utimes_fd does the same), and reject
  utimensat(AT_FDCWD, NULL, ...) with EFAULT before futimens(-2, ...)
  would return EBADF and diverge from Linux.

src/syscall/fs-stat.c (sys_fstat, sys_newfstatat, sys_statx)
- Zero-initialize the local struct stat before the host call so
  partial FUSE / /proc emulator backfills do not expose stack garbage
  through translate_stat / translate_statx.

src/syscall/abi.h
- Define LINUX_UTIME_NOW and LINUX_UTIME_OMIT alongside the existing
  AT_* sentinels so the fs.c translation reads from one source of truth.
@jserv jserv merged commit 828e9ca into main May 26, 2026
4 checks passed
@jserv jserv deleted the fix branch May 26, 2026 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant