netdev - Re: [PATCH v5 3/3] vsock/test: verify socket options after setting them

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: <20ebaf65-503f-40a3-b8f3-ac1e649e2fac@linux.ibm.com>
Date: Tue, 12 Nov 2024 09:18:48 -0600
From: Konstantin Shkolnyy <kshk@...ux.ibm.com>
To: Stefano Garzarella <sgarzare@...hat.com>
Cc: virtualization@...ts.linux.dev, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, mjrosato@...ux.ibm.com
Subject: Re: [PATCH v5 3/3] vsock/test: verify socket options after setting
 them

On 11/12/2024 02:58, Stefano Garzarella wrote:
> On Thu, Nov 07, 2024 at 07:17:26PM -0600, Konstantin Shkolnyy wrote:
>> Replace setsockopt() calls with calls to functions that follow
>> setsockopt() with getsockopt() and check that the returned value and its
>> size are the same as have been set.
>>
>> Signed-off-by: Konstantin Shkolnyy <kshk@...ux.ibm.com>
>> ---
>> tools/testing/vsock/Makefile              |   8 +-
>> tools/testing/vsock/control.c             |   8 +-
>> tools/testing/vsock/msg_zerocopy_common.c |   8 +-
>> tools/testing/vsock/util_socket.c         | 149 ++++++++++++++++++++++
>> tools/testing/vsock/util_socket.h         |  19 +++
>> tools/testing/vsock/vsock_perf.c          |  24 ++--
>> tools/testing/vsock/vsock_test.c          |  40 +++---
>> 7 files changed, 208 insertions(+), 48 deletions(-)
>> create mode 100644 tools/testing/vsock/util_socket.c
>> create mode 100644 tools/testing/vsock/util_socket.h
>>
>> diff --git a/tools/testing/vsock/Makefile b/tools/testing/vsock/Makefile
>> index 6e0b4e95e230..1ec0b3a67aa4 100644
>> --- a/tools/testing/vsock/Makefile
>> +++ b/tools/testing/vsock/Makefile
>> @@ -1,12 +1,12 @@
>> # SPDX-License-Identifier: GPL-2.0-only
>> all: test vsock_perf
>> test: vsock_test vsock_diag_test vsock_uring_test
>> -vsock_test: vsock_test.o vsock_test_zerocopy.o timeout.o control.o 
>> util.o msg_zerocopy_common.o
>> -vsock_diag_test: vsock_diag_test.o timeout.o control.o util.o
>> -vsock_perf: vsock_perf.o msg_zerocopy_common.o
>> +vsock_test: vsock_test.o vsock_test_zerocopy.o timeout.o control.o 
>> util.o msg_zerocopy_common.o util_socket.o
>> +vsock_diag_test: vsock_diag_test.o timeout.o control.o util.o 
>> util_socket.o
>> +vsock_perf: vsock_perf.o msg_zerocopy_common.o util_socket.o
> 
> I would add the new functions to check setsockopt in util.c
> 
> vsock_perf is more of a tool to measure performance than a test, so
> we can avoid calling these checks there, tests should cover all
> cases regardless of vsock_perf.

The problem is that vsock_perf calls enable_so_zerocopy() which has to
call the new setsockopt_int_check() because it's also called by 
vsock_test. Do you prefer to give vsock_perf its own version of
enable_so_zerocopy() which doesn't call setsockopt_int_check()?