On 5/26/25 10:25, Stefano Garzarella wrote:
> On Fri, May 23, 2025 at 12:31:16AM +0200, Michal Luczaj wrote:
>> Increase the coverage of test for UAF due to socket unbinding, and losing
>> transport in general. It's a follow up to commit 301a62dfb0d0 ("vsock/test:
>> Add test for UAF due to socket unbinding") and discussion in [1].
>>
>> The idea remains the same: take an unconnected stream socket with a
>> transport assigned and then attempt to switch the transport by trying (and
>> failing) to connect to some other CID. Now do this iterating over all the
>> well known CIDs (plus one).
>>
>> Note that having only a virtio transport loaded (without vhost_vsock) is
>> unsupported; test will always pass. Depending on transports available, a
>
> Do you think it might make sense to print a warning if we are in this
> case, perhaps by parsing /proc/modules and looking at vsock
> dependencies?
That'd nice, but would parsing /proc/modules work if a transport is
compiled-in (not a module)?
>> +static bool test_stream_transport_uaf(int cid)
>> {
>> + struct sockaddr_vm addr = {
>> + .svm_family = AF_VSOCK,
>> + .svm_cid = cid,
>> + .svm_port = VMADDR_PORT_ANY
>> + };
>> int sockets[MAX_PORT_RETRIES];
>> - struct sockaddr_vm addr;
>> - int fd, i, alen;
>> + socklen_t alen;
>> + int fd, i, c;
>>
>> - fd = vsock_bind(VMADDR_CID_ANY, VMADDR_PORT_ANY, SOCK_STREAM);
>> + fd = socket(AF_VSOCK, SOCK_STREAM, 0);
>> + if (fd < 0) {
>> + perror("socket");
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> + if (bind(fd, (struct sockaddr *)&addr, sizeof(addr))) {
>> + if (errno != EADDRNOTAVAIL) {
>> + perror("Unexpected bind() errno");
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> + close(fd);
>> + return false;
>
> Perhaps we should mention in the commit or in a comment above this
> function, what we return and why we can expect EADDRNOTAVAIL.
Something like
/* Probe for a transport by attempting a local CID bind. Unavailable
* transport (or more specifically: an unsupported transport/CID
* combination) results in EADDRNOTAVAIL, other errnos are fatal.
*/
?
And I've just realized feeding VMADDR_CID_HYPERVISOR to bind() doesn't make
sense at all. Will fix.
> What about adding a vsock_bind_try() in util.c that can fail returning
> errno, so we can share most of the code with vsock_bind()?
Ah, yes, good idea.
>> +static void test_stream_transport_uaf_client(const struct test_opts *opts)
>> +{
>> + bool tested = false;
>> + int cid;
>> +
>> + for (cid = VMADDR_CID_HYPERVISOR; cid <= VMADDR_CID_HOST + 1; ++cid)
>
>> + tested |= test_stream_transport_uaf(cid);
>> +
>> + if (!tested)
>> + fprintf(stderr, "No transport tested\n");
>> +
>> control_writeln("DONE");
>
> While we're at it, I think we can remove this message, looking at
> run_tests() in util.c, we already have a barrier.
Ok, sure. Note that console output gets slightly de-synchronised: server
will immediately print next test's prompt and wait there.
Thanks,
Michal
