linux-kernel - Re: [PATCH net-next] vsock/test: Cover more CIDs in transport

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7zqv5toj2qjucy7fvaebbpwj6pth53uunsbapwhgrhwbr5pq5t@gp7h6klhr5sj>
Date: Wed, 28 May 2025 11:08:37 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: Michal Luczaj <mhal@...x.co>
Cc: virtualization@...ts.linux.dev, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] vsock/test: Cover more CIDs in transport_uaf
 test

On Wed, May 28, 2025 at 10:58:28AM +0200, Michal Luczaj wrote:
>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.

Ok, I see, so let's go in that direction.

>
>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?

Whichever you prefer, if you are uncertain about the next version and 
want to speed things up with a review while waiting, then go with RFC, 
but if you think all comments are resolved and the next version is ready 
to be merged, wait for the reopening.
Thanks for asking!

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

Yep, feel free to discard my suggestion, we can fix it later.

Thanks,
Stefano