| Message ID | YqCoWrAXeGHPvTfO@kili (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | RDMA/erdma: signedness bug in erdma_request_vectors() | expand |
On 6/8/22 9:47 PM, Dan Carpenter wrote: > The dev->attrs.irq_num variable is a u32 so the error handling won't > work. In this code we are passing "1" as the minimum number of IRQs and > if it cannot allocate the minimum number of IRQs then the function > returns -ENOSPC. This means that it cannot return 0 here. > > Fix the signedness bug by using a "int ret;" and preserve the error > code instead of always returning -ENOSPC. > > Fixes: d4d7a22521c9 ("RDMA/erdma: Add the erdma module") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Since this bug is found in the initial upstream patch of our new driver, I would squash your changes into the relevant patch in our next version patch set to make the commit history cleaner. Thanks, Cheng Xu
On Thu, Jun 09, 2022 at 11:00:03AM +0800, Cheng Xu wrote: > > > On 6/8/22 9:47 PM, Dan Carpenter wrote: > > The dev->attrs.irq_num variable is a u32 so the error handling won't > > work. In this code we are passing "1" as the minimum number of IRQs and > > if it cannot allocate the minimum number of IRQs then the function > > returns -ENOSPC. This means that it cannot return 0 here. > > > > Fix the signedness bug by using a "int ret;" and preserve the error > > code instead of always returning -ENOSPC. > > > > Fixes: d4d7a22521c9 ("RDMA/erdma: Add the erdma module") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > Since this bug is found in the initial upstream patch of our new > driver, I would squash your changes into the relevant patch in our next > version patch set to make the commit history cleaner. Sure, no problem. regards, dan carpenter
diff --git a/drivers/infiniband/hw/erdma/erdma_main.c b/drivers/infiniband/hw/erdma/erdma_main.c index 7c6728aebff0..078bb1c8c9bf 100644 --- a/drivers/infiniband/hw/erdma/erdma_main.c +++ b/drivers/infiniband/hw/erdma/erdma_main.c @@ -185,14 +185,15 @@ static void erdma_dwqe_resource_init(struct erdma_dev *dev) static int erdma_request_vectors(struct erdma_dev *dev) { int expect_irq_num = min(num_possible_cpus() + 1, ERDMA_NUM_MSIX_VEC); + int ret; - dev->attrs.irq_num = pci_alloc_irq_vectors(dev->pdev, 1, expect_irq_num, - PCI_IRQ_MSIX); - if (dev->attrs.irq_num <= 0) { + ret = pci_alloc_irq_vectors(dev->pdev, 1, expect_irq_num, PCI_IRQ_MSIX); + if (ret < 0) { dev_err(&dev->pdev->dev, "request irq vectors failed(%d)\n", dev->attrs.irq_num); - return -ENOSPC; + return ret; } + dev->attrs.irq_num = ret; return 0; }
The dev->attrs.irq_num variable is a u32 so the error handling won't work. In this code we are passing "1" as the minimum number of IRQs and if it cannot allocate the minimum number of IRQs then the function returns -ENOSPC. This means that it cannot return 0 here. Fix the signedness bug by using a "int ret;" and preserve the error code instead of always returning -ENOSPC. Fixes: d4d7a22521c9 ("RDMA/erdma: Add the erdma module") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/infiniband/hw/erdma/erdma_main.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)