Networking: Fix another deadlock condition. tcp_write_buffer_alloc() calls sem_wait() with network locked. That worked if CONFIG_NET_NOINTS was not defined because interrupts are automatically restored when the wait happens. But with CONFIG_NET_NOINTS=y, the wait blocks with the network locked -- bad style and also can lead to a deadlock condition

4 files changed, 57 insertions, 48 deletions

diff --git a/nuttx/net/socket/send.c b/nuttx/net/socket/send.c
index 1b7358481..c3ad1694c 100644
--- a/nuttx/net/socket/send.c
+++ b/nuttx/net/socket/send.c
@@ -152,8 +152,13 @@ ssize_t psock_send(FAR struct socket *psock, FAR const void *buf, size_t len,
 
       default:
         {
-          ret = ERROR;
+          /* EDESTADDRREQ.  Signifies that the socket is not connection-mode
+           * and no peer address is set.
+           */
+
+          ret = -EDESTADDRREQ;
         }
+        break;
     }
 
   return ret;
diff --git a/nuttx/net/tcp/tcp.h b/nuttx/net/tcp/tcp.h
index 1d6e78e8c..98b9d68d9 100644
--- a/nuttx/net/tcp/tcp.h
+++ b/nuttx/net/tcp/tcp.h
@@ -1000,7 +1000,7 @@ void tcp_wrbuffer_initialize(void);
  *   None
  *
  * Assumptions:
- *   Called from user logic with interrupts enabled.
+ *   Called from user logic with the network locked.
  *
  ****************************************************************************/
 
diff --git a/nuttx/net/tcp/tcp_send_buffered.c b/nuttx/net/tcp/tcp_send_buffered.c
index 43f80a856..ca8dddc4a 100644
--- a/nuttx/net/tcp/tcp_send_buffered.c
+++ b/nuttx/net/tcp/tcp_send_buffered.c
@@ -918,6 +918,7 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf,
                        size_t len)
 {
   FAR struct tcp_conn_s *conn;
+  FAR struct tcp_wrbuffer_s *wrb;
   net_lock_t save;
   ssize_t    result = 0;
   int        err;
@@ -940,6 +941,7 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf,
   /* Make sure that the IP address mapping is in the ARP table */
 
   conn = (FAR struct tcp_conn_s *)psock->s_conn;
+
 #ifdef CONFIG_NET_ARP_SEND
   ret = arp_send(conn->u.ipv4.raddr);
   if (ret < 0)
@@ -958,10 +960,23 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf,
 
   psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND);
 
-  save = net_lock();
-
   if (len > 0)
     {
+      /* Allocate a write buffer.  Careful, the network will be momentarily
+       * unlocked here.
+       */
+
+      save = net_lock();
+      wrb = tcp_wrbuffer_alloc();
+      if (!wrb)
+        {
+          /* A buffer allocation error occurred */
+
+          ndbg("ERROR: Failed to allocate write buffer\n");
+          err = ENOMEM;
+          goto errout_with_lock;
+        }
+
       /* Allocate resources to receive a callback */
 
       if (!psock->s_sndcb)
@@ -976,61 +991,43 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf,
           /* A buffer allocation error occurred */
 
           ndbg("ERROR: Failed to allocate callback\n");
-          result = -ENOMEM;
+          err = ENOMEM;
+          goto errout_with_wrb;
         }
