[ipoib] Fix a race when chain-loading undionly.kpxe in IPoIB

The Infiniband link status change callback ipoib_link_state_changed()
may be called while the IPoIB device is closed, in which case there
will not be an IPoIB queue pair to be joined to the IPv4 broadcast
group.  This leads to NULL pointer dereferences in ib_mcast_attach()
and ib_mcast_detach().

Fix by not attempting to join (or leave) the broadcast group unless we
actually have an IPoIB queue pair.

Signed-off-by: Wissam Shoukair <wissams@mellanox.com>
Modified-by: Michael Brown <mcb30@ipxe.org>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/drivers/net/ipoib.c b/src/drivers/net/ipoib.c
index 7ecb2c5dd056ba9f7678ec74566aaa5113028178..6552d764e312ecd487a92143ab4b52ece84242e4 100644 (file)
--- a/src/drivers/net/ipoib.c
+++ b/src/drivers/net/ipoib.c
@@ -791,7 +791,8 @@ static void ipoib_link_state_changed ( struct ib_device *ibdev ) {
        int rc;
 
        /* Leave existing broadcast group */
-       ipoib_leave_broadcast_group ( ipoib );
+       if ( ipoib->qp )
+               ipoib_leave_broadcast_group ( ipoib );
 
        /* Update MAC address based on potentially-new GID prefix */
        memcpy ( &ipoib->mac.gid.s.prefix, &ibdev->gid.s.prefix,
@@ -806,7 +807,7 @@ static void ipoib_link_state_changed ( struct ib_device *ibdev ) {
        netdev_link_err ( netdev, ( rc ? rc : -EINPROGRESS_JOINING ) );
 
        /* Join new broadcast group */
-       if ( ib_is_open ( ibdev ) && ib_link_ok ( ibdev ) &&
+       if ( ib_is_open ( ibdev ) && ib_link_ok ( ibdev ) && ipoib->qp &&
             ( ( rc = ipoib_join_broadcast_group ( ipoib ) ) != 0 ) ) {
                DBGC ( ipoib, "IPoIB %p could not rejoin broadcast group: "
                       "%s\n", ipoib, strerror ( rc ) );
@@ -894,7 +895,9 @@ static void ipoib_close ( struct net_device *netdev ) {
 
        /* Tear down the queues */
        ib_destroy_qp ( ibdev, ipoib->qp );
+       ipoib->qp = NULL;
        ib_destroy_cq ( ibdev, ipoib->cq );
+       ipoib->cq = NULL;
 
        /* Close IB device */
        ib_close ( ibdev );

diff --git a/src/net/infiniband.c b/src/net/infiniband.c
index 8ba031f1df0d3c77a233d8166e2b4217f443c3da..2e3d76d541cdaadaec09921cfa0e235b685fafef 100644 (file)
--- a/src/net/infiniband.c
+++ b/src/net/infiniband.c
@@ -718,6 +718,9 @@ int ib_mcast_attach ( struct ib_device *ibdev, struct ib_queue_pair *qp,
        struct ib_multicast_gid *mgid;
        int rc;
 
+       /* Sanity check */
+       assert ( qp != NULL );
+
        /* Add to software multicast GID list */
        mgid = zalloc ( sizeof ( *mgid ) );
        if ( ! mgid ) {
@@ -751,6 +754,9 @@ void ib_mcast_detach ( struct ib_device *ibdev, struct ib_queue_pair *qp,
                       union ib_gid *gid ) {
        struct ib_multicast_gid *mgid;
 
+       /* Sanity check */
+       assert ( qp != NULL );
+
        /* Remove from hardware multicast GID list */
        ibdev->op->mcast_detach ( ibdev, qp, gid );
 

diff --git a/src/net/infiniband/ib_mcast.c b/src/net/infiniband/ib_mcast.c
index 02369269f59d4724700bece0ad67a873dbdf2715..fc4ff7f0a9d2ebccfa11c6fa2c44a5238e381ab3 100644 (file)
--- a/src/net/infiniband/ib_mcast.c
+++ b/src/net/infiniband/ib_mcast.c
@@ -150,6 +150,9 @@ int ib_mcast_join ( struct ib_device *ibdev, struct ib_queue_pair *qp,
        DBGC ( ibdev, "IBDEV %p QPN %lx joining " IB_GID_FMT "\n",
               ibdev, qp->qpn, IB_GID_ARGS ( gid ) );
 
+       /* Sanity check */
+       assert ( qp != NULL );
+
        /* Initialise structure */
        membership->qp = qp;
        memcpy ( &membership->gid, gid, sizeof ( membership->gid ) );
@@ -199,6 +202,9 @@ void ib_mcast_leave ( struct ib_device *ibdev, struct ib_queue_pair *qp,
        DBGC ( ibdev, "IBDEV %p QPN %lx leaving " IB_GID_FMT "\n",
               ibdev, qp->qpn, IB_GID_ARGS ( gid ) );
 
+       /* Sanity check */
+       assert ( qp != NULL );
+
        /* Detach from multicast GID */
        ib_mcast_detach ( ibdev, qp, &membership->gid );