Skip to contentSkip to conversation

Decode the SAPKeyTableRecord block number properly, and don't segv

PATCH/opensm Jason Gunthorpe 3 participants 9 messages Feb 11, 2011 – Feb 25, 2011 5 messages not in the archive InfiniBand

lore
Share & tools
lore

Jason Gunthorpe

author
#0 lore raw

* Properly byteswap block_num before using it
* Properly check bounds of the block number before de-referencing it.

This fixes a remotely triggered null pointer dereference.

---
 opensm/include/iba/ib_types.h      |    2 +-
 opensm/opensm/osm_sa_pkey_record.c |   16 ++++++++++++----
 2 files changed, 13 insertions(+), 5 deletions(-)
opensm/include/iba/ib_types.h +1 -1
index e1bc102..24f5662 100644
@@ -6724,7 +6724,7 @@ typedef struct _ib_pkey_table {
 #include <complib/cl_packon.h>
 typedef struct _ib_pkey_table_record {
 	ib_net16_t lid;		// for CA: lid of port, for switch lid of port 0
-	uint16_t block_num;
+	ib_net16_t block_num;
 	uint8_t port_num;	// for switch: port number, for CA: reserved
 	uint8_t reserved1;
 	uint16_t reserved2;
opensm/opensm/osm_sa_pkey_record.c +12 -4
index e4930d0..cf50430 100644
@@ -71,6 +71,7 @@ static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp,
 	osm_pkey_item_t *p_rec_item;
 	uint16_t lid;
 	ib_api_status_t status = IB_SUCCESS;
+	ib_pkey_table_t *tbl;
 
 	OSM_LOG_ENTER(sa->p_log);
 

@@ -98,8 +99,15 @@ static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp,
 	p_rec_item->rec.lid = lid;
 	p_rec_item->rec.block_num = block;
 	p_rec_item->rec.port_num = osm_physp_get_port_num(p_physp);
-	p_rec_item->rec.pkey_tbl =
-	    *(osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp), block));
+	/* FIXME: There are ninf.PartitionCap or swinf.PartitionEnforcementCap
+	   pkey entries so everything in that range is a valid block number
+	   even if opensm is not using it. Return 0. However things outside
+	   that range should return no entries.. Not sure how to figure that
+	   here? The range of pkey_tbl can be less than the cap, so
+	   this falsely triggers. */
+	tbl = osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp), block);
+	if (tbl)
+		p_rec_item->rec.pkey_tbl = *tbl;
 
 	cl_qlist_insert_tail(p_ctxt->p_list, &p_rec_item->list_item);
 

@@ -269,14 +277,14 @@ void osm_pkey_rec_rcv_process(IN void *ctx, IN void *data)
 	context.p_list = &rec_list;
 	context.comp_mask = p_rcvd_mad->comp_mask;
 	context.sa = sa;
-	context.block_num = p_rcvd_rec->block_num;
+	context.block_num = cl_ntoh16(p_rcvd_rec->block_num);
 	context.p_req_physp = p_req_physp;
 
 	OSM_LOG(sa->p_log, OSM_LOG_DEBUG,
 		"Got Query Lid:%u(%02X), Block:0x%02X(%02X), Port:0x%02X(%02X)\n",
 		cl_ntoh16(p_rcvd_rec->lid),
 		(comp_mask & IB_PKEY_COMPMASK_LID) != 0, p_rcvd_rec->port_num,
-		(comp_mask & IB_PKEY_COMPMASK_PORT) != 0, p_rcvd_rec->block_num,
+		(comp_mask & IB_PKEY_COMPMASK_PORT) != 0, context.block_num,
 		(comp_mask & IB_PKEY_COMPMASK_BLOCK) != 0);
 
 	cl_plock_acquire(sa->p_lock);
Signature · 6 lines
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20110211221206.GA8532-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org

#2 lore raw

Hi Jason,

Quoted 14 lines · Jason Gunthorpe

* Properly byteswap block_num before using it
* Properly check bounds of the block number before de-referencing it.

This fixes a remotely triggered null pointer dereference.

---
 opensm/include/iba/ib_types.h      |    2 +-
 opensm/opensm/osm_sa_pkey_record.c |   16 ++++++++++++----
 2 files changed, 13 insertions(+), 5 deletions(-)
opensm/include/iba/ib_types.h +0 -0
index e1bc102..24f5662 100644

