Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for fuzzing the Linux AF_VSOCK socket family, including the addition of a new pseudo-syscall syz_vsock_virtio_reprobe to trigger concurrent driver unbind/bind operations. It updates the system call descriptions for VSOCK sockets (adding SOCK_SEQPACKET, zero-copy, socket diagnostics, and timeout options), implements the reprobe logic in the executor, and adds support for launching QEMU instances with a vhost-vsock-pci device using a randomized guest CID. I have no feedback to provide as there are no review comments to evaluate and the implementation aligns with the repository guidelines.
github-actions
Bot
removed
the
well-shaped
Codecov Report❌ Patch coverage is
Additional details and impacted files
🚀 New features to boost your workflow:
|
a-nogikh
left a comment
a-nogikh
left a comment
There was a problem hiding this comment.
Thanks!
The first commit looks good to me (left just one comment on it) - could you please look into that and post it as a separate PR that we'll merge quickly?
Regarding the other commits, I haven't looked too closely at the code since I have a concern about the overall approach.
As I understand, here you make the fuzzed VM play the guest side - it can send the packets to the host, but on our usual fuzzing setup nobody will ever answer. I have a suspicion that it will not add much coverage to what we already could reach.
What if we make the fuzzed kernel play the role of the host? E.g. by adding two pseudo syscalls:
- One that would open and properly configure
/dev/vhost-vsock(syz_vhost_vsock_setup), e.g. by usingVHOST_SET_MEM_TABLEto map guest's buffer on the host. - One that would be the guest and send messages on the mapped ring (
syz_vhost_vsock_send).
I'd ask an agent to try it out and see if it improves the coverage as compared to what we already have on syzbot.
| } | ||
| } | ||
|
|
||
| func TestRandomVsockCID(t *testing.T) { |
There was a problem hiding this comment.
I don't think we need this test - it basically tests the Go library's random number generation.
| } | ||
| if inst.cfg.Vsock { | ||
| args = append(args, | ||
| "-device", fmt.Sprintf("vhost-vsock-pci,guest-cid=%v", inst.vsockCID), |
There was a problem hiding this comment.
Note that it won't work for all architectures, e.g. on s390x it should have been vhost-vsock-ccw and on arm - vhost-vsock-device (?)
|
|
||
| # MSG_ZEROCOPY is honored once SO_ZEROCOPY is set; completions are read with recvmsg(MSG_ERRQUEUE). | ||
| setsockopt$SO_ZEROCOPY_vsock(fd sock_vsock_stream, level const[SOL_SOCKET], opt const[SO_ZEROCOPY], val ptr[in, bool32], len len[val]) | ||
| sendmsg$vsock_stream(fd sock_vsock_stream, msg ptr[in, send_msghdr], f flags[vsock_send_flags]) |
There was a problem hiding this comment.
The kernel will just reject the messages that have a destination set. Maybe we should add a specialized send_msghdr for the vsock purposes, which would always keep it 0?
https://elixir.bootlin.com/linux/v7.3/source/net/vmw_vsock/af_vsock.c#L2213
`accept4$vsock_stream` passed `addrlen` by value, so `accept4()` always failed with `-EFAULT` and accepted sockets never reached the fuzzer. The descriptions also had no `SOCK_SEQPACKET` sockets, no `svm_flags`/`VMADDR_FLAG_TO_HOST`, no `getsockopt` or 64-bit connect timeout, and no typed `MSG_ZEROCOPY` path. vsock diag dumps used the 2-byte generic `sock_diag_req`, which `vsock_diag_handler_dump()` rejects because it needs a 24-byte `struct vsock_diag_req`. Fix `accept4$vsock_stream`, split `svm_zero` into `svm_flags` and padding, and add `socket$vsock_seqpacket`, `getsockopt$vsock`, `setsockopt$SO_VM_SOCKETS_CONNECT_TIMEOUT_NEW`, `setsockopt$SO_ZEROCOPY_vsock`, `sendmsg$vsock_stream`, and `sendmsg$vsock_diag`. `sendmsg$vsock_stream` uses a vsock-specific `send_msghdr_vsock` without a destination, since `vsock_connectible_sendmsg()` rejects messages that carry one. Add manual seed programs for loopback stream (including zerocopy and diag), seqpacket, and vhost-vsock transport selection.
The vhost-vsock descriptions only reach the ioctl surface of `/dev/vhost-vsock`. Nothing acts as the guest, so `vhost_vsock_handle_tx_kick()`, `virtio_transport_recv_pkt()` and the reply path in `vhost_transport_do_send_pkt()` only run on ring state the fuzzer builds by chance, which is almost never a valid packet. Add two pseudo-syscalls that make the fuzzed kernel the host and the executor the guest. `syz_vhost_vsock_setup(cid)` opens `/dev/vhost-vsock`, identity-maps the rx/tx virtqueues and one buffer per descriptor with `VHOST_SET_MEM_TABLE`, assigns the guest CID and starts the device. `syz_vhost_vsock_send(fd, pkt, len)` puts one `virtio_vsock_hdr` + payload packet on the tx virtqueue and kicks the host. It also hands used rx buffers back, so host replies keep flowing. The executor fills in the two CIDs of the header, so every generated packet passes the host's address check. The rest of the header is a typed syzlang struct. The guest CID comes from the per-proc range of `vmaddr_cid` `my`, so host AF_VSOCK sockets can also connect to the emulated guest. Add a manual test program. A guest REQUEST connects to a host listener, data flows both ways and the guest shuts down. A host socket bound to a known port then connects to the guest, which completes the connection with RESPONSE.
b44e7d9 to
71dd13a
Compare
roglew
changed the title
|
Thanks for the review. Here is what changed. The descriptions commit is now its own PR, #8086. It also takes the You were right that the guest-side setup gets little without a peer, so this PR is now only the host-side approach you suggested. The executor fills in The seed Coverage was measured with KCOV on Linux 6.18 using
This matches the gap on syzbot's coverage page for A 13-minute The guest-side commits (QEMU Your other two comments are addressed on that branch. |
|
In case the above comment didn't make it obvious enough, I had an agent take a swing at the revisions. If anything seems weird, if anything ended up too long or annoying to review, or if I should take a closer manual look at anything let me know. |
Stacked on #8086 (first commit); the second commit is this PR.
The vhost-vsock descriptions only reach the ioctl surface of
/dev/vhost-vsock. Nothing acts as the guest, sovhost_vsock_handle_tx_kick(),virtio_transport_recv_pkt()and the reply path invhost_transport_do_send_pkt()only run on ring state the fuzzer builds by chance, which is almost never a valid packet.This adds two pseudo-syscalls that make the fuzzed kernel the host and the executor the guest, as suggested in the review.
syz_vhost_vsock_setup(cid)opens/dev/vhost-vsockand identity-maps the rx and tx virtqueues plus one 4 KB buffer per descriptor withVHOST_SET_MEM_TABLE. It negotiatesVIRTIO_F_VERSION_1(andVIRTIO_VSOCK_F_SEQPACKETwhere available), assigns the guest CID, posts all rx buffers and starts the device. It returns the vhost fd as avhost_vsock_guestresource, avhost_vsock, so the existingioctl$VHOST_*calls can race a running device.syz_vhost_vsock_send(fd, pkt, len)puts one packet on the tx virtqueue and kicks the host, after handing back the rx buffers the host has used. The executor fills insrc_cidanddst_cid; the rest ofvirtio_vsock_hdrand the payload come from the program.Coverage was measured with KCOV on Linux 6.18 for the single seed
sys/linux/test/vsock_vhost_guest, against the existing ioctl-only vhost seed.drivers/vhost/vsock.cgoes from 26 to 49 of 147 PCs,net/vmw_vsock/virtio_transport_common.cfrom 34 to 95 of 284, andnet/vmw_vsock/af_vsock.cfrom 52 to 110 of 421. The comment below has the function list and a 13-minute manager run that hitWARNING in check_mm.Tested:
make descriptions,make executorwith -Werror,pkg/runtest TestParsing,pkg/csourcetests. The generated C reproducer for the seed compiles with gcc (gnu17, gnu23, -m32) and clang. The seed passes end to end on a 6.18 VM:accept4returns,recvfromgets the payload, and the host-initiatedconnectcompletes.