Skip to content

Conversation

@0xzre
Copy link

@0xzre 0xzre commented Jan 3, 2026

Fixes #2370

Add comprehensive tests for write(2), pwrite(2), and writev(2) syscalls
to improve coverage as requested in the issue.

New tests include:

  • Bad buffer states (EFAULT): nullptr buffer, bad iov_base address
  • Bad file descriptors (EBADF): closed fd, negative fd, read-only fd, O_PATH fd
  • Offset behavior: verify offset increments, pwrite doesn't change offset
  • EOF handling: write extends past EOF, pwrite creates holes
  • Pipes: write to pipe, pwrite returns ESPIPE, write to read end fails
  • Symlinks: write through symlink, O_NOFOLLOW returns ELOOP
  • Special files: /dev/null and /dev/zero
  • writev edge cases: empty iovec, zero-length entries

Also adds similar tests to pwrite64.cc for consistency.

@google-cla
Copy link

google-cla bot commented Jan 3, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@0xzre 0xzre requested a review from ayushr2 January 6, 2026 12:07
@ayushr2
Copy link
Collaborator

ayushr2 commented Jan 6, 2026

Thanks, could you squash your commits?

gvisor/CONTRIBUTING.md

Lines 95 to 99 in 4e9165c

When iterating on a pull request, please amend your commit rather than creating
new commits on the same branch. Our merge bot applies all commits from the
branch to master and does not yet support squash merging. If you'd like to
submit a pull request with multiple commits, please ensure they are independent,
logical units of work that can be applied separately.

@0xzre 0xzre force-pushed the write-syscall-tests branch from 2acf754 to 2b30584 Compare January 6, 2026 15:40
@0xzre
Copy link
Author

0xzre commented Jan 6, 2026

thanks for the guidance @ayushr2 , squashed them🙏

copybara-service bot pushed a commit that referenced this pull request Jan 6, 2026
Fixes #2370

Add comprehensive tests for write(2), pwrite(2), and writev(2) syscalls
to improve coverage as requested in the issue.

New tests include:
- Bad buffer states (EFAULT): nullptr buffer, bad iov_base address
- Bad file descriptors (EBADF): closed fd, negative fd, read-only fd, O_PATH fd
- Offset behavior: verify offset increments, pwrite doesn't change offset
- EOF handling: write extends past EOF, pwrite creates holes
- Pipes: write to pipe, pwrite returns ESPIPE, write to read end fails
- Symlinks: write through symlink, O_NOFOLLOW returns ELOOP
- Special files: /dev/null and /dev/zero
- writev edge cases: empty iovec, zero-length entries

Also adds similar tests to `pwrite64.cc` for consistency.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12426 from 0xzre:write-syscall-tests 2b30584
PiperOrigin-RevId: 852782177
@0xzre
Copy link
Author

0xzre commented Jan 6, 2026

there seems to be error on build CI because compiler warning, lookin on it

@0xzre
Copy link
Author

0xzre commented Jan 6, 2026

it seems the compiler detects and warns about passing nullptr at compile time, should I use volatile to prevent compiler from optimizing away the nullptr check? @ayushr2
something like:

  volatile void* ptr = nullptr;
  EXPECT_THAT(pwrite64(fd.get(), const_cast<void*>(ptr), 1, 0),
         SyscallFailsWithErrno(EFAULT));

@ayushr2
Copy link
Collaborator

ayushr2 commented Jan 6, 2026

I am a bit wary about obfuscating the pointer like that to fool libc. The libc headers declare these functions with attributes (specifically __attribute__((nonnull))) indicating that the buffer pointer must not be null when the size is non-zero.

@ayushr2
Copy link
Collaborator

ayushr2 commented Jan 6, 2026

Maybe bypass the libc wrapper using syscall():

// Use syscall() to bypass libc non-null checks.
  EXPECT_THAT(syscall(SYS_pwrite64, fd.get(), nullptr, 1, 0),
              SyscallFailsWithErrno(EFAULT));