opensm/include/iba/ib_types.h typedef struct _ib_pkey_table { · line 6730

Quoted 8 lines
opensm/include/iba/ib_types.h +1 -1
@@ -6724,7 +6724,7 @@ typedef struct _ib_pkey_table {
 #include <complib/cl_packon.h>
 typedef struct _ib_pkey_table_record {
 	ib_net16_t lid;		// for CA: lid of port, for switch lid of port 0
-	uint16_t block_num;
+	ib_net16_t block_num;
 	uint8_t port_num;	// for switch: port number, for CA: reserved
 	uint8_t reserved1;
 	uint16_t reserved2;

I guess reserved2 could also be changed to ib_net16_t, right?

Quoted 19 lines
opensm/opensm/osm_sa_pkey_record.c +2 -2
index e4930d0..cf50430 100644
@@ -71,6 +71,7 @@ static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp,
 	osm_pkey_item_t *p_rec_item;
 	uint16_t lid;
 	ib_api_status_t status = IB_SUCCESS;
+	ib_pkey_table_t *tbl;
 
 	OSM_LOG_ENTER(sa->p_log);
 

@@ -98,8 +99,15 @@ static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp,
 	p_rec_item->rec.lid = lid;
 	p_rec_item->rec.block_num = block;
 	p_rec_item->rec.port_num = osm_physp_get_port_num(p_physp);
-	p_rec_item->rec.pkey_tbl =
-	    *(osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp), block));
+	/* FIXME: There are ninf.PartitionCap or swinf.PartitionEnforcementCap

opensm/opensm/osm_sa_pkey_record.c static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp, · line 112

Quoted 10 lines
opensm/opensm/osm_sa_pkey_record.c +8 -0
+	   pkey entries so everything in that range is a valid block number
+	   even if opensm is not using it. Return 0. However things outside
+	   that range should return no entries.. Not sure how to figure that
+	   here? The range of pkey_tbl can be less than the cap, so
+	   this falsely triggers. */
+	tbl = osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp), block);
+	if (tbl)
+		p_rec_item->rec.pkey_tbl = *tbl;
 
 	cl_qlist_insert_tail(p_ctxt->p_list, &p_rec_item->list_item);

What is the expected behaviour when IB PKey table block is empty? rec.pkey_tbl might be uninitialized here.
Shouldn't SubnAdmGetResp contain ERR_NO_RECORDS in such case?

Quoted 21 lines
opensm/opensm/osm_sa_pkey_record.c +2 -2
@@ -269,14 +277,14 @@ void osm_pkey_rec_rcv_process(IN void *ctx, IN void *data)
 	context.p_list = &rec_list;
 	context.comp_mask = p_rcvd_mad->comp_mask;
 	context.sa = sa;
-	context.block_num = p_rcvd_rec->block_num;
+	context.block_num = cl_ntoh16(p_rcvd_rec->block_num);
 	context.p_req_physp = p_req_physp;
 
 	OSM_LOG(sa->p_log, OSM_LOG_DEBUG,
 		"Got Query Lid:%u(%02X), Block:0x%02X(%02X), Port:0x%02X(%02X)\n",
 		cl_ntoh16(p_rcvd_rec->lid),
 		(comp_mask & IB_PKEY_COMPMASK_LID) != 0, p_rcvd_rec->port_num,
-		(comp_mask & IB_PKEY_COMPMASK_PORT) != 0, p_rcvd_rec->block_num,
+		(comp_mask & IB_PKEY_COMPMASK_PORT) != 0, context.block_num,
 		(comp_mask & IB_PKEY_COMPMASK_BLOCK) != 0);
 
 	cl_plock_acquire(sa->p_lock);
Signature · 1 line
1.7.1

Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20110223172019.GA8537-iQai9MGU/dyyaiaB+Ve85laTQe2KTcn/@public.gmane.org

author
#4 lore raw
Quoted 1 line · Alex Netes

I guess reserved2 could also be changed to ib_net16_t, right?

Sure, but unrelated, might be lots more too?

Quoted 27 lines
Quoted 27 lines
opensm/opensm/osm_sa_pkey_record.c +11 -2
index e4930d0..cf50430 100644
+++ b/opensm/opensm/osm_sa_pkey_record.c

