irda update 7/7:

        o [CORRECT] Prevent dealock on simultaneous peer IrNET connections
                Only the primary peer will accept the IrNET connection

diff --git a/include/net/irda/irlap.h b/include/net/irda/irlap.h
index 55fc5cb4794440ce39aa39600c73749fabc77cb4..ae30117cbf6e8bb1ef88cc02895469cd0cdf2eba 100644 (file)
--- a/include/net/irda/irlap.h
+++ b/include/net/irda/irlap.h
@@ -31,8 +31,6 @@
 #include <linux/types.h>
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>
-#include <linux/ppp_defs.h>
-#include <linux/ppp-comp.h>
 #include <linux/timer.h>
 
 #include <net/irda/irlap_event.h>
@@ -240,4 +238,24 @@ void irlap_apply_connection_parameters(struct irlap_cb *self, int now);
 #define IRLAP_GET_HEADER_SIZE(self) (LAP_MAX_HEADER)
 #define IRLAP_GET_TX_QUEUE_LEN(self) skb_queue_len(&self->txq)
 
+/* Return TRUE if the node is in primary mode (i.e. master)
+ * - Jean II */
+static inline int irlap_is_primary(struct irlap_cb *self)
+{
+       int ret;
+       switch(self->state) {
+       case LAP_XMIT_P:
+       case LAP_NRM_P:
+               ret = 1;
+               break;
+       case LAP_XMIT_S:
+       case LAP_NRM_S:
+               ret = 0;
+               break;
+       default:
+               ret = -1;
+       }
+       return(ret);
+}
+
 #endif

diff --git a/include/net/irda/irttp.h b/include/net/irda/irttp.h
index e4653bee886b581278e952fde03ce7936057e263..c603927ddb9b79582a7ab01ce34efced02028bf8 100644 (file)
--- a/include/net/irda/irttp.h
+++ b/include/net/irda/irttp.h
@@ -197,6 +197,18 @@ static inline void irttp_listen(struct tsap_cb *self)
        self->dtsap_sel = LSAP_ANY;
 }
 
+/* Return TRUE if the node is in primary mode (i.e. master)
+ * - Jean II */
+static inline int irttp_is_primary(struct tsap_cb *self)
+{
+       if ((self == NULL) ||
+           (self->lsap == NULL) ||
+           (self->lsap->lap == NULL) ||
+           (self->lsap->lap->irlap == NULL))
+               return -2;
+       return(irlap_is_primary(self->lsap->lap->irlap));
+}
+
 extern struct irttp_cb *irttp;
 
 #endif /* IRTTP_H */

diff --git a/net/irda/irnet/irnet.h b/net/irda/irnet/irnet.h
index f45b4e05695edf1c537d43023fdf3ea060e1ca42..45b5186a31d51820038e2e1cfceac23b1ede0554 100644 (file)
--- a/net/irda/irnet/irnet.h
+++ b/net/irda/irnet/irnet.h
@@ -217,6 +217,11 @@
  *       Better fix in irnet_disconnect_indication() :
  *       - if connected, kill pppd via hangup.
  *       - if not connected, reenable ppp Tx, which trigger IrNET retry.
+ *
+ * v12 - 10.4.02 - Jean II
+ *     o Fix race condition in irnet_connect_indication().
+ *       If the socket was already trying to connect, drop old connection
+ *       and use new one only if acting as primary. See comments.
  */
 
 /***************************** INCLUDES *****************************/

diff --git a/net/irda/irnet/irnet_irda.c b/net/irda/irnet/irnet_irda.c
index e7466b3f527ac318c97c1b4fa7a76cafe763dcea..8e629b9c4f1f267f3dcf7cf80b3393fe6ca74c19 100644 (file)
--- a/net/irda/irnet/irnet_irda.c
+++ b/net/irda/irnet/irnet_irda.c
@@ -1340,46 +1340,80 @@ irnet_connect_indication(void *         instance,
       return;
     }
 