// Use syscall() to bypass libc non-null checks.
  EXPECT_THAT(syscall(SYS_write, fd.get(), nullptr, 1),
              SyscallFailsWithErrno(EFAULT));

(be sure to add comments to justify the syscall() usage)

We do this in some other tests as well.

@ayushr2
Copy link
Collaborator

ayushr2 commented Jan 6, 2026

Also, could you run clang-tidy on this? Seeing a warning no header providing "SEEK_SET" is directly included in test/syscalls/linux/pwrite64.cc.

@0xzre
Copy link
Author

0xzre commented Jan 6, 2026

Also, could you run clang-tidy on this? Seeing a warning no header providing "SEEK_SET" is directly included in test/syscalls/linux/pwrite64.cc.

hi can help ref which build this warning appear on the CI?
also I ran clang-tidy test/syscalls/linux/pwrite64.cc -- -I. -I./include -I/usr/include -std=c++17 but it the expected warning does not appear 🤔 I did not find also any command to clang-tidy on this codebase

@ayushr2
Copy link
Collaborator

ayushr2 commented Jan 6, 2026

Also, could you run clang-tidy on this? Seeing a warning no header providing "SEEK_SET" is directly included in test/syscalls/linux/pwrite64.cc.

hi can help ref which build this warning appear on the CI? also I ran clang-tidy test/syscalls/linux/pwrite64.cc -- -I. -I./include -I/usr/include -std=c++17 but it the expected warning does not appear 🤔 I did not find also any command to clang-tidy on this codebase

This is from our internal CI build. You can probably ignore this as both files already have this warning and it is not introduced by this PR. Lets fix the other issues.

@0xzre 0xzre force-pushed the write-syscall-tests branch from 2b30584 to 6b51a40 Compare January 7, 2026 13:58
copybara-service bot pushed a commit that referenced this pull request Jan 7, 2026
Fixes #2370

Add comprehensive tests for write(2), pwrite(2), and writev(2) syscalls
to improve coverage as requested in the issue.

New tests include:
- Bad buffer states (EFAULT): nullptr buffer, bad iov_base address
- Bad file descriptors (EBADF): closed fd, negative fd, read-only fd, O_PATH fd
- Offset behavior: verify offset increments, pwrite doesn't change offset
- EOF handling: write extends past EOF, pwrite creates holes
- Pipes: write to pipe, pwrite returns ESPIPE, write to read end fails
- Symlinks: write through symlink, O_NOFOLLOW returns ELOOP
- Special files: /dev/null and /dev/zero
- writev edge cases: empty iovec, zero-length entries

Also adds similar tests to `pwrite64.cc` for consistency.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12426 from 0xzre:write-syscall-tests 6b51a40
PiperOrigin-RevId: 852782177
copybara-service bot pushed a commit that referenced this pull request Jan 7, 2026
Fixes #2370

Add comprehensive tests for write(2), pwrite(2), and writev(2) syscalls
to improve coverage as requested in the issue.

New tests include:
- Bad buffer states (EFAULT): nullptr buffer, bad iov_base address
- Bad file descriptors (EBADF): closed fd, negative fd, read-only fd, O_PATH fd
- Offset behavior: verify offset increments, pwrite doesn't change offset
- EOF handling: write extends past EOF, pwrite creates holes
- Pipes: write to pipe, pwrite returns ESPIPE, write to read end fails
- Symlinks: write through symlink, O_NOFOLLOW returns ELOOP
- Special files: /dev/null and /dev/zero
- writev edge cases: empty iovec, zero-length entries

Also adds similar tests to `pwrite64.cc` for consistency.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12426 from 0xzre:write-syscall-tests 6b51a40
PiperOrigin-RevId: 852782177
copybara-service bot pushed a commit that referenced this pull request Jan 7, 2026
Fixes #2370

