zhhyu7 opened a new pull request, #21254:
URL: https://github.com/apache/nuttx/pull/21254
## Summary
`tcp_connect()` put the new conn on `g_active_tcp_connections` as its last
step, holding only the conn-list lock. The device lock was not held, so
between that insertion and the callback setup in `psock_tcp_connect()`,
a concurrent `netdev_txnotify_dev()` could drive
`devif_poll_tcp_connections()`
over the list and reach the conn while its callbacks were not yet installed.
Since `tcp_connect()` sets `tcpstateflags = TCP_SYN_SENT` and `timeout =
true`,
the poll could start the handshake on a half-initialized conn.
Move the insertion out of `tcp_connect()` into a new `tcp_add_active_conn()`
and call it from `psock_tcp_connect()` on both paths: inside the
`conn_dev_lock()` region for the blocking case, and directly for the
non-blocking (`-EINPROGRESS`) case. `tcp_alloc_accept()` uses the same helper
instead of open-coding lock/insert/unlock.
Files: `net/tcp/tcp.h`, `net/tcp/tcp_conn.c`, `net/tcp/tcp_connect.c`
## Impact
If the `conn` is added to the activation queue without lock protection,
other threads might trigger `netdev_txnotify_dev`, causing this `conn`
to also trigger the three-way handshake.
## Testing
`sim:tcploop` (IPv6 loopback, `CONFIG_DEBUG_ASSERTIONS=y`). Loopback connect
plus data exchange, 5 runs each, before and after:
- blocking connect: `connect`=0, accept succeeds, 4 bytes received — PASS
- non-blocking connect: `connect`=-1/`EINPROGRESS`, accept succeeds, 4 bytes
received — PASS
One of the test examples is as follows:
```
#include <nuttx/config.h>
#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>
#include <fcntl.h>
#include <arpa/inet.h>
#include <sys/socket.h>
#include <netinet/in.h>
#define PORT 5555
int main(int argc, char *argv[])
{
struct sockaddr_in6 a;
int lfd;
int cfd;
int afd;
int one = 1;
int ret;
int nonblock = (argc > 1 && argv[1][0] == 'n');
lfd = socket(PF_INET6, SOCK_STREAM, 0);
setsockopt(lfd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one));
memset(&a, 0, sizeof(a));
a.sin6_family = AF_INET6;
a.sin6_port = htons(PORT);
if (bind(lfd, (struct sockaddr *)&a, sizeof(a)) < 0)
[160/1954] {
printf("bind errno=%d\n", errno);
return 1;
}
if (listen(lfd, 4) < 0)
{
printf("listen errno=%d\n", errno);
return 1;
}
printf("listening, mode=%s\n", nonblock ? "nonblock" : "block");
fflush(stdout);
cfd = socket(PF_INET6, SOCK_STREAM, 0);
if (nonblock)
{
fcntl(cfd, F_SETFL, fcntl(cfd, F_GETFL, 0) | O_NONBLOCK);
}
inet_pton(AF_INET6, "::1", &a.sin6_addr);
ret = connect(cfd, (struct sockaddr *)&a, sizeof(a));
printf("connect ret=%d errno=%d%s\n", ret, ret < 0 ? errno : 0,
(ret < 0 && errno == EINPROGRESS) ? " (EINPROGRESS, as expected)"
: "");
fflush(stdout);
if (ret < 0 && errno != EINPROGRESS)
{
printf("RESULT: FAIL\n");
return 1;
}
afd = accept(lfd, NULL, NULL);
printf("accept ret=%d errno=%d\n", afd, afd < 0 ? errno : 0);
fflush(stdout);
if (afd < 0)
{
printf("RESULT: FAIL\n");
return 1;
}
/* Prove the handshake finished: exchange data. */
fcntl(cfd, F_SETFL, fcntl(cfd, F_GETFL, 0) & ~O_NONBLOCK);
if (send(cfd, "ping", 4, 0) != 4)
{
printf("send errno=%d\nRESULT: FAIL\n", errno);
return 1;
}
{
char buf[8];
ssize_t n = recv(afd, buf, sizeof(buf), 0);
printf("server received %zd bytes\n", n);
if (n != 4)
{
printf("RESULT: FAIL\n");
return 1;
}
}
close(cfd);
close(afd);
close(lfd);
printf("RESULT: PASS\n");
return 0;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
