On 14:01 Wed 23 Feb , Hal Rosenstock wrote: > On Wed, Feb 23, 2011 at 12:20 PM, Alex Netes <[email protected]> wrote: > > Hi Jason, > > > > On 15:12 Fri 11 Feb , Jason Gunthorpe wrote: > >> * 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(-) > >> > >> diff --git a/opensm/include/iba/ib_types.h b/opensm/include/iba/ib_types.h > >> index e1bc102..24f5662 100644 > >> --- a/opensm/include/iba/ib_types.h > >> +++ b/opensm/include/iba/ib_types.h > >> @@ -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? > > > >> diff --git a/opensm/opensm/osm_sa_pkey_record.c > >> b/opensm/opensm/osm_sa_pkey_record.c > >> index e4930d0..cf50430 100644 > >> --- a/opensm/opensm/osm_sa_pkey_record.c > >> +++ 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; > >> > >> 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. > -- Hal > > >> > >> @@ -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); > >> -- > >> 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
