]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
vsock/test: fix test for null ptr deref when transport changes
authorStefano Garzarella <sgarzare@redhat.com>
Tue, 8 Jul 2025 11:17:01 +0000 (13:17 +0200)
committerJakub Kicinski <kuba@kernel.org>
Thu, 10 Jul 2025 02:33:07 +0000 (19:33 -0700)
In test_stream_transport_change_client(), the client sends CONTROL_CONTINUE
on each iteration, even when connect() is unsuccessful. This causes a flood
of control messages in the server that hangs around for more than 10
seconds after the test finishes, triggering several timeouts and causing
subsequent tests to fail. This was discovered in testing a newly proposed
test that failed in this way on the client side:
    ...
    33 - SOCK_STREAM transport change null-ptr-deref...ok
    34 - SOCK_STREAM ioctl(SIOCINQ) functionality...recv timed out

The CONTROL_CONTINUE message is used only to tell to the server to call
accept() to consume successful connections, so that subsequent connect()
will not fail for finding the queue full.

Send CONTROL_CONTINUE message only when the connect() has succeeded, or
found the queue full. Note that the second connect() can also succeed if
the first one was interrupted after sending the request.

Fixes: 3a764d93385c ("vsock/test: Add test for null ptr deref when transport changes")
Cc: leonardi@redhat.com
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
Link: https://patch.msgid.link/20250708111701.129585-1-sgarzare@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
tools/testing/vsock/vsock_test.c

index a66d2360133dd0e36940a5907679aeacc8af7714..d4517386e551e3e3908aabc14bc4608695e9c45f 100644 (file)
@@ -2006,6 +2006,7 @@ static void test_stream_transport_change_client(const struct test_opts *opts)
                        .svm_cid = opts->peer_cid,
                        .svm_port = opts->peer_port,
                };
+               bool send_control = false;
                int s;
 
                s = socket(AF_VSOCK, SOCK_STREAM, 0);
@@ -2026,19 +2027,29 @@ static void test_stream_transport_change_client(const struct test_opts *opts)
                        exit(EXIT_FAILURE);
                }
 
+               /* Notify the server if the connect() is successful or the
+                * receiver connection queue is full, so it will do accept()
+                * to drain it.
+                */
+               if (!ret || errno == ECONNRESET)
+                       send_control = true;
+
                /* Set CID to 0 cause a transport change. */
                sa.svm_cid = 0;
 
-               /* Ignore return value since it can fail or not.
-                * If the previous connect is interrupted while the
-                * connection request is already sent, the second
+               /* There is a case where this will not fail:
+                * if the previous connect() is interrupted while the
+                * connection request is already sent, this second
                 * connect() will wait for the response.
                 */
-               connect(s, (struct sockaddr *)&sa, sizeof(sa));
+               ret = connect(s, (struct sockaddr *)&sa, sizeof(sa));
+               if (!ret || errno == ECONNRESET)
+                       send_control = true;
 
                close(s);
 
-               control_writeulong(CONTROL_CONTINUE);
+               if (send_control)
+                       control_writeulong(CONTROL_CONTINUE);
 
        } while (current_nsec() < tout);