UCT/IB: Fix handling of invalid IB address

src/uct/ib/base/ib_iface.c

@@ -516,8 +516,8 @@ void uct_ib_iface_address_pack(uct_ib_iface_t *iface, uct_ib_address_t *ib_addr)
     uct_ib_address_pack(&params, ib_addr);
 }
 
-void uct_ib_address_unpack(const uct_ib_address_t *ib_addr,
-                           uct_ib_address_pack_params_t *params_p)
+ucs_status_t uct_ib_address_unpack(const uct_ib_address_t *ib_addr,
+                                   uct_ib_address_pack_params_t *params_p)
 {
     const void *ptr                     = ib_addr + 1;
     /* silence cppcheck warning */
@@ -554,14 +554,15 @@ void uct_ib_address_unpack(const uct_ib_address_t *ib_addr,
             params.flags |= UCT_IB_ADDRESS_PACK_FLAG_INTERFACE_ID;
         }
 
-        if (ib_addr->flags & UCT_IB_ADDRESS_FLAG_SUBNET16) {
+        if (ucs_test_all_flags(ib_addr->flags,
+                               UCT_IB_ADDRESS_FLAG_SUBNET16 |
+                               UCT_IB_ADDRESS_FLAG_SUBNET64)) {
+            return UCS_ERR_INVALID_PARAM;
+        } else if (ib_addr->flags & UCT_IB_ADDRESS_FLAG_SUBNET16) {
             site_local_subnet = *ucs_serialize_next(&ptr, const uint16_t);
             params.gid.global.subnet_prefix = UCT_IB_SITE_LOCAL_PREFIX |
                                               (site_local_subnet << 48);
-            ucs_assert(!(ib_addr->flags & UCT_IB_ADDRESS_FLAG_SUBNET64));
-        }
-
-        if (ib_addr->flags & UCT_IB_ADDRESS_FLAG_SUBNET64) {
+        } else if (ib_addr->flags & UCT_IB_ADDRESS_FLAG_SUBNET64) {
             params.gid.global.subnet_prefix =
                     *ucs_serialize_next(&ptr, const uint64_t);
             params.flags |= UCT_IB_ADDRESS_PACK_FLAG_SUBNET_PREFIX;
@@ -586,15 +587,20 @@ void uct_ib_address_unpack(const uct_ib_address_t *ib_addr,
     params.flags |= UCT_IB_ADDRESS_PACK_FLAG_PKEY;
 
     *params_p = params;
+    return UCS_OK;
 }
 
 const char *uct_ib_address_str(const uct_ib_address_t *ib_addr, char *buf,
                                size_t max)
 {
     uct_ib_address_pack_params_t params;
+    ucs_status_t status;
     char *p, *endp;
 
-    uct_ib_address_unpack(ib_addr, &params);
+    status = uct_ib_address_unpack(ib_addr, &params);
+    if (status != UCS_OK) {
+        return "<invalid address>";
+    }
 
     p    = buf;
     endp = buf + max;
@@ -810,8 +816,12 @@ int uct_ib_iface_is_same_device(const uct_ib_address_t *ib_addr, uint16_t dlid,
                                 const union ibv_gid *dgid)
 {
     uct_ib_address_pack_params_t params;
+    ucs_status_t status;
 
-    uct_ib_address_unpack(ib_addr, &params);
+    status = uct_ib_address_unpack(ib_addr, &params);
+    if (status != UCS_OK) {
+        return 0;
+    }
 
     if (!(params.flags & UCT_IB_ADDRESS_PACK_FLAG_ETH) &&
         (dlid != params.lid)) {
@@ -849,8 +859,12 @@ static int uct_ib_iface_dev_addr_is_reachable(
     int is_local_eth                = uct_ib_iface_is_roce(iface);
     uct_ib_address_pack_params_t params;
     const char *flid_info_str;
+    ucs_status_t status;
 
-    uct_ib_address_unpack(ib_addr, &params);
+    status = uct_ib_address_unpack(ib_addr, &params);
+    if (status != UCS_OK) {
+        return 0;
+    }
 
     /* at least one PKEY has to be with full membership */
     if (!((params.pkey | iface->pkey) & UCT_IB_PKEY_MEMBERSHIP_MASK)) {
@@ -1028,20 +1042,25 @@ uint16_t uct_ib_iface_resolve_remote_flid(uct_ib_iface_t *iface,
     return uct_ib_iface_gid_extract_flid(gid);
 }
 
-void uct_ib_iface_fill_ah_attr_from_addr(uct_ib_iface_t *iface,
-                                         const uct_ib_address_t *ib_addr,
-                                         unsigned path_index,
-                                         struct ibv_ah_attr *ah_attr,
-                                         enum ibv_mtu *path_mtu)
+ucs_status_t
+uct_ib_iface_fill_ah_attr_from_addr(uct_ib_iface_t *iface,
+                                    const uct_ib_address_t *ib_addr,
+                                    unsigned path_index,
+                                    struct ibv_ah_attr *ah_attr,
+                                    enum ibv_mtu *path_mtu)
 {
     union ibv_gid *gid = NULL;
     uint16_t lid, flid = 0;
     uct_ib_address_pack_params_t params;
+    ucs_status_t status;
 
     ucs_assert(!uct_ib_iface_is_roce(iface) ==
                !(ib_addr->flags & UCT_IB_ADDRESS_FLAG_LINK_LAYER_ETH));
 
-    uct_ib_address_unpack(ib_addr, &params);
+    status = uct_ib_address_unpack(ib_addr, &params);
+    if (status != UCS_OK) {
+        return status;
+    }
 
     if (params.flags & UCT_IB_ADDRESS_PACK_FLAG_PATH_MTU) {
         ucs_assert(params.path_mtu != UCT_IB_ADDRESS_INVALID_PATH_MTU);
@@ -1067,6 +1086,7 @@ void uct_ib_iface_fill_ah_attr_from_addr(uct_ib_iface_t *iface,
     lid = (flid == 0) ? params.lid : flid;
     uct_ib_iface_fill_ah_attr_from_gid_lid(iface, lid, gid, params.gid_index,
                                            path_index, ah_attr);
+    return UCS_OK;
 }
 
 static ucs_status_t uct_ib_iface_init_pkey(uct_ib_iface_t *iface,

src/uct/ib/base/ib_iface.h

@@ -526,8 +526,8 @@ void uct_ib_iface_address_pack(uct_ib_iface_t *iface, uct_ib_address_t *ib_addr)
  * @param [out] params_p   Filled with address attributes as in
  *                         @ref uct_ib_address_pack_params_t.
  */
-void uct_ib_address_unpack(const uct_ib_address_t *ib_addr,
-                           uct_ib_address_pack_params_t *params_p);
+ucs_status_t uct_ib_address_unpack(const uct_ib_address_t *ib_addr,
+                                   uct_ib_address_pack_params_t *params_p);
 
 
 /**
@@ -617,11 +617,12 @@ void uct_ib_iface_fill_ah_attr_from_gid_lid(uct_ib_iface_t *iface, uint16_t lid,
                                             unsigned path_index,
                                             struct ibv_ah_attr *ah_attr);
 
-void uct_ib_iface_fill_ah_attr_from_addr(uct_ib_iface_t *iface,
-                                         const uct_ib_address_t *ib_addr,
-                                         unsigned path_index,
-                                         struct ibv_ah_attr *ah_attr,
-                                         enum ibv_mtu *path_mtu);
+ucs_status_t
+uct_ib_iface_fill_ah_attr_from_addr(uct_ib_iface_t *iface,
+                                    const uct_ib_address_t *ib_addr,
+                                    unsigned path_index,
+                                    struct ibv_ah_attr *ah_attr,
+                                    enum ibv_mtu *path_mtu);
 
 ucs_status_t uct_ib_iface_pre_arm(uct_ib_iface_t *iface);
 

src/uct/ib/mlx5/gga/gga_mlx5.c

@@ -508,11 +508,14 @@ uct_gga_mlx5_ep_connect_to_ep_v2(uct_ep_h tl_ep,
     enum ibv_mtu path_mtu;
     ucs_status_t status;
 
-    uct_ib_iface_fill_ah_attr_from_addr(&iface->super.super, ib_addr,
-                                        ep->super.super.path_index, &ah_attr,
-                                        &path_mtu);
-    ucs_assert(path_mtu != UCT_IB_ADDRESS_INVALID_PATH_MTU);
+    status = uct_ib_iface_fill_ah_attr_from_addr(&iface->super.super, ib_addr,
+                                                 ep->super.super.path_index,
+                                                 &ah_attr, &path_mtu);
+    if (status != UCS_OK) {
+        return status;
+    }
 
+    ucs_assert(path_mtu != UCT_IB_ADDRESS_INVALID_PATH_MTU);
     qp_num = uct_ib_unpack_uint24(gga_ep_addr->super.qp_num);
     status = uct_rc_mlx5_iface_common_devx_connect_qp(
             iface, &ep->super.tx.wq.super, qp_num, &ah_attr, path_mtu,

src/uct/ib/mlx5/ib_mlx5.c

@@ -371,7 +371,12 @@ ucs_status_t uct_ib_mlx5_get_compact_av(uct_ib_iface_t *iface, int *compact_av)
         return status;
     }
 
-    uct_ib_iface_fill_ah_attr_from_addr(iface, ib_addr, 0, &ah_attr, &path_mtu);
+    status = uct_ib_iface_fill_ah_attr_from_addr(iface, ib_addr, 0, &ah_attr,
+                                                 &path_mtu);
+    if (status != UCS_OK) {
+        return status;
+    }
+
     ah_attr.is_global = iface->config.force_global_addr;
     status = uct_ib_iface_create_ah(iface, &ah_attr, "compact AV check", &ah);
     if (status != UCS_OK) {

src/uct/ib/mlx5/rc/rc_mlx5_ep.c

@@ -787,9 +787,13 @@ uct_rc_mlx5_ep_connect_to_ep_v2(uct_ep_h tl_ep,
     uint32_t flush_rkey_hi;
     ucs_status_t status;
 
-    uct_ib_iface_fill_ah_attr_from_addr(&iface->super.super, ib_addr,
-                                        ep->super.super.path_index, &ah_attr,
-                                        &path_mtu);
+    status = uct_ib_iface_fill_ah_attr_from_addr(&iface->super.super, ib_addr,
+                                                 ep->super.super.path_index,
+                                                 &ah_attr, &path_mtu);
+    if (status != UCS_OK) {
+        return status;
+    }
+
     ucs_assert(path_mtu != UCT_IB_ADDRESS_INVALID_PATH_MTU);
 
     if (UCT_RC_MLX5_TM_ENABLED(iface)) {

src/uct/ib/mlx5/ud/ud_mlx5_common.c

@@ -45,8 +45,12 @@ uct_ud_mlx5_iface_get_av(uct_ib_iface_t *iface,
     struct ibv_ah_attr  ah_attr;
     enum ibv_mtu        path_mtu;
 
-    uct_ib_iface_fill_ah_attr_from_addr(iface, ib_addr, path_index, &ah_attr,
-                                        &path_mtu);
+    status = uct_ib_iface_fill_ah_attr_from_addr(iface, ib_addr, path_index,
+                                                 &ah_attr, &path_mtu);
+    if (status != UCS_OK) {
+        return status;
+    }
+
     status = uct_ib_iface_create_ah(iface, &ah_attr, usage, &ah);
     if (status != UCS_OK) {
         return status;

src/uct/ib/rc/verbs/rc_verbs_ep.c

@@ -643,9 +643,13 @@ uct_rc_verbs_ep_connect_to_ep_v2(uct_ep_h tl_ep,
     struct ibv_ah_attr ah_attr;
     enum ibv_mtu path_mtu;
 
-    uct_ib_iface_fill_ah_attr_from_addr(&iface->super, ib_addr,
-                                        ep->super.path_index, &ah_attr,
-                                        &path_mtu);
+    status = uct_ib_iface_fill_ah_attr_from_addr(&iface->super, ib_addr,
+                                                 ep->super.path_index, &ah_attr,
+                                                 &path_mtu);
+    if (status != UCS_OK) {
+        return status;
+    }
+
     ucs_assert(path_mtu != UCT_IB_ADDRESS_INVALID_PATH_MTU);
 
     qp_num = uct_ib_unpack_uint24(rc_addr->super.qp_num);

src/uct/ib/ud/verbs/ud_verbs.c

@@ -597,8 +597,12 @@ uct_ud_verbs_iface_unpack_peer_address(uct_ud_iface_t *iface,
 
     memset(peer_address, 0, sizeof(*peer_address));
 
-    uct_ib_iface_fill_ah_attr_from_addr(ib_iface, ib_addr, path_index,
-                                        &ah_attr, &path_mtu);
+    status = uct_ib_iface_fill_ah_attr_from_addr(ib_iface, ib_addr, path_index,
+                                                 &ah_attr, &path_mtu);
+    if (status != UCS_OK) {
+        return status;
+    }
+
     status = uct_ib_iface_create_ah(ib_iface, &ah_attr, "UD verbs connect",
                                     &peer_address->ah);
     if (status != UCS_OK) {
@@ -633,8 +637,12 @@ int uct_ud_verbs_ep_is_connected(const uct_ep_h tl_ep,
     }
 
     ib_addr = (uct_ib_address_t*)params->device_addr;
-    uct_ib_iface_fill_ah_attr_from_addr(ib_iface, ib_addr, ep->super.path_index,
-                                        &ah_attr, &path_mtu);
+    status  = uct_ib_iface_fill_ah_attr_from_addr(ib_iface, ib_addr,
+                                                  ep->super.path_index, &ah_attr,
+                                                  &path_mtu);
+    if (status != UCS_OK) {
+        return 0;
+    }
 
     status = uct_ib_device_get_ah_cached(uct_ib_iface_device(ib_iface),
                                          &ah_attr, &ah);

test/gtest/uct/ib/test_ib.cc

@@ -109,7 +109,7 @@ class test_uct_ib_addr : public test_uct_ib {
         uct_ib_address_pack(&pack_params, ib_addr);
 
         uct_ib_address_pack_params_t unpack_params;
-        uct_ib_address_unpack(ib_addr, &unpack_params);
+        ASSERT_UCS_OK(uct_ib_address_unpack(ib_addr, &unpack_params));
 
         if (uct_ib_iface_is_roce(iface)) {
             EXPECT_TRUE(iface->config.force_global_addr);
@@ -202,7 +202,7 @@ UCS_TEST_P(test_uct_ib_addr, address_pack_path_mtu, "IB_PATH_MTU=2048")
     uct_ib_address_t *addr = (uct_ib_address_t*)&buffer[0];
     uct_ib_iface_address_pack(iface, addr);
     uct_ib_address_pack_params_t params;
-    uct_ib_address_unpack(addr, &params);
+    ASSERT_UCS_OK(uct_ib_address_unpack(addr, &params));
     EXPECT_TRUE(params.flags & UCT_IB_ADDRESS_PACK_FLAG_PATH_MTU);
     EXPECT_EQ(IBV_MTU_2048, params.path_mtu);
 }
@@ -464,7 +464,7 @@ class test_uct_ib_sl : public test_uct_ib_with_specific_port {
                                    UCT_IB_MLX5_DP_ORDERING_OOO_ALL);
         return rc_has_ddp || dc_has_ddp;
     }
-    
+
     void test_check_ib_sl_config() {
         const char *max_avail_sl_str = getenv("GTEST_MAX_IB_SL");
         uint8_t sl, max_avail_sl;

test/gtest/uct/ib/test_ib_pkey.cc

@@ -135,7 +135,7 @@ class test_uct_ib_pkey : public test_uct_ib_with_specific_port {
         uct_ib_address_pack_params_t params;
 
         uct_ib_iface_address_pack(iface, ib_addr);
-        uct_ib_address_unpack(ib_addr, &params);
+        ASSERT_UCS_OK(uct_ib_address_unpack(ib_addr, &params));
         EXPECT_TRUE(params.flags & UCT_IB_ADDRESS_PACK_FLAG_PKEY);
         EXPECT_EQ(m_pkey[0], params.pkey);
 

test/gtest/uct/ib/test_ud_ds.cc

@@ -107,8 +107,8 @@ unsigned test_ud_ds::N = 1000;
 UCS_TEST_P(test_ud_ds, if_addr) {
     uct_ib_address_pack_params_t unpack_params1, unpack_params2;
 
-    uct_ib_address_unpack(ib_adr1, &unpack_params1);
-    uct_ib_address_unpack(ib_adr2, &unpack_params2);
+    ASSERT_UCS_OK(uct_ib_address_unpack(ib_adr1, &unpack_params1));
+    ASSERT_UCS_OK(uct_ib_address_unpack(ib_adr2, &unpack_params2));
     EXPECT_EQ(unpack_params1.lid, unpack_params2.lid);
     EXPECT_EQ(unpack_params1.gid.global.subnet_prefix,
               unpack_params2.gid.global.subnet_prefix);