Re: [PATCH rdma-next] RDMA/core: Fix protection fault in get_pkey_idx_qp_list

[Jason Gunthorpe <jgg@mellanox.com>]

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@mellanox.com>
> >>>
> >>> 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