Re: [PATCH rdma-next 6/8] RDMA/netlink: Protect device query from device removal

Jason Gunthorpe
2017-12-27T23:05:52Z

On Sun, Dec 24, 2017 at 03:43:26PM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> There is a chance that device will be removed during device query
> operations and it will cause to kernel panic in the flows which
> doesn't hold lists_rwsem semaphore.
>
> Fixes: e5c9469efcb1 ("RDMA/netlink: Add nldev device doit implementation")
> Reviewed-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> drivers/infiniband/core/nldev.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> index 2b631307349d..e3033d7a4029 100644
> +++ b/drivers/infiniband/core/nldev.c
> @@ -141,36 +141,41 @@ static int nldev_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
> struct ib_device *device;
> struct sk_buff *msg;
> u32 index;
> - int err;
> + int ret = -ENOMEM;
>
> - err = nlmsg_parse(nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1,
> + ret = nlmsg_parse(nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1,
> nldev_policy, extack);
> - if (err || !tb[RDMA_NLDEV_ATTR_DEV_INDEX])
> + if (ret || !tb[RDMA_NLDEV_ATTR_DEV_INDEX])
> return -EINVAL;
>
> index = nla_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
>
> - device = __ib_device_get_by_index(index);

It is not possible to correctly use __ib_device_get_by_index without
grabbing one of the locks which are static to device.c

Thus it should not be in core_priv.h and every single user must be
wrong, please fix them all here and make it static again.

Jason