casaroli commented on code in PR #21403:
URL: https://github.com/apache/nuttx/pull/21403#discussion_r4459300662
##########
net/rpmsg/rpmsg_sockif.c:
##########
@@ -345,6 +351,20 @@ static int rpmsg_socket_ept_cb(FAR struct rpmsg_endpoint
*ept,
rpmsg_socket_post(&conn->sendsem);
rpmsg_socket_poll_notify(conn, POLLOUT);
nxmutex_unlock(&conn->sendlock);
+
+ /* Accept new connection and complete the final handshake */
+
+ if (conn->server)
Review Comment:
Not fixed: `conn->server` still points to the listener after the connection
is queued (line 398) and after accept() (lines 975-990), and
`rpmsg_socket_ns_unbind()` locks `server->recvlock` at line 602. When the
listener is closed before the peer unbinds, that lock is in freed memory.
Fix: set `conn->server = NULL` under `conn->recvlock` once
`rpmsg_socket_pend_remove()` succeeds at line 389.
##########
net/rpmsg/rpmsg_sockif.c:
##########
@@ -599,33 +627,28 @@ static void rpmsg_socket_ns_bind(FAR struct rpmsg_device
*rdev,
return;
}
+ nxmutex_lock(&server->recvlock);
+ if (server->pending >= server->backlog)
+ {
+ /* Reject the connection */
+
+ nerr("Reject the connection, Max listen %d\n", server->backlog);
+ nxmutex_unlock(&server->recvlock);
+ rpmsg_destroy_ept(&new->ept);
+ rpmsg_socket_free(new);
+ return;
+ }
+
+ server->pending++;
Review Comment:
The count is fixed, but line 607 frees the connection while its endpoint
`conn->ept` (embedded, line 100) is still registered: after `ns_unbind_cb`
returns, `rpmsg_virtio_ns_callback()` still calls `rpmsg_ept_decref()` on it,
because `release_cb` is set.
Fix: `conn->crefs = 0; rpmsg_destroy_ept(&conn->ept);` instead of
`rpmsg_socket_free(conn)`, and let `release_cb` free the connection.
--
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]