@@ -71,6 +71,7 @@ static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp,
 	osm_pkey_item_t *p_rec_item;
 	uint16_t lid;
 	ib_api_status_t status = IB_SUCCESS;
+	ib_pkey_table_t *tbl;
 
 	OSM_LOG_ENTER(sa->p_log);
 

@@ -98,8 +99,15 @@ static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp,
 	p_rec_item->rec.lid = lid;
 	p_rec_item->rec.block_num = block;
 	p_rec_item->rec.port_num = osm_physp_get_port_num(p_physp);
-	p_rec_item->rec.pkey_tbl =
-	    *(osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp), block));
+	/* FIXME: There are ninf.PartitionCap or swinf.PartitionEnforcementCap
+	   pkey entries so everything in that range is a valid block number
+	   even if opensm is not using it. Return 0. However things outside
+	   that range should return no entries.. Not sure how to figure that
+	   here? The range of pkey_tbl can be less than the cap, so
+	   this falsely triggers. */
+	tbl = osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp), block);
+	if (tbl)
+		p_rec_item->rec.pkey_tbl = *tbl;
Quoted 3 lines

opensm/opensm/osm_sa_pkey_record.c static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp, · line 112

Quoted 1 line
opensm/opensm/osm_sa_pkey_record.c +0 -0
 	cl_qlist_insert_tail(p_ctxt->p_list, &p_rec_item->list_item);

What is the expected behaviour when IB PKey table block is empty?

IBA is unclear in this case. In my view, direct queries for SA records should return the same result as a direct SMP for the same data, so returning a wack of 0 is appropriate. Another view would be that the SA data is 'empty' so ERR_NO_RECORDS is OK, which matches what GetTable would return.

Quoted 1 line

rec.pkey_tbl might be uninitialized here.

All of p_rec_item is memset to 0.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw
In reply to a message not in the archive
Quoted 56 lines · Alex Netes

Hi Jason,

Quoted 16 lines · Jason Gunthorpe

* Properly byteswap block_num before using it
* Properly check bounds of the block number before de-referencing it.

This fixes a remotely triggered null pointer dereference.
---
 opensm/include/iba/ib_types.h      |    2 +- opensm/opensm/osm_sa_pkey_record.c |   16 ++++++++++++----

 2 files changed, 13 insertions(+), 5 deletions(-)
opensm/include/iba/ib_types.h +0 -0
index e1bc102..24f5662 100644
@@ -6724,7 +6724,7 @@ typedef struct _ib_pkey_table {
  #include <complib/cl_packon.h>
  typedef struct _ib_pkey_table_record {
Quoted 6 lines

      ib_net16_t lid;         // for CA: lid of port, for switch lid of port 0
-     uint16_t block_num;
+     ib_net16_t block_num;
      uint8_t port_num;       // for switch: port number, for CA: reserved uint8_t reserved1;
      uint16_t reserved2;

I guess reserved2 could also be changed to ib_net16_t, right?

Quoted 28 lines
opensm/opensm/osm_sa_pkey_record.c +10 -2
index e4930d0..cf50430 100644
@@ -71,6 +71,7 @@ static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp,
       osm_pkey_item_t *p_rec_item;
       uint16_t lid;
       ib_api_status_t status = IB_SUCCESS;
+     ib_pkey_table_t *tbl;
 
       OSM_LOG_ENTER(sa->p_log);
 

@@ -98,8 +99,15 @@ static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp,
       p_rec_item->rec.lid = lid;
       p_rec_item->rec.block_num = block;
       p_rec_item->rec.port_num = osm_physp_get_port_num(p_physp);
-     p_rec_item->rec.pkey_tbl =
-         *(osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp), block));
+     /* FIXME: There are ninf.PartitionCap or swinf.PartitionEnforcementCap
+        pkey entries so everything in that range is a valid block number
+        even if opensm is not using it. Return 0. However things outside
+        that range should return no entries.. Not sure how to figure that
+        here? The range of pkey_tbl can be less than the cap, so
+        this falsely triggers. */
+     tbl = osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp), block);
+     if (tbl)
+             p_rec_item->rec.pkey_tbl = *tbl;
Quoted 5 lines

opensm/opensm/osm_sa_pkey_record.c static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp, · line 112

Quoted 1 line
opensm/opensm/osm_sa_pkey_record.c +0 -0
       cl_qlist_insert_tail(p_ctxt->p_list, &p_rec_item->list_item);