Add comprehensive tests for write(2), pwrite(2), and writev(2) syscalls
to improve coverage as requested in the issue.

New tests include:
- Bad buffer states (EFAULT): nullptr buffer, bad iov_base address
- Bad file descriptors (EBADF): closed fd, negative fd, read-only fd, O_PATH fd
- Offset behavior: verify offset increments, pwrite doesn't change offset
- EOF handling: write extends past EOF, pwrite creates holes
- Pipes: write to pipe, pwrite returns ESPIPE, write to read end fails
- Symlinks: write through symlink, O_NOFOLLOW returns ELOOP
- Special files: /dev/null and /dev/zero
- writev edge cases: empty iovec, zero-length entries

Also adds similar tests to `pwrite64.cc` for consistency.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12426 from 0xzre:write-syscall-tests 6b51a40
PiperOrigin-RevId: 852782177
@0xzre
Copy link
Author

0xzre commented Jan 7, 2026

checked the build errors, and found all related on how FUSE translates EFAULT into EIO. skipping the tests that doesn't apply to FUSE fixes this.

local test with bazel test //test/syscalls:write_test_runsc_systrap_fuse //test/syscalls:pwrite64_test_runsc_systrap_fuse attached.

image image

@0xzre 0xzre force-pushed the write-syscall-tests branch from 6b51a40 to 7880a4c Compare January 7, 2026 16:13
@ayushr2
Copy link
Collaborator

ayushr2 commented Jan 7, 2026

found all related on how FUSE translates EFAULT into EIO

I think this might be a bug in our FUSE implementation. Looking.

@0xzre 0xzre force-pushed the write-syscall-tests branch from 7880a4c to 4f4666f Compare January 8, 2026 03:46
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
Fixes #2370

Add comprehensive tests for write(2), pwrite(2), and writev(2) syscalls
to improve coverage as requested in the issue.

New tests include:
- Bad buffer states (EFAULT): nullptr buffer, bad iov_base address
- Bad file descriptors (EBADF): closed fd, negative fd, read-only fd, O_PATH fd
- Offset behavior: verify offset increments, pwrite doesn't change offset
- EOF handling: write extends past EOF, pwrite creates holes
- Pipes: write to pipe, pwrite returns ESPIPE, write to read end fails
- Symlinks: write through symlink, O_NOFOLLOW returns ELOOP
- Special files: /dev/null and /dev/zero
- writev edge cases: empty iovec, zero-length entries

Also adds similar tests to `pwrite64.cc` for consistency.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12426 from 0xzre:write-syscall-tests 4f4666f
PiperOrigin-RevId: 852782177
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
Fixes #2370

Add comprehensive tests for write(2), pwrite(2), and writev(2) syscalls
to improve coverage as requested in the issue.

New tests include:
- Bad buffer states (EFAULT): nullptr buffer, bad iov_base address
- Bad file descriptors (EBADF): closed fd, negative fd, read-only fd, O_PATH fd
- Offset behavior: verify offset increments, pwrite doesn't change offset
- EOF handling: write extends past EOF, pwrite creates holes
- Pipes: write to pipe, pwrite returns ESPIPE, write to read end fails
- Symlinks: write through symlink, O_NOFOLLOW returns ELOOP
- Special files: /dev/null and /dev/zero
- writev edge cases: empty iovec, zero-length entries

Also adds similar tests to `pwrite64.cc` for consistency.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12426 from 0xzre:write-syscall-tests 4f4666f
PiperOrigin-RevId: 852782177
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
Fixes #2370

Add comprehensive tests for write(2), pwrite(2), and writev(2) syscalls
to improve coverage as requested in the issue.

New tests include:
- Bad buffer states (EFAULT): nullptr buffer, bad iov_base address
- Bad file descriptors (EBADF): closed fd, negative fd, read-only fd, O_PATH fd
- Offset behavior: verify offset increments, pwrite doesn't change offset
- EOF handling: write extends past EOF, pwrite creates holes
- Pipes: write to pipe, pwrite returns ESPIPE, write to read end fails
- Symlinks: write through symlink, O_NOFOLLOW returns ELOOP
- Special files: /dev/null and /dev/zero
- writev edge cases: empty iovec, zero-length entries

