[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5252: net/tcp/tcp_send*: reliably obtain the TCP connection pointer in TCP event handlers

[GitBox <gi...@apache.org>]

anchao commented on a change in pull request #5252:
URL: https://github.com/apache/incubator-nuttx/pull/5252#discussion_r797481639



##########
File path: net/tcp/tcp_send_buffered.c
##########
@@ -355,10 +355,39 @@ static uint16_t psock_send_eventhandler(FAR struct net_driver_s *dev,
                                         FAR void *pvconn, FAR void *pvpriv,
                                         uint16_t flags)
 {
-  FAR struct tcp_conn_s *conn = (FAR struct tcp_conn_s *)pvconn;
+  /* FAR struct tcp_conn_s *conn = (FAR struct tcp_conn_s *)pvconn;
+   *
+   * Do not use pvconn argument to get the TCP connection pointer (the above
+   * commented line) because pvconn is normally NULL for some events like
+   * NETDEV_DOWN. Instead, the TCP connection pointer can be reliably
+   * obtained from the corresponding TCP socket.
+   */
+
   FAR struct socket *psock = (FAR struct socket *)pvpriv;
+  FAR struct tcp_conn_s *conn;
   bool rexmit = false;
 
+  DEBUGASSERT(psock != NULL);
+
+  /* Get the TCP connection pointer reliably from
+   * the corresponding TCP socket.
+   */
+
+  conn = psock->s_conn;
+  DEBUGASSERT(conn != NULL);
+
+  /* The TCP socket is connected and, hence, should be bound to a device.
+   * Make sure that the polling device is the one that we are bound to.
+   */
+
+  DEBUGASSERT(conn->dev != NULL);

Review comment:
       I have faced a similar issue on internal code base, the socket handle becomes a wild pointer since the socket instance has been closed before devif callback, the root cause is multiple socket instances(duplicate by task_create/system/exec ...) referencing a tcp connect instance at the same time, if the socket referenced by the s_sndcb is closed, then crash happened.
   
   The solution is move all the socket members into connect instance to ensure that connect does not reference the duplicate resources.
   
   This issue has been solved last year, I will prepare the patch for upstream.




-- 
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org