What is the expected behaviour when IB PKey table block is empty? rec.pkey_tbl might be uninitialized here.
Shouldn't SubnAdmGetResp contain ERR_NO_RECORDS in such case?

It depends on the method used in the SA query. GetTable never returns ERR_NO_RECORDS whereas a Get can.

-- Hal

Quoted 17 lines
Quoted 17 lines
opensm/opensm/osm_sa_pkey_record.c +2 -2
@@ -269,14 +277,14 @@ void osm_pkey_rec_rcv_process(IN void *ctx, IN void *data)
       context.p_list = &rec_list;
       context.comp_mask = p_rcvd_mad->comp_mask;
       context.sa = sa;
-     context.block_num = p_rcvd_rec->block_num;
+     context.block_num = cl_ntoh16(p_rcvd_rec->block_num);
       context.p_req_physp = p_req_physp;
 
       OSM_LOG(sa->p_log, OSM_LOG_DEBUG,
               "Got Query Lid:%u(%02X), Block:0x%02X(%02X), Port:0x%02X(%02X)\n",
               cl_ntoh16(p_rcvd_rec->lid),
               (comp_mask & IB_PKEY_COMPMASK_LID) != 0, p_rcvd_rec->port_num,
-             (comp_mask & IB_PKEY_COMPMASK_PORT) != 0, p_rcvd_rec->block_num,
+             (comp_mask & IB_PKEY_COMPMASK_PORT) != 0, context.block_num,
               (comp_mask & IB_PKEY_COMPMASK_BLOCK) != 0);
Quoted 11 lines
Quoted 4 lines
opensm/opensm/osm_sa_pkey_record.c +0 -1
       cl_plock_acquire(sa->p_lock);
--

1.7.1

Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): AANLkTinm5Uh-9h+WMh6O8dLsPr8wpdt0BJmOu3+Dt8vj-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#7 lore raw
Quoted 57 lines · Hal Rosenstock
Quoted 56 lines · Alex Netes

Hi Jason,

Quoted 16 lines · Jason Gunthorpe

* Properly byteswap block_num before using it
* Properly check bounds of the block number before de-referencing it.

This fixes a remotely triggered null pointer dereference.
---
 opensm/include/iba/ib_types.h      |    2 +- opensm/opensm/osm_sa_pkey_record.c |   16 ++++++++++++----

 2 files changed, 13 insertions(+), 5 deletions(-)
opensm/include/iba/ib_types.h +0 -0
index e1bc102..24f5662 100644
@@ -6724,7 +6724,7 @@ typedef struct _ib_pkey_table {
  #include <complib/cl_packon.h>
  typedef struct _ib_pkey_table_record {
Quoted 6 lines

      ib_net16_t lid;         // for CA: lid of port, for switch lid of port 0
-     uint16_t block_num;
+     ib_net16_t block_num;
      uint8_t port_num;       // for switch: port number, for CA: reserved uint8_t reserved1;
      uint16_t reserved2;

I guess reserved2 could also be changed to ib_net16_t, right?

Quoted 28 lines
opensm/opensm/osm_sa_pkey_record.c +10 -2
index e4930d0..cf50430 100644
@@ -71,6 +71,7 @@ static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp,
       osm_pkey_item_t *p_rec_item;
       uint16_t lid;
       ib_api_status_t status = IB_SUCCESS;
+     ib_pkey_table_t *tbl;
 
       OSM_LOG_ENTER(sa->p_log);
 

@@ -98,8 +99,15 @@ static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp,
       p_rec_item->rec.lid = lid;
       p_rec_item->rec.block_num = block;
       p_rec_item->rec.port_num = osm_physp_get_port_num(p_physp);
-     p_rec_item->rec.pkey_tbl =
-         *(osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp), block));
+     /* FIXME: There are ninf.PartitionCap or swinf.PartitionEnforcementCap
+        pkey entries so everything in that range is a valid block number
+        even if opensm is not using it. Return 0. However things outside
+        that range should return no entries.. Not sure how to figure that
+        here? The range of pkey_tbl can be less than the cap, so
+        this falsely triggers. */
+     tbl = osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp), block);
+     if (tbl)
+             p_rec_item->rec.pkey_tbl = *tbl;
Quoted 8 lines
Quoted 5 lines

opensm/opensm/osm_sa_pkey_record.c static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp, · line 112

