On Wed, Jan 29, 2020 at 02:43:51PM +0200, Gal Pressman wrote: > On 29/01/2020 14:14, Maor Gottlieb wrote: > > > > On 1/29/2020 2:06 PM, Gal Pressman wrote: > >> On 26/01/2020 19:15, Leon Romanovsky wrote: > >>> From: Maor Gottlieb <maorg@xxxxxxxxxxxx> > >>> > >>> We don't need to set pkey as valid in case that user set only one > >>> of pkey index or port number, otherwise it will be resulted in NULL > >>> pointer dereference while accessing to uninitialized pkey list. > >> Why would the pkey list be uninitialized? Isn't it initialized as an empty list > >> on device registration? > > > > It will try to access to list of invalid port / pkey, e.g. to list of > > port 0. port_data is indexed by port number. > > dev->port_data[pp->port_num].pkey_list > > Makes sense. > Shouldn't there be a check in the (!qp_pps) section as well? We shouldn't assign > the field unless the mask is given. Indeed, reading a qp_attr field without the corresponding bt in qp_attr_mask set should be wrong. > Does this work correctly if the user issues two calls to modify_qp where the > first one modifies the pkey index and the second the port number (if that's even > possible)? > Is it expected that the state would stay invalid? Also sounds wrong .. and then there is the confusing testing of state != IB_PORT_PKEY_NOT_VALID but nothing ever assigns IB_PORT_PKEY_NOT_VALID.. Humm. Jason
