[ofa-general] Re: [PATCH v3 1/3] Create a new library libibnetdisc

[Ira Weiny]

Sasha,

On Fri, 24 Apr 2009 20:53:25 +0300
Sasha Khapyorsky <sashak at voltaire.com> wrote:

> On 15:42 Fri 03 Apr     , Ira Weiny wrote:
> > diff --git a/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h b/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h

[snip]

> > +	/* quick cache of switchinfo below */
> > +	int smaenhsp0;
> > +	/* use libibmad decoder functions for switchinfo */
> > +	//WHY does this not work???
> > +	//uint8_t switchinfo[sizeof (ib_switch_info_t)];
> 
> This is a right question - sizeof(ib_switch_info_t) < 64.

Ok, I missed this.  And forgot about that comment!  Thanks for fixing.  I did not experience any crashes though.  :-/  Good thing you caught this.

Ira

> 
> > +	uint8_t switchinfo[64];
> > +
> > +	/* quick cache of info below */
> > +	uint64_t guid;
> > +	int type;
> > +	int numports;
> > +	/* use libibmad decoder functions for info */
> > +	uint8_t info[sizeof(ib_node_info_t)];
> 
> Above, here and in some other places. Those buffers are used as rcvdata
> with smp_query_via(), it assumes SMP MADs and 64 bytes of data is always
> copied there. So when actual buffer is smaller bad things may happen.
> I'm fixing this with such addition:
> 
> diff --git a/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h b/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
> index a882994..bc108ab 100644
> --- a/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
> +++ b/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
> @@ -57,18 +57,16 @@ typedef struct node {
>  	/* quick cache of switchinfo below */
>  	int smaenhsp0;
>  	/* use libibmad decoder functions for switchinfo */
> -	//WHY does this not work???
> -	//uint8_t switchinfo[sizeof (ib_switch_info_t)];
> -	uint8_t switchinfo[64];
> +	uint8_t switchinfo[IB_SMP_DATA_SIZE];
>  
>  	/* quick cache of info below */
>  	uint64_t guid;
>  	int type;
>  	int numports;
>  	/* use libibmad decoder functions for info */
> -	uint8_t info[sizeof(ib_node_info_t)];
> +	uint8_t info[IB_SMP_DATA_SIZE];
>  
> -	char nodedesc[IB_NODE_DESCRIPTION_SIZE];
> +	char nodedesc[IB_SMP_DATA_SIZE];
>  
>  	struct port **ports; /* in order array of port pointers */
>  				/* the size of this array is info.numports + 1 */
> @@ -96,7 +94,7 @@ typedef struct port {
>  	uint16_t base_lid;
>  	uint8_t lmc;
>  	/* use libibmad decoder functions for info */
> -	uint8_t info[sizeof(ib_port_info_t)];
> +	uint8_t info[IB_SMP_DATA_SIZE];
>  } ibnd_port_t;
>  
>  
> diff --git a/infiniband-diags/libibnetdisc/src/ibnetdisc.c b/infiniband-diags/libibnetdisc/src/ibnetdisc.c
> index 479bae7..3fd3b76 100644
> --- a/infiniband-diags/libibnetdisc/src/ibnetdisc.c
> +++ b/infiniband-diags/libibnetdisc/src/ibnetdisc.c
> @@ -231,7 +231,7 @@ ibnd_find_node_guid(ibnd_fabric_t *fabric, uint64_t guid)
>  ibnd_node_t *
>  ibnd_update_node(ibnd_node_t *node)
>  {
> -	char portinfo_port0[sizeof (ib_port_info_t)];
> +	char portinfo_port0[IB_SMP_DATA_SIZE];
>  	void *nd = node->nodedesc;
>  	int p = 0;
>  	struct ibnd_fabric *f = CONV_FABRIC_INTERNAL(node->fabric);
> 
> Sasha


-- 
Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
weiny2 at llnl.gov