Quoted 1 line
opensm/opensm/osm_sa_pkey_record.c +0 -0
       cl_qlist_insert_tail(p_ctxt->p_list, &p_rec_item->list_item);

What is the expected behaviour when IB PKey table block is empty? rec.pkey_tbl might be uninitialized here.
Shouldn't SubnAdmGetResp contain ERR_NO_RECORDS in such case?

It depends on the method used in the SA query. GetTable never returns ERR_NO_RECORDS whereas a Get can.

I'm referring to a block that is fully above the PartitionCap in the above. For a block included by the PartitionCap, there's should always be a "valid" block to be returned even if the entries indicate invalid pkey.

-- Hal

Quoted 32 lines

-- Hal

Quoted 28 lines
Quoted 19 lines
opensm/opensm/osm_sa_pkey_record.c +2 -3
@@ -269,14 +277,14 @@ void osm_pkey_rec_rcv_process(IN void *ctx, IN void *data)
       context.p_list = &rec_list;
       context.comp_mask = p_rcvd_mad->comp_mask;
       context.sa = sa;
-     context.block_num = p_rcvd_rec->block_num;
+     context.block_num = cl_ntoh16(p_rcvd_rec->block_num);
       context.p_req_physp = p_req_physp;
 
       OSM_LOG(sa->p_log, OSM_LOG_DEBUG,
               "Got Query Lid:%u(%02X), Block:0x%02X(%02X), Port:0x%02X(%02X)\n",
               cl_ntoh16(p_rcvd_rec->lid),
               (comp_mask & IB_PKEY_COMPMASK_LID) != 0, p_rcvd_rec->port_num,
-             (comp_mask & IB_PKEY_COMPMASK_PORT) != 0, p_rcvd_rec->block_num,
+             (comp_mask & IB_PKEY_COMPMASK_PORT) != 0, context.block_num,
               (comp_mask & IB_PKEY_COMPMASK_BLOCK) != 0);
 
       cl_plock_acquire(sa->p_lock);
--
Quoted 2 lines

1.7.1

Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#8 lore raw
In reply to a message not in the archive
Quoted 57 lines · Hal Rosenstock
Quoted 56 lines · Alex Netes

Hi Jason,

Quoted 16 lines · Jason Gunthorpe

* Properly byteswap block_num before using it
* Properly check bounds of the block number before de-referencing it.

This fixes a remotely triggered null pointer dereference.
---
 opensm/include/iba/ib_types.h      |    2 +- opensm/opensm/osm_sa_pkey_record.c |   16 ++++++++++++----

 2 files changed, 13 insertions(+), 5 deletions(-)
opensm/include/iba/ib_types.h +0 -0
index e1bc102..24f5662 100644
@@ -6724,7 +6724,7 @@ typedef struct _ib_pkey_table {
  #include <complib/cl_packon.h>
  typedef struct _ib_pkey_table_record {
Quoted 6 lines

      ib_net16_t lid;         // for CA: lid of port, for switch lid of port 0
-     uint16_t block_num;
+     ib_net16_t block_num;
      uint8_t port_num;       // for switch: port number, for CA: reserved uint8_t reserved1;
      uint16_t reserved2;

I guess reserved2 could also be changed to ib_net16_t, right?

Quoted 28 lines
opensm/opensm/osm_sa_pkey_record.c +10 -2
index e4930d0..cf50430 100644
@@ -71,6 +71,7 @@ static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp,
       osm_pkey_item_t *p_rec_item;
       uint16_t lid;
       ib_api_status_t status = IB_SUCCESS;
+     ib_pkey_table_t *tbl;
 
       OSM_LOG_ENTER(sa->p_log);
 

@@ -98,8 +99,15 @@ static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp,
       p_rec_item->rec.lid = lid;
       p_rec_item->rec.block_num = block;
       p_rec_item->rec.port_num = osm_physp_get_port_num(p_physp);
-     p_rec_item->rec.pkey_tbl =
-         *(osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp), block));
+     /* FIXME: There are ninf.PartitionCap or swinf.PartitionEnforcementCap
+        pkey entries so everything in that range is a valid block number
+        even if opensm is not using it. Return 0. However things outside
+        that range should return no entries.. Not sure how to figure that
+        here? The range of pkey_tbl can be less than the cap, so
+        this falsely triggers. */
+     tbl = osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp), block);
+     if (tbl)
+             p_rec_item->rec.pkey_tbl = *tbl;
Quoted 9 lines
Quoted 5 lines