Also adds similar tests to `pwrite64.cc` for consistency.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12426 from 0xzre:write-syscall-tests 4f4666f
PiperOrigin-RevId: 852782177
@ayushr2
Copy link
Collaborator

ayushr2 commented Jan 8, 2026

Your tests caught another bug in tmpfs :) This is why testing is important.

I will send a fix for that and then try to get this submitted.

@0xzre
Copy link
Author

0xzre commented Jan 8, 2026

Your tests caught another bug in tmpfs :) This is why testing is important.

I will send a fix for that and then try to get this submitted.

glad the test is proving its worth immediately. let me know if I can help w anything

@ayushr2
Copy link
Collaborator

ayushr2 commented Jan 9, 2026

8a0eddd (fix) is submitted

copybara-service bot pushed a commit that referenced this pull request Jan 9, 2026
Fixes #2370

Add comprehensive tests for write(2), pwrite(2), and writev(2) syscalls
to improve coverage as requested in the issue.

New tests include:
- Bad buffer states (EFAULT): nullptr buffer, bad iov_base address
- Bad file descriptors (EBADF): closed fd, negative fd, read-only fd, O_PATH fd
- Offset behavior: verify offset increments, pwrite doesn't change offset
- EOF handling: write extends past EOF, pwrite creates holes
- Pipes: write to pipe, pwrite returns ESPIPE, write to read end fails
- Symlinks: write through symlink, O_NOFOLLOW returns ELOOP
- Special files: /dev/null and /dev/zero
- writev edge cases: empty iovec, zero-length entries

Also adds similar tests to `pwrite64.cc` for consistency.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12426 from 0xzre:write-syscall-tests 4f4666f
PiperOrigin-RevId: 852782177
copybara-service bot pushed a commit that referenced this pull request Jan 9, 2026
Fixes #2370

Add comprehensive tests for write(2), pwrite(2), and writev(2) syscalls
to improve coverage as requested in the issue.

New tests include:
- Bad buffer states (EFAULT): nullptr buffer, bad iov_base address
- Bad file descriptors (EBADF): closed fd, negative fd, read-only fd, O_PATH fd
- Offset behavior: verify offset increments, pwrite doesn't change offset
- EOF handling: write extends past EOF, pwrite creates holes
- Pipes: write to pipe, pwrite returns ESPIPE, write to read end fails
- Symlinks: write through symlink, O_NOFOLLOW returns ELOOP
- Special files: /dev/null and /dev/zero
- writev edge cases: empty iovec, zero-length entries

Also adds similar tests to `pwrite64.cc` for consistency.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12426 from 0xzre:write-syscall-tests 4f4666f
PiperOrigin-RevId: 852782177
copybara-service bot pushed a commit that referenced this pull request Jan 9, 2026
Fixes #2370

Add comprehensive tests for write(2), pwrite(2), and writev(2) syscalls
to improve coverage as requested in the issue.

New tests include:
- Bad buffer states (EFAULT): nullptr buffer, bad iov_base address
- Bad file descriptors (EBADF): closed fd, negative fd, read-only fd, O_PATH fd
- Offset behavior: verify offset increments, pwrite doesn't change offset
- EOF handling: write extends past EOF, pwrite creates holes
- Pipes: write to pipe, pwrite returns ESPIPE, write to read end fails
- Symlinks: write through symlink, O_NOFOLLOW returns ELOOP
- Special files: /dev/null and /dev/zero
- writev edge cases: empty iovec, zero-length entries

Also adds similar tests to `pwrite64.cc` for consistency.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12426 from 0xzre:write-syscall-tests 4f4666f
PiperOrigin-RevId: 852782177
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Beef up write syscall tests

2 participants