[bug report] IB/hns: Add driver files for hns RoCE driver

[Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>]

Hello oulijun,

The patch 9a4435375cd1: "IB/hns: Add driver files for hns RoCE
driver" from Jul 21, 2016, leads to the following static checker
warning:

	drivers/infiniband/hw/hns/hns_roce_mr.c:575 hns_roce_reg_user_mr()
	warn: no lower bound on 'n'

drivers/infiniband/hw/hns/hns_roce_mr.c
   542  struct ib_mr *hns_roce_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
   543                                     u64 virt_addr, int access_flags,
   544                                     struct ib_udata *udata)
   545  {
   546          struct hns_roce_dev *hr_dev = to_hr_dev(pd->device);
   547          struct device *dev = &hr_dev->pdev->dev;
   548          struct hns_roce_mr *mr = NULL;
   549          int ret = 0;
   550          int n = 0;
                ^^^^^^^^^

Notice this is signed.  Please don't initialize variables to bogus
values.  The compiler has a feature to warn about uninitialized
variables but by assigning bogus valus to "n" then you are disabling the
safety checks and potentially hiding bugs.

   551  
   552          mr = kmalloc(sizeof(*mr), GFP_KERNEL);
   553          if (!mr)
   554                  return ERR_PTR(-ENOMEM);
   555  
   556          mr->umem = ib_umem_get(pd->uobject->context, start, length,
   557                                 access_flags, 0);
   558          if (IS_ERR(mr->umem)) {
   559                  ret = PTR_ERR(mr->umem);
   560                  goto err_free;
   561          }
   562  
   563          n = ib_umem_page_count(mr->umem);

Depending on the config then ib_umem_page_count() can return -EINVAL.
Probably it's not possible here.  Anyway, probably the right thing is
to check:

	if (n < 0) {
		ret = -EINVAL;
		goto umem;
	}

It silences the static checker warning.

   564          if (mr->umem->page_size != HNS_ROCE_HEM_PAGE_SIZE) {
   565                  dev_err(dev, "Just support 4K page size but is 0x%x now!\n",
   566                          mr->umem->page_size);

Should we continue here or is there supposed to be a goto umem;?

   567          }
   568  
   569          if (n > HNS_ROCE_MAX_MTPT_PBL_NUM) {
   570                  dev_err(dev, " MR len %lld err. MR is limited to 4G at most!\n",
   571                          length);


We should be setting "ret = -EINVAL;" here.

   572                  goto err_umem;
   573          }
   574  
   575          ret = hns_roce_mr_alloc(hr_dev, to_hr_pd(pd)->pdn, virt_addr, length,
   576                                  access_flags, n, mr);
   577          if (ret)
   578                  goto err_umem;
   579  

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html