opensm/opensm/osm_sa_pkey_record.c static void sa_pkey_create(IN osm_sa_t * sa, IN osm_physp_t * p_physp, · line 112

Quoted 1 line
opensm/opensm/osm_sa_pkey_record.c +0 -0
       cl_qlist_insert_tail(p_ctxt->p_list, &p_rec_item->list_item);

What is the expected behaviour when IB PKey table block is empty? rec.pkey_tbl might be uninitialized here.
Shouldn't SubnAdmGetResp contain ERR_NO_RECORDS in such case?

It depends on the method used in the SA query. GetTable never returns ERR_NO_RECORDS whereas a Get can.

In case when IB PKey table block is empty, p_rec_item->rec.pkey_tbl would be uninitialized.

Quoted 30 lines

-- Hal

Quoted 28 lines
Quoted 19 lines
opensm/opensm/osm_sa_pkey_record.c +2 -3
@@ -269,14 +277,14 @@ void osm_pkey_rec_rcv_process(IN void *ctx, IN void *data)
       context.p_list = &rec_list;
       context.comp_mask = p_rcvd_mad->comp_mask;
       context.sa = sa;
-     context.block_num = p_rcvd_rec->block_num;
+     context.block_num = cl_ntoh16(p_rcvd_rec->block_num);
       context.p_req_physp = p_req_physp;
 
       OSM_LOG(sa->p_log, OSM_LOG_DEBUG,
               "Got Query Lid:%u(%02X), Block:0x%02X(%02X), Port:0x%02X(%02X)\n",
               cl_ntoh16(p_rcvd_rec->lid),
               (comp_mask & IB_PKEY_COMPMASK_LID) != 0, p_rcvd_rec->port_num,
-             (comp_mask & IB_PKEY_COMPMASK_PORT) != 0, p_rcvd_rec->block_num,
+             (comp_mask & IB_PKEY_COMPMASK_PORT) != 0, context.block_num,
               (comp_mask & IB_PKEY_COMPMASK_BLOCK) != 0);
 
       cl_plock_acquire(sa->p_lock);
--
Quoted 2 lines

1.7.1

Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20110225112514.GC8537-iQai9MGU/dyyaiaB+Ve85laTQe2KTcn/@public.gmane.org

author
#10 lore raw
Quoted 8 lines · Alex Netes
Quoted 6 lines
Quoted 3 lines

What is the expected behaviour when IB PKey table block is empty? rec.pkey_tbl might be uninitialized here.
Shouldn't SubnAdmGetResp contain ERR_NO_RECORDS in such case?

It depends on the method used in the SA query. GetTable never returns ERR_NO_RECORDS whereas a Get can.

In case when IB PKey table block is empty, p_rec_item->rec.pkey_tbl would be uninitialized.

No, it isn't, it is zero.

        memset(p_rec_item, 0, sizeof(*p_rec_item));

        p_rec_item->rec.lid = lid;
        p_rec_item->rec.block_num = block;
        p_rec_item->rec.port_num = osm_physp_get_port_num(p_physp);
        p_rec_item->rec.pkey_tbl =
            *(osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp),
	    block));

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20110225193509.GA9157-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org

#12 lore raw
Quoted 23 lines · Jason Gunthorpe
Quoted 8 lines · Alex Netes
Quoted 6 lines
Quoted 3 lines

What is the expected behaviour when IB PKey table block is empty? rec.pkey_tbl might be uninitialized here.
Shouldn't SubnAdmGetResp contain ERR_NO_RECORDS in such case?

It depends on the method used in the SA query. GetTable never returns ERR_NO_RECORDS whereas a Get can.

In case when IB PKey table block is empty, p_rec_item->rec.pkey_tbl would be uninitialized.

No, it isn't, it is zero.

        memset(p_rec_item, 0, sizeof(*p_rec_item));

        p_rec_item->rec.lid = lid;
        p_rec_item->rec.block_num = block;
        p_rec_item->rec.port_num = osm_physp_get_port_num(p_physp);
        p_rec_item->rec.pkey_tbl =
            *(osm_pkey_tbl_block_get(osm_physp_get_pkey_tbl(p_physp),
	    block));
Quoted 1 line

Jason

Sorry. Missed that.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html