Skip to content

Commit 97b9432

Browse files
davem330davem330
committed
Merge branch 'vsock-fixes'
Filippo Storniolo says: ==================== vsock: fix server prevents clients from reconnecting This patch series introduce fix and tests for the following vsock bug: If the same remote peer, using the same port, tries to connect to a server on a listening port more than once, the server will reject the connection, causing a "connection reset by peer" error on the remote peer. This is due to the presence of a dangling socket from a previous connection in both the connected and bound socket lists. The inconsistency of the above lists only occurs when the remote peer disconnects and the server remains active. This bug does not occur when the server socket is closed. More details on the first patch changelog. The remaining patches are refactoring and test. ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents 7425627 + d80f63f commit 97b9432

4 files changed

Lines changed: 139 additions & 17 deletions

File tree

‎net/vmw_vsock/virtio_transport_common.c‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,11 +1369,17 @@ virtio_transport_recv_connected(struct sock *sk,
13691369
vsk->peer_shutdown |= RCV_SHUTDOWN;
13701370
if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SHUTDOWN_SEND)
13711371
vsk->peer_shutdown |= SEND_SHUTDOWN;
1372-
if (vsk->peer_shutdown == SHUTDOWN_MASK &&
1373-
vsock_stream_has_data(vsk) <= 0 &&
1374-
!sock_flag(sk, SOCK_DONE)) {
1375-
(void)virtio_transport_reset(vsk, NULL);
1376-
virtio_transport_do_close(vsk, true);
1372+
if (vsk->peer_shutdown == SHUTDOWN_MASK) {
1373+
if (vsock_stream_has_data(vsk) <= 0 && !sock_flag(sk, SOCK_DONE)) {
1374+
(void)virtio_transport_reset(vsk, NULL);
1375+
virtio_transport_do_close(vsk, true);
1376+
}
1377+
/* Remove this socket anyway because the remote peer sent
1378+
* the shutdown. This way a new connection will succeed
1379+
* if the remote peer uses the same source port,
1380+
* even if the old socket is still unreleased, but now disconnected.
1381+
*/
1382+
vsock_remove_sock(vsk);
13771383
}
13781384
if (le32_to_cpu(virtio_vsock_hdr(skb)->flags))
13791385
sk->sk_state_change(sk);

‎tools/testing/vsock/util.c‎

Lines changed: 75 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,48 @@ void vsock_wait_remote_close(int fd)
8585
close(epollfd);
8686
}
8787

