On Mon, May 26, 2025 at 10:44:05PM +0200, Michal Luczaj wrote:
On 5/26/25 16:39, Stefano Garzarella wrote:
On Mon, May 26, 2025 at 02:51:18PM +0200, Michal Luczaj wrote:
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)?
Good point, I think not, maybe we can see something under /sys/module,
though, I would say let's do best effort without going crazy ;-)
Grepping through /proc/kallsyms would do the trick. Is this still a sane
ground?
It also depends on a config right?
I see CONFIG_KALLSYMS, CONFIG_KALLSYMS_ALL, etc. but yeah, if it's
enabled, it should work for both modules and built-in transports.
And I've just realized feeding VMADDR_CID_HYPERVISOR to bind() doesn't make
sense at all. Will fix.
Yeah, we don't support it for now and maybe it makes sense only in the
VMM code (e.g. QEMU), but it's a test, so if you want to leave to stress
it more, I don't think it's a big issue.
All right, I'll keep it then. Fails quickly and politely anyway.
+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.
I see, however I don't have a strong opinion, you can leave it that way
if you prefer.
How about adding a sync point to run_tests()? E.g.
Yep, why not, of course in another series :-)
And if you like, you can remove that specific sync point in that series
and check also other tests, but I think we have only that one.
Thanks,
Stefano
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index de25892f865f..79a02b52dc19 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -451,6 +451,9 @@ void run_tests(const struct test_case *test_cases,
run(opts);
printf("ok\n");
+
+ control_writeln("RUN_TESTS_SYNC");
+ control_expectln("RUN_TESTS_SYNC");
}
}
