Re: [PATCH net-next] vsock/test: Cover more CIDs in transport_uaf test

[Michal Luczaj <mhal@rbox.co>]

On 5/27/25 10:41, Stefano Garzarella wrote:
> 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:
>>>>>> 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.

FWIW, tools/testing/selftests/net/config has CONFIG_KALLSYMS=y, which
is enough for being able to check symbols like virtio_transport and
vhost_transport.

Administrative query: while net-next is closed, am I supposed to mark this
series as "RFC" and post v2 for a review as usual, or is it better to just
hold off until net-next opens?

>>>>>> +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.

OK, I'll leave that for later.

Thanks,
Michal