88+
/* Bind to <bind_port>, connect to <cid, port> and return the file descriptor. */
89+
int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_port, int type)
90+
{
91+
struct sockaddr_vm sa_client = {
92+
.svm_family = AF_VSOCK,
93+
.svm_cid = VMADDR_CID_ANY,
94+
.svm_port = bind_port,
95+
};
96+
struct sockaddr_vm sa_server = {
97+
.svm_family = AF_VSOCK,
98+
.svm_cid = cid,
99+
.svm_port = port,
100+
};
101+
102+
int client_fd, ret;
103+
104+
client_fd = socket(AF_VSOCK, type, 0);
105+
if (client_fd < 0) {
106+
perror("socket");
107+
exit(EXIT_FAILURE);
108+
}
109+
110+
if (bind(client_fd, (struct sockaddr *)&sa_client, sizeof(sa_client))) {
111+
perror("bind");
112+
exit(EXIT_FAILURE);
113+
}
114+
115+
timeout_begin(TIMEOUT);
116+
do {
117+
ret = connect(client_fd, (struct sockaddr *)&sa_server, sizeof(sa_server));
118+
timeout_check("connect");
119+
} while (ret < 0 && errno == EINTR);
120+
timeout_end();
121+
122+
if (ret < 0) {
123+
perror("connect");
124+
exit(EXIT_FAILURE);
125+
}
126+
127+
return client_fd;
128+
}
129+
88130
/* Connect to <cid, port> and return the file descriptor. */
89131
static int vsock_connect(unsigned int cid, unsigned int port, int type)
90132
{
@@ -104,6 +146,10 @@ static int vsock_connect(unsigned int cid, unsigned int port, int type)
104146
control_expectln("LISTENING");
105147

106148
fd = socket(AF_VSOCK, type, 0);
149+
if (fd < 0) {
150+
perror("socket");
151+
exit(EXIT_FAILURE);
152+
}
107153

108154
timeout_begin(TIMEOUT);
109155
do {
@@ -132,11 +178,8 @@ int vsock_seqpacket_connect(unsigned int cid, unsigned int port)
132178
return vsock_connect(cid, port, SOCK_SEQPACKET);
133179
}
134180

135-
/* Listen on <cid, port> and return the first incoming connection. The remote
136-
* address is stored to clientaddrp. clientaddrp may be NULL.
137-
*/
138-
static int vsock_accept(unsigned int cid, unsigned int port,
139-
struct sockaddr_vm *clientaddrp, int type)
181+
/* Listen on <cid, port> and return the file descriptor. */
182+
static int vsock_listen(unsigned int cid, unsigned int port, int type)
140183
{
141184
union {
142185
struct sockaddr sa;
@@ -148,16 +191,13 @@ static int vsock_accept(unsigned int cid, unsigned int port,
148191
.svm_cid = cid,
149192
},
150193
};
151-
union {
152-
struct sockaddr sa;
153-
struct sockaddr_vm svm;
154-
} clientaddr;
155-
socklen_t clientaddr_len = sizeof(clientaddr.svm);
156194
int fd;
157-
int client_fd;
158-
int old_errno;
159195

160196
fd = socket(AF_VSOCK, type, 0);
197+
if (fd < 0) {
198+
perror("socket");
199+
exit(EXIT_FAILURE);
200+
}
161201

162202
if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
163203
perror("bind");
@@ -169,6 +209,24 @@ static int vsock_accept(unsigned int cid, unsigned int port,
169209
exit(EXIT_FAILURE);
170210
}
171211

212+
return fd;
213+
}
214+
215+
/* Listen on <cid, port> and return the first incoming connection. The remote
216+
* address is stored to clientaddrp. clientaddrp may be NULL.
217+
*/
218+
static int vsock_accept(unsigned int cid, unsigned int port,
219+
struct sockaddr_vm *clientaddrp, int type)
220+
{
221+
union {
222+
struct sockaddr sa;
223+
struct sockaddr_vm svm;
224+
} clientaddr;
225+
socklen_t clientaddr_len = sizeof(clientaddr.svm);
226+
int fd, client_fd, old_errno;
227+
228+
fd = vsock_listen(cid, port, type);
229+
172230
control_writeln("LISTENING");
173231

174232
timeout_begin(TIMEOUT);
@@ -207,6 +265,11 @@ int vsock_stream_accept(unsigned int cid, unsigned int port,
207265
return vsock_accept(cid, port, clientaddrp, SOCK_STREAM);
208266
}
209267

268+
int vsock_stream_listen(unsigned int cid, unsigned int port)
269+
{
270+
return vsock_listen(cid, port, SOCK_STREAM);
271+
}
272+
210273
int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
211274
struct sockaddr_vm *clientaddrp)
212275
{

‎tools/testing/vsock/util.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,12 @@ struct test_case {
3636
void init_signals(void);
3737
unsigned int parse_cid(const char *str);
3838
int vsock_stream_connect(unsigned int cid, unsigned int port);
39+
int vsock_bind_connect(unsigned int cid, unsigned int port,
40+
unsigned int bind_port, int type);
3941
int vsock_seqpacket_connect(unsigned int cid, unsigned int port);
4042
int vsock_stream_accept(unsigned int cid, unsigned int port,
4143
struct sockaddr_vm *clientaddrp);
44+
int vsock_stream_listen(unsigned int cid, unsigned int port);
4245
int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
4346
struct sockaddr_vm *clientaddrp);
4447
void vsock_wait_remote_close(int fd);

‎tools/testing/vsock/vsock_test.c‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,6 +1180,51 @@ static void test_stream_shutrd_server(const struct test_opts *opts)
11801180
close(fd);
11811181
}
11821182

1183+
static void test_double_bind_connect_server(const struct test_opts *opts)
1184+
{
1185+
int listen_fd, client_fd, i;
1186+
struct sockaddr_vm sa_client;
1187+
socklen_t socklen_client = sizeof(sa_client);
1188+
1189+
listen_fd = vsock_stream_listen(VMADDR_CID_ANY, 1234);
1190+
1191+
for (i = 0; i < 2; i++) {
1192+
control_writeln("LISTENING");
1193+
1194+
timeout_begin(TIMEOUT);
1195+
do {
1196+
client_fd = accept(listen_fd, (struct sockaddr *)&sa_client,
1197+
&socklen_client);
1198+
timeout_check("accept");
1199+
} while (client_fd < 0 && errno == EINTR);
1200+
timeout_end();
1201+
1202+
if (client_fd < 0) {
1203+
perror("accept");
1204+
exit(EXIT_FAILURE);
1205+
}
1206+
1207+
/* Waiting for remote peer to close connection */
1208+
vsock_wait_remote_close(client_fd);
1209+
}
1210+
1211+
close(listen_fd);
1212+
}
1213+
1214+
static void test_double_bind_connect_client(const struct test_opts *opts)
1215+
{
1216+
int i, client_fd;
1217+
1218+
for (i = 0; i < 2; i++) {
1219+
/* Wait until server is ready to accept a new connection */
1220+
control_expectln("LISTENING");
1221+
1222+
client_fd = vsock_bind_connect(opts->peer_cid, 1234, 4321, SOCK_STREAM);
1223+
1224+
close(client_fd);
1225+
}
1226+
}
1227+
11831228
static struct test_case test_cases[] = {
11841229
{
11851230
.name = "SOCK_STREAM connection reset",
@@ -1285,6 +1330,11 @@ static struct test_case test_cases[] = {
12851330
.run_client = test_stream_msgzcopy_empty_errq_client,
12861331
.run_server = test_stream_msgzcopy_empty_errq_server,
12871332
},
1333+
{
1334+
.name = "SOCK_STREAM double bind connect",
1335+
.run_client = test_double_bind_connect_client,
1336+
.run_server = test_double_bind_connect_server,
1337+
},
12881338
{},
12891339
};
12901340

0 commit comments

Comments
 (0)