| Message ID | 20250204-vsock-linger-nullderef-v1-2-6eb1760fa93e@rbox.co (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | vsock: null-ptr-deref when SO_LINGER enabled | expand |
On Tue, Feb 04, 2025 at 01:29:53AM +0100, Michal Luczaj wrote: >Explicitly close() a TCP_ESTABLISHED (connectible) socket with SO_LINGER >enabled. May trigger a null pointer dereference. > >Signed-off-by: Michal Luczaj <mhal@rbox.co> >--- > tools/testing/vsock/vsock_test.c | 41 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > >diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c >index dfff8b288265f96b602cb1bfa0e6dce02f114222..d0f6d253ac72d08a957cb81a3c38fcc72bec5a53 100644 >--- a/tools/testing/vsock/vsock_test.c >+++ b/tools/testing/vsock/vsock_test.c >@@ -1788,6 +1788,42 @@ static void test_stream_connect_retry_server(const struct test_opts *opts) > close(fd); > } > >+static void test_stream_linger_client(const struct test_opts *opts) >+{ >+ struct linger optval = { >+ .l_onoff = 1, >+ .l_linger = 1 >+ }; >+ int fd; >+ >+ fd = vsock_stream_connect(opts->peer_cid, opts->peer_port); >+ if (fd < 0) { >+ perror("connect"); >+ exit(EXIT_FAILURE); >+ } >+ >+ if (setsockopt(fd, SOL_SOCKET, SO_LINGER, &optval, sizeof(optval))) { >+ perror("setsockopt(SO_LINGER)"); >+ exit(EXIT_FAILURE); >+ } Since we are testing SO_LINGER, will also be nice to check if it's working properly, since one of the fixes proposed could break it. To test, we may set a small SO_VM_SOCKETS_BUFFER_SIZE on the receive side and try to send more than that value, obviously without reading anything into the receiver, and check that close() here, returns after the timeout we set in .l_linger. Of course, we could also add it later, but while we're at it, it crossed my mind. WDYT? Thanks, Stefano >+ >+ close(fd); >+} >+ >+static void test_stream_linger_server(const struct test_opts *opts) >+{ >+ int fd; >+ >+ fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL); >+ if (fd < 0) { >+ perror("accept"); >+ exit(EXIT_FAILURE); >+ } >+ >+ vsock_wait_remote_close(fd); >+ close(fd); >+} >+ > static struct test_case test_cases[] = { > { > .name = "SOCK_STREAM connection reset", >@@ -1943,6 +1979,11 @@ static struct test_case test_cases[] = { > .run_client = test_stream_connect_retry_client, > .run_server = test_stream_connect_retry_server, > }, >+ { >+ .name = "SOCK_STREAM SO_LINGER null-ptr-deref", >+ .run_client = test_stream_linger_client, >+ .run_server = test_stream_linger_server, >+ }, > {}, > }; > > >-- >2.48.1 >
On 2/4/25 11:48, Stefano Garzarella wrote: > On Tue, Feb 04, 2025 at 01:29:53AM +0100, Michal Luczaj wrote: >> ... >> +static void test_stream_linger_client(const struct test_opts *opts) >> +{ >> + struct linger optval = { >> + .l_onoff = 1, >> + .l_linger = 1 >> + }; >> + int fd; >> + >> + fd = vsock_stream_connect(opts->peer_cid, opts->peer_port); >> + if (fd < 0) { >> + perror("connect"); >> + exit(EXIT_FAILURE); >> + } >> + >> + if (setsockopt(fd, SOL_SOCKET, SO_LINGER, &optval, sizeof(optval))) { >> + perror("setsockopt(SO_LINGER)"); >> + exit(EXIT_FAILURE); >> + } > > Since we are testing SO_LINGER, will also be nice to check if it's > working properly, since one of the fixes proposed could break it. > > To test, we may set a small SO_VM_SOCKETS_BUFFER_SIZE on the receive > side and try to send more than that value, obviously without reading > anything into the receiver, and check that close() here, returns after > the timeout we set in .l_linger. I may be doing something wrong, but (at least for loopback transport) it seems that close() lingers until data is received, not sent (without even touching SO_VM_SOCKETS_BUFFER_SIZE). ``` import struct, fcntl, termios, time from socket import * def linger(s, timeout): if s.family == AF_VSOCK: s.setsockopt(SOL_SOCKET, SO_LINGER, (timeout<<32) | 1) elif s.family == AF_INET: s.setsockopt(SOL_SOCKET, SO_LINGER, struct.pack('ii', 1, timeout)) else: assert False def unsent(s): SIOCOUTQ = termios.TIOCOUTQ return struct.unpack('I', fcntl.ioctl(s, SIOCOUTQ, bytes(4)))[0] def check_lingering(family, addr): lis = socket(family, SOCK_STREAM) lis.bind(addr) lis.listen() s = socket(family, SOCK_STREAM) linger(s, 2) s.connect(lis.getsockname()) for _ in range(1, 1<<8): s.send(b'x') while unsent(s) != 0: pass print("closing...") ts = time.time() s.close() print(f"done in %ds" % (time.time() - ts)) check_lingering(AF_INET, ('127.0.0.1', 1234)) check_lingering(AF_VSOCK, (1, 1234)) # VMADDR_CID_LOCAL ``` Gives me: closing... done in 0s closing... done in 2s
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c index dfff8b288265f96b602cb1bfa0e6dce02f114222..d0f6d253ac72d08a957cb81a3c38fcc72bec5a53 100644 --- a/tools/testing/vsock/vsock_test.c +++ b/tools/testing/vsock/vsock_test.c @@ -1788,6 +1788,42 @@ static void test_stream_connect_retry_server(const struct test_opts *opts) close(fd); } +static void test_stream_linger_client(const struct test_opts *opts) +{ + struct linger optval = { + .l_onoff = 1, + .l_linger = 1 + }; + int fd; + + fd = vsock_stream_connect(opts->peer_cid, opts->peer_port); + if (fd < 0) { + perror("connect"); + exit(EXIT_FAILURE); + } + + if (setsockopt(fd, SOL_SOCKET, SO_LINGER, &optval, sizeof(optval))) { + perror("setsockopt(SO_LINGER)"); + exit(EXIT_FAILURE); + } + + close(fd); +} + +static void test_stream_linger_server(const struct test_opts *opts) +{ + int fd; + + fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL); + if (fd < 0) { + perror("accept"); + exit(EXIT_FAILURE); + } + + vsock_wait_remote_close(fd); + close(fd); +} + static struct test_case test_cases[] = { { .name = "SOCK_STREAM connection reset", @@ -1943,6 +1979,11 @@ static struct test_case test_cases[] = { .run_client = test_stream_connect_retry_client, .run_server = test_stream_connect_retry_server, }, + { + .name = "SOCK_STREAM SO_LINGER null-ptr-deref", + .run_client = test_stream_linger_client, + .run_server = test_stream_linger_server, + }, {}, };
Explicitly close() a TCP_ESTABLISHED (connectible) socket with SO_LINGER enabled. May trigger a null pointer dereference. Signed-off-by: Michal Luczaj <mhal@rbox.co> --- tools/testing/vsock/vsock_test.c | 41 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)