-  /* Socket connecting ?
-   * Clear up flag : prevent irnet_disconnect_indication() to mess up tsap */
-  if(test_and_clear_bit(0, &new->ttp_connect))
-    {
-      /* The socket is trying to connect to the other end and may have sent
-       * a IrTTP connection request and is waiting for a connection response
-       * (that may never come).
-       * Now, the pain is that the socket may have opened a tsap and is
-       * waiting on it, while the other end is trying to connect to it on
-       * another tsap.
-       */
-      DERROR(IRDA_CB_ERROR, "Socket already connecting. Ouch !\n");
+  /* The following code is a bit tricky, so need comments ;-)
+   */
+  /* If ttp_connect is set, the socket is trying to connect to the other
+   * end and may have sent a IrTTP connection request and is waiting for
+   * a connection response (that may never come).
+   * Now, the pain is that the socket may have opened a tsap and is
+   * waiting on it, while the other end is trying to connect to it on
+   * another tsap.
+   * Because IrNET can be peer to peer, we need to workaround this.
+   * Furthermore, the way the irnetd script is implemented, the
+   * target will create a second IrNET connection back to the
+   * originator and expect the originator to bind this new connection
+   * to the original PPPD instance.
+   * And of course, if we don't use irnetd, we can have a race when
+   * both side try to connect simultaneously, which could leave both
+   * connections half closed (yuck).
+   * Conclusions :
+   *   1) The "originator" must accept the new connection and get rid
+   *      of the old one so that irnetd works
+   *   2) One side must deny the new connection to avoid races,
+   *      but both side must agree on which side it is...
+   * Most often, the originator is primary at the LAP layer.
+   * Jean II
+   */
+  /* Now, let's look at the way I wrote the test...
+   * We need to clear up the ttp_connect flag atomically to prevent
+   * irnet_disconnect_indication() to mess up the tsap we are going to close.
+   * We want to clear the ttp_connect flag only if we close the tsap,
+   * otherwise we will never close it, so we need to check for primary
+   * *before* doing the test on the flag.
+   * And of course, ALLOW_SIMULT_CONNECT can disable this entirely...
+   * Jean II
+   */
+
+  /* Socket already connecting ? On primary ? */
+  if(0
 #ifdef ALLOW_SIMULT_CONNECT
-      /* Cleanup the TSAP if necessary - IrIAP will be cleaned up later */
+     || ((irttp_is_primary(server->tsap) == 1) /* primary */
+        && (test_and_clear_bit(0, &new->ttp_connect)))
+#endif /* ALLOW_SIMULT_CONNECT */
+     )
+    {
+      DERROR(IRDA_CB_ERROR, "Socket already connecting, but going to reuse it !\n");
+
+      /* Cleanup the old TSAP if necessary - IrIAP will be cleaned up later */
       if(new->tsap != NULL)
        {
-         /* Close the connection the new socket was attempting.
-          * This seems to be safe... */
+         /* Close the old connection the new socket was attempting,
+          * so that we can hook it up to the new connection.
+          * It's now safe to do it... */
          irttp_close_tsap(new->tsap);
          new->tsap = NULL;
        }
-      /* Note : no return, fall through... */
-#else /* ALLOW_SIMULT_CONNECT */
-      irnet_disconnect_server(server, skb);
-      return;
-#endif /* ALLOW_SIMULT_CONNECT */
     }
   else
-    /* If socket is not connecting or connected, tsap should be NULL */
-    if(new->tsap != NULL)
-      {
-       /* If we are here, we are also in irnet_disconnect_indication(),
-        * and it's a nice race condition... On the other hand, we can't be
-        * in irda_irnet_destroy() otherwise we would not have found the
-        * socket in the hashbin. */
-       /* Better get out of here, otherwise we will mess up tsaps ! */
-       DERROR(IRDA_CB_ERROR, "Race condition detected, abort connect...\n");
-       irnet_disconnect_server(server, skb);
-       return;
-      }
+    {
+      /* Three options :
+       * 1) socket was not connecting or connected : ttp_connect should be 0.
+       * 2) we don't want to connect the socket because we are secondary or
+       * ALLOW_SIMULT_CONNECT is undefined. ttp_connect should be 1.
+       * 3) we are half way in irnet_disconnect_indication(), and it's a
+       * nice race condition... Fortunately, we can detect that by checking
+       * if tsap is still alive. On the other hand, we can't be in
+       * irda_irnet_destroy() otherwise we would not have found this
+       * socket in the hashbin.
+       * Jean II */
+      if((test_bit(0, &new->ttp_connect)) || (new->tsap != NULL))
+       {
+         /* Don't mess this socket, somebody else in in charge... */
+         DERROR(IRDA_CB_ERROR, "Race condition detected, socket in use, abort connect...\n");
+         irnet_disconnect_server(server, skb);
+         return;
+       }
+    }
 
   /* So : at this point, we have a socket, and it is idle. Good ! */
   irnet_connect_socket(server, new, qos, max_sdu_size, max_header_size);