Skip to content

Commit 6e6c62b

Browse files
kuba-mookuba-moo
committed
Merge branch 'vsock-transport-reassignment-and-error-handling-issues'
Michal Luczaj says: ==================== vsock: Transport reassignment and error handling issues Series deals with two issues: - socket reference count imbalance due to an unforgiving transport release (triggered by transport reassignment); - unintentional API feature, a failing connect() making the socket impossible to use for any subsequent connect() attempts. v2: https://lore.kernel.org/[email protected] v1: https://lore.kernel.org/[email protected] ==================== Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
2 parents 9e6c4e6 + 4695f64 commit 6e6c62b

File tree

4 files changed

+153
-72
lines changed

4 files changed

+153
-72
lines changed

‎net/vmw_vsock/af_vsock.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,10 @@ EXPORT_SYMBOL_GPL(vsock_find_connected_socket);
337337

338338
void vsock_remove_sock(struct vsock_sock *vsk)
339339
{
340-
vsock_remove_bound(vsk);
340+
/* Transport reassignment must not remove the binding. */
341+
if (sock_flag(sk_vsock(vsk), SOCK_DEAD))
342+
vsock_remove_bound(vsk);
343+
341344
vsock_remove_connected(vsk);
342345
}
343346
EXPORT_SYMBOL_GPL(vsock_remove_sock);
@@ -821,12 +824,13 @@ static void __vsock_release(struct sock *sk, int level)
821824
*/
822825
lock_sock_nested(sk, level);
823826

827+
sock_orphan(sk);
828+
824829
if (vsk->transport)
825830
vsk->transport->release(vsk);
826831
else if (sock_type_connectible(sk->sk_type))
827832
vsock_remove_sock(vsk);
828833

829-
sock_orphan(sk);
830834
sk->sk_shutdown = SHUTDOWN_MASK;
831835

832836
skb_queue_purge(&sk->sk_receive_queue);
@@ -1519,6 +1523,11 @@ static int vsock_connect(struct socket *sock, struct sockaddr *addr,
15191523
if (err < 0)
15201524
goto out;
15211525

1526+
/* sk_err might have been set as a result of an earlier
1527+
* (failed) connect attempt.
1528+
*/
1529+
sk->sk_err = 0;
1530+
15221531
/* Mark sock as connecting and set the error code to in
15231532
* progress in case this is a non-blocking connect.
15241533
*/

‎tools/testing/vsock/util.c

Lines changed: 34 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -96,41 +96,57 @@ void vsock_wait_remote_close(int fd)
9696
close(epollfd);
9797
}
9898

99-
/* Bind to <bind_port>, connect to <cid, port> and return the file descriptor. */
100-
int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_port, int type)
99+
/* Create socket <type>, bind to <cid, port> and return the file descriptor. */
100+
int vsock_bind(unsigned int cid, unsigned int port, int type)
101101
{
102-
struct sockaddr_vm sa_client = {
103-
.svm_family = AF_VSOCK,
104-
.svm_cid = VMADDR_CID_ANY,
105-
.svm_port = bind_port,
106-
};
107-
struct sockaddr_vm sa_server = {
102+
struct sockaddr_vm sa = {
108103
.svm_family = AF_VSOCK,
109104
.svm_cid = cid,
110105
.svm_port = port,
111106
};
107+
int fd;
112108

113-
int client_fd, ret;
114-
115-
client_fd = socket(AF_VSOCK, type, 0);
116-
if (client_fd < 0) {
109+
fd = socket(AF_VSOCK, type, 0);
110+
if (fd < 0) {
117111
perror("socket");
118112
exit(EXIT_FAILURE);
119113
}
120114

121-
if (bind(client_fd, (struct sockaddr *)&sa_client, sizeof(sa_client))) {
115+
if (bind(fd, (struct sockaddr *)&sa, sizeof(sa))) {
122116
perror("bind");
123117
exit(EXIT_FAILURE);
124118
}
125119

120+
return fd;
121+
}
122+
123+
int vsock_connect_fd(int fd, unsigned int cid, unsigned int port)
124+
{
125+
struct sockaddr_vm sa = {
126+
.svm_family = AF_VSOCK,
127+
.svm_cid = cid,
128+
.svm_port = port,
129+
};
130+
int ret;
131+
126132
timeout_begin(TIMEOUT);
127133
do {
128-
ret = connect(client_fd, (struct sockaddr *)&sa_server, sizeof(sa_server));
134+
ret = connect(fd, (struct sockaddr *)&sa, sizeof(sa));
129135
timeout_check("connect");
130136
} while (ret < 0 && errno == EINTR);
131137
timeout_end();
132138

133-
if (ret < 0) {
139+
return ret;
140+
}
141+
142+
/* Bind to <bind_port>, connect to <cid, port> and return the file descriptor. */
143+
int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_port, int type)
144+
{
145+
int client_fd;
146+
147+
client_fd = vsock_bind(VMADDR_CID_ANY, bind_port, type);
148+
149+
if (vsock_connect_fd(client_fd, cid, port)) {
134150
perror("connect");
135151
exit(EXIT_FAILURE);
136152
}
@@ -141,17 +157,6 @@ int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_po
141157
/* Connect to <cid, port> and return the file descriptor. */
142158
int vsock_connect(unsigned int cid, unsigned int port, int type)
143159
{
144-
union {
145-
struct sockaddr sa;
146-
struct sockaddr_vm svm;
147-
} addr = {
148-
.svm = {
149-
.svm_family = AF_VSOCK,
150-
.svm_port = port,
151-
.svm_cid = cid,
152-
},
153-
};
154-
int ret;
155160
int fd;
156161

157162
control_expectln("LISTENING");
@@ -162,20 +167,14 @@ int vsock_connect(unsigned int cid, unsigned int port, int type)
162167
exit(EXIT_FAILURE);
163168
}
164169

165-
timeout_begin(TIMEOUT);
166-
do {
167-
ret = connect(fd, &addr.sa, sizeof(addr.svm));
168-
timeout_check("connect");
169-
} while (ret < 0 && errno == EINTR);
170-
timeout_end();
171-
172-
if (ret < 0) {
170+
if (vsock_connect_fd(fd, cid, port)) {
173171
int old_errno = errno;
174172

175173
close(fd);
176174
fd = -1;
177175
errno = old_errno;
178176
}
177+
179178
return fd;
180179
}
181180

@@ -192,28 +191,9 @@ int vsock_seqpacket_connect(unsigned int cid, unsigned int port)
192191
/* Listen on <cid, port> and return the file descriptor. */
193192
static int vsock_listen(unsigned int cid, unsigned int port, int type)
194193
{
195-
union {
196-
struct sockaddr sa;
197-
struct sockaddr_vm svm;
198-
} addr = {
199-
.svm = {
200-
.svm_family = AF_VSOCK,
201-
.svm_port = port,
202-
.svm_cid = cid,
203-
},
204-
};
205194
int fd;
206195

207-
fd = socket(AF_VSOCK, type, 0);
208-
if (fd < 0) {
209-
perror("socket");
210-
exit(EXIT_FAILURE);
211-
}
212-
213-
if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
214-
perror("bind");
215-
exit(EXIT_FAILURE);
216-
}
196+
fd = vsock_bind(cid, port, type);
217197

218198
if (listen(fd, 1) < 0) {
219199
perror("listen");

‎tools/testing/vsock/util.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,12 @@ struct test_case {
3939
void init_signals(void);
4040
unsigned int parse_cid(const char *str);
4141
unsigned int parse_port(const char *str);
42+
int vsock_connect_fd(int fd, unsigned int cid, unsigned int port);
4243
int vsock_connect(unsigned int cid, unsigned int port, int type);
4344
int vsock_accept(unsigned int cid, unsigned int port,
4445
struct sockaddr_vm *clientaddrp, int type);
4546
int vsock_stream_connect(unsigned int cid, unsigned int port);
47+
int vsock_bind(unsigned int cid, unsigned int port, int type);
4648
int vsock_bind_connect(unsigned int cid, unsigned int port,
4749
unsigned int bind_port, int type);
4850
int vsock_seqpacket_connect(unsigned int cid, unsigned int port);

‎tools/testing/vsock/vsock_test.c

Lines changed: 106 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -113,24 +113,9 @@ static void test_stream_bind_only_client(const struct test_opts *opts)
113113

114114
static void test_stream_bind_only_server(const struct test_opts *opts)
115115
{
116-
union {
117-
struct sockaddr sa;
118-
struct sockaddr_vm svm;
119-
} addr = {
120-
.svm = {
121-
.svm_family = AF_VSOCK,
122-
.svm_port = opts->peer_port,
123-
.svm_cid = VMADDR_CID_ANY,
124-
},
125-
};
126116
int fd;
127117

128-
fd = socket(AF_VSOCK, SOCK_STREAM, 0);
129-
130-
if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
131-
perror("bind");
132-
exit(EXIT_FAILURE);
133-
}
118+
fd = vsock_bind(VMADDR_CID_ANY, opts->peer_port, SOCK_STREAM);
134119

135120
/* Notify the client that the server is ready */
136121
control_writeln("BIND");
@@ -1708,6 +1693,101 @@ static void test_stream_msgzcopy_leak_zcskb_server(const struct test_opts *opts)
17081693
close(fd);
17091694
}
17101695

1696+
#define MAX_PORT_RETRIES 24 /* net/vmw_vsock/af_vsock.c */
1697+
1698+
/* Test attempts to trigger a transport release for an unbound socket. This can
1699+
* lead to a reference count mishandling.
1700+
*/
1701+
static void test_stream_transport_uaf_client(const struct test_opts *opts)
1702+
{
1703+
int sockets[MAX_PORT_RETRIES];
1704+
struct sockaddr_vm addr;
1705+
int fd, i, alen;
1706+
1707+
fd = vsock_bind(VMADDR_CID_ANY, VMADDR_PORT_ANY, SOCK_STREAM);
1708+
1709+
alen = sizeof(addr);
1710+
if (getsockname(fd, (struct sockaddr *)&addr, &alen)) {
1711+
perror("getsockname");
1712+
exit(EXIT_FAILURE);
1713+
}
1714+
1715+
for (i = 0; i < MAX_PORT_RETRIES; ++i)
1716+
sockets[i] = vsock_bind(VMADDR_CID_ANY, ++addr.svm_port,
1717+
SOCK_STREAM);
1718+
1719+
close(fd);
1720+
fd = socket(AF_VSOCK, SOCK_STREAM, 0);
1721+
if (fd < 0) {
1722+
perror("socket");
1723+
exit(EXIT_FAILURE);
1724+
}
1725+
1726+
if (!vsock_connect_fd(fd, addr.svm_cid, addr.svm_port)) {
1727+
perror("Unexpected connect() #1 success");
1728+
exit(EXIT_FAILURE);
1729+
}
1730+
1731+
/* Vulnerable system may crash now. */
1732+
if (!vsock_connect_fd(fd, VMADDR_CID_HOST, VMADDR_PORT_ANY)) {
1733+
perror("Unexpected connect() #2 success");
1734+
exit(EXIT_FAILURE);
1735+
}
1736+
1737+
close(fd);
1738+
while (i--)
1739+
close(sockets[i]);
1740+
1741+
control_writeln("DONE");
1742+
}
1743+
1744+
static void test_stream_transport_uaf_server(const struct test_opts *opts)
1745+
{
1746+
control_expectln("DONE");
1747+
}
1748+
1749+
static void test_stream_connect_retry_client(const struct test_opts *opts)
1750+
{
1751+
int fd;
1752+
1753+
fd = socket(AF_VSOCK, SOCK_STREAM, 0);
1754+
if (fd < 0) {
1755+
perror("socket");
1756+
exit(EXIT_FAILURE);
1757+
}
1758+
1759+
if (!vsock_connect_fd(fd, opts->peer_cid, opts->peer_port)) {
1760+
fprintf(stderr, "Unexpected connect() #1 success\n");
1761+
exit(EXIT_FAILURE);
1762+
}
1763+
1764+
control_writeln("LISTEN");
1765+
control_expectln("LISTENING");
1766+
1767+
if (vsock_connect_fd(fd, opts->peer_cid, opts->peer_port)) {
1768+
perror("connect() #2");
1769+
exit(EXIT_FAILURE);
1770+
}
1771+
1772+
close(fd);
1773+
}
1774+
1775+
static void test_stream_connect_retry_server(const struct test_opts *opts)
1776+
{
1777+
int fd;
1778+
1779+
control_expectln("LISTEN");
1780+
1781+
fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
1782+
if (fd < 0) {
1783+
perror("accept");
1784+
exit(EXIT_FAILURE);
1785+
}
1786+
1787+
vsock_wait_remote_close(fd);
1788+
close(fd);
1789+
}
1790+
17111791
static struct test_case test_cases[] = {
17121792
{
17131793
.name = "SOCK_STREAM connection reset",
@@ -1853,6 +1933,16 @@ static struct test_case test_cases[] = {
18531933
.run_client = test_stream_msgzcopy_leak_zcskb_client,
18541934
.run_server = test_stream_msgzcopy_leak_zcskb_server,
18551935
},
1936+
{
1937+
.name = "SOCK_STREAM transport release use-after-free",
1938+
.run_client = test_stream_transport_uaf_client,
1939+
.run_server = test_stream_transport_uaf_server,
1940+
},
1941+
{
1942+
.name = "SOCK_STREAM retry failed connect()",
1943+
.run_client = test_stream_connect_retry_client,
1944+
.run_server = test_stream_connect_retry_server,
1945+
},
18561946
{},
18571947
};
18581948

0 commit comments

Comments
 (0)