linux-kernel - Re: [PATCH net v2] vsock/test: Fix occasional failure in SOCK_STREAM SHUT

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: <CAGxU2F40O3xDSwA4m6r+O5bWoTJgRXqGfyMiLH_sMij+YmE5aw@mail.gmail.com>
Date: Mon, 26 May 2025 15:55:55 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: Konstantin Shkolnyy <kshk@...ux.ibm.com>
Cc: virtualization@...ts.linux.dev, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, mjrosato@...ux.ibm.com
Subject: Re: [PATCH net v2] vsock/test: Fix occasional failure in SOCK_STREAM
 SHUT_RD test

On Mon, 26 May 2025 at 15:50, Konstantin Shkolnyy <kshk@...ux.ibm.com> wrote:
>
> The test outputs:
> "SOCK_STREAM SHUT_RD...expected send(2) failure, got 1".
>
> It tests that shutdown(fd, SHUT_RD) on one side causes send() to fail on
> the other side. However, sometimes there is a delay in delivery of the
> SHUT_RD command, send() succeeds and the test fails, even though the
> command is properly delivered and send() starts failing several
> milliseconds later.
>
> The delay occurs in the kernel because the used buffer notification
> callback virtio_vsock_rx_done(), called upon receipt of the SHUT_RD
> command, doesn't immediately disable send(). It delegates that to
> a kernel thread (via vsock->rx_work). Sometimes that thread is delayed
> more than the test expects.
>
> Change the test to keep calling send() until it fails or a timeout occurs.
>
> Fixes: b698bd97c5711 ("test/vsock: shutdowned socket test")
> Signed-off-by: Konstantin Shkolnyy <kshk@...ux.ibm.com>
> ---
> Changes in v2:
>  - Move the new function to utils.c.

BTW I think I already fixed the same issue in this series:
https://lore.kernel.org/netdev/20250514141927.159456-1-sgarzare@redhat.com/

Can you check it?

Thanks,
Stefano

>
>  tools/testing/vsock/util.c       | 11 +++++++++++
>  tools/testing/vsock/util.h       |  1 +
>  tools/testing/vsock/vsock_test.c | 14 ++------------
>  3 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
> index de25892f865f..04ac88dc4d3a 100644
> --- a/tools/testing/vsock/util.c
> +++ b/tools/testing/vsock/util.c
> @@ -798,3 +798,14 @@ void enable_so_zerocopy_check(int fd)
>         setsockopt_int_check(fd, SOL_SOCKET, SO_ZEROCOPY, 1,
>                              "setsockopt SO_ZEROCOPY");
>  }
> +
> +void vsock_test_for_send_failure(int fd, int send_flags)
> +{
> +       timeout_begin(TIMEOUT);
> +       while (true) {
> +               if (send(fd, "A", 1, send_flags) == -1)
> +                       return;
> +               timeout_check("expected send(2) failure");
> +       }
> +       timeout_end();
> +}
> diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
> index d1f765ce3eee..58c17cfb63d4 100644
> --- a/tools/testing/vsock/util.h
> +++ b/tools/testing/vsock/util.h
> @@ -79,4 +79,5 @@ void setsockopt_int_check(int fd, int level, int optname, int val,
>  void setsockopt_timeval_check(int fd, int level, int optname,
>                               struct timeval val, char const *errmsg);
>  void enable_so_zerocopy_check(int fd);
> +void vsock_test_for_send_failure(int fd, int send_flags);
>  #endif /* UTIL_H */
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> index 613551132a96..b68a85a6d929 100644
> --- a/tools/testing/vsock/vsock_test.c
> +++ b/tools/testing/vsock/vsock_test.c
> @@ -1060,15 +1060,9 @@ static void sigpipe(int signo)
>
>  static void test_stream_check_sigpipe(int fd)
>  {
> -       ssize_t res;
> -
>         have_sigpipe = 0;
>
> -       res = send(fd, "A", 1, 0);
> -       if (res != -1) {
> -               fprintf(stderr, "expected send(2) failure, got %zi\n", res);
> -               exit(EXIT_FAILURE);
> -       }
> +       vsock_test_for_send_failure(fd, 0);
>
>         if (!have_sigpipe) {
>                 fprintf(stderr, "SIGPIPE expected\n");
> @@ -1077,11 +1071,7 @@ static void test_stream_check_sigpipe(int fd)
>
>         have_sigpipe = 0;
>
> -       res = send(fd, "A", 1, MSG_NOSIGNAL);
> -       if (res != -1) {
> -               fprintf(stderr, "expected send(2) failure, got %zi\n", res);
> -               exit(EXIT_FAILURE);
> -       }
> +       vsock_test_for_send_failure(fd, MSG_NOSIGNAL);
>
>         if (have_sigpipe) {
>                 fprintf(stderr, "SIGPIPE not expected\n");
> --
> 2.34.1
>