-      else
-        {
-          FAR struct tcp_wrbuffer_s *wrb;
 
-          /* Set up the callback in the connection */
+      /* Set up the callback in the connection */
 
-          psock->s_sndcb->flags = (TCP_ACKDATA | TCP_REXMIT | TCP_POLL |
-                                   TCP_CLOSE | TCP_ABORT | TCP_TIMEDOUT);
-          psock->s_sndcb->priv  = (void*)psock;
-          psock->s_sndcb->event = psock_send_interrupt;
+      psock->s_sndcb->flags = (TCP_ACKDATA | TCP_REXMIT | TCP_POLL |
+                               TCP_CLOSE | TCP_ABORT | TCP_TIMEDOUT);
+      psock->s_sndcb->priv  = (void*)psock;
+      psock->s_sndcb->event = psock_send_interrupt;
 
-          /* Allocate an write buffer */
+      /* Initialize the write buffer */
 
-          wrb = tcp_wrbuffer_alloc();
-          if (wrb)
-            {
-              /* Initialize the write buffer */
-
-              WRB_SEQNO(wrb) = (unsigned)-1;
-              WRB_NRTX(wrb)  = 0;
-              WRB_COPYIN(wrb, (FAR uint8_t *)buf, len);
+      WRB_SEQNO(wrb) = (unsigned)-1;
+      WRB_NRTX(wrb)  = 0;
+      WRB_COPYIN(wrb, (FAR uint8_t *)buf, len);
 
-              /* Dump I/O buffer chain */
+      /* Dump I/O buffer chain */
 
-              WRB_DUMP("I/O buffer chain", wrb, WRB_PKTLEN(wrb), 0);
+      WRB_DUMP("I/O buffer chain", wrb, WRB_PKTLEN(wrb), 0);
 
-              /* psock_send_interrupt() will send data in FIFO order from the
-               * conn->write_q
-               */
-
-              sq_addlast(&wrb->wb_node, &conn->write_q);
-              nvdbg("Queued WRB=%p pktlen=%u write_q(%p,%p)\n",
-                    wrb, WRB_PKTLEN(wrb),
-                    conn->write_q.head, conn->write_q.tail);
+      /* psock_send_interrupt() will send data in FIFO order from the
+       * conn->write_q
+       */
 
-              /* Notify the device driver of the availability of TX data */
+      sq_addlast(&wrb->wb_node, &conn->write_q);
+      nvdbg("Queued WRB=%p pktlen=%u write_q(%p,%p)\n",
+            wrb, WRB_PKTLEN(wrb),
+            conn->write_q.head, conn->write_q.tail);
 
-              send_txnotify(psock, conn);
-              result = len;
-            }
+      /* Notify the device driver of the availability of TX data */
 
-          /* A buffer allocation error occurred */
-
-          else
-            {
-              ndbg("ERROR: Failed to allocate write buffer\n");
-              result = -ENOMEM;
-            }
-        }
+      send_txnotify(psock, conn);
+      net_unlock(save);
+      result = len;
     }
 
-  net_unlock(save);
-
   /* Set the socket state to idle */
 
   psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_IDLE);
@@ -1059,6 +1056,12 @@ ssize_t psock_tcp_send(FAR struct socket *psock, FAR const void *buf,
 
   return result;
 
+errout_with_wrb:
+  tcp_wrbuffer_release(wrb);
+
+errout_with_lock:
+  net_unlock(save);
+
 errout:
   set_errno(err);
   return ERROR;
diff --git a/nuttx/net/tcp/tcp_wrbuffer.c b/nuttx/net/tcp/tcp_wrbuffer.c
index 162d30ad6..3bdb8b2ba 100644
--- a/nuttx/net/tcp/tcp_wrbuffer.c
+++ b/nuttx/net/tcp/tcp_wrbuffer.c
@@ -54,6 +54,7 @@
 #include <assert.h>
 #include <debug.h>
 
+#include <nuttx/net/net.h>
 #include <nuttx/net/iob.h>
 
 #include "tcp/tcp.h"
@@ -132,7 +133,7 @@ void tcp_wrbuffer_initialize(void)
  *   None
  *
  * Assumptions:
- *   Called from user logic with interrupts enabled.
+ *   Called from user logic with the network locked.
  *
  ****************************************************************************/
 
@@ -148,7 +149,7 @@ FAR struct tcp_wrbuffer_s *tcp_wrbuffer_alloc(void)
    * buffer
    */
 
-  DEBUGVERIFY(sem_wait(&g_wrbuffer.sem));
+  DEBUGVERIFY(net_lockedwait(&g_wrbuffer.sem));
 
   /* Now, we are guaranteed to have a write buffer structure reserved
    * for us in the free list.