RDMA/bnxt_re: fix a size calculation
Subsystems (3)
PATCH 1/1 RDMA/bnxt_re: fix a size calculation
This is from static analysis not from testing. Depending on the value of rcfw->cmdq_depth, then this might not cause an issue at runtime.
The BITS_TO_LONGS() macro tells us how many longs it take to hold a bitmap. In other words, it divides by the number if bits per long and rounds up. Then we want to take that number and multiple by sizeof(long) to get the number of bytes to allocate.
The code here does the multiplication first so the rounding up is done in the wrong place. So imagine we want to allocate 1 bit, then "(1 * 8) / 64 = 1" when we round up. But it should be
"(1 / 64) * 8 = 8". In other words, because of the rounding difference we might allocate up to "sizeof(long) - 1" bytes fewer than intended.
--- drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/infiniband/hw/bnxt_re/qplib_rcfw.c +1 -2
@@ -684,8 +684,7 @@ int bnxt_qplib_enable_rcfw_channel(struct pci_dev *pdev,/* General */ rcfw->seq_num =0 ;set_bit (FIRMWARE_FIRST_FLAG , &rcfw->flags); - bmap_size =BITS_TO_LONGS (rcfw->cmdq_depth * -sizeof (unsigned long )); + bmap_size =BITS_TO_LONGS (rcfw->cmdq_depth) *sizeof (unsigned long ); rcfw->cmdq_bitmap =kzalloc (bmap_size,GFP_KERNEL );if (!rcfw->cmdq_bitmap)return -ENOMEM ;
Signature · 1 line
2.17.1
Quoted 36 lines · Dan Carpenter
This is from static analysis not from testing. Depending on the value of
rcfw->cmdq_depth, then this might not cause an issue at runtime.The
BITS_TO_LONGS()macro tells us how many longs it take to hold a bitmap. In other words, it divides by the number if bits per long and rounds up. Then we want to take that number and multiple by sizeof(long) to get the number of bytes to allocate.The code here does the multiplication first so the rounding up is done in the wrong place. So imagine we want to allocate 1 bit, then "(1 * 8) / 64 = 1" when we round up. But it should be
"(1 / 64) * 8 = 8". In other words, because of the rounding difference we might allocate up to "sizeof(long) - 1" bytes fewer than intended.Fixes 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")Signed-off-by Dan Carpenter <[email protected]>--- drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/infiniband/hw/bnxt_re/qplib_rcfw.c +1
-3@@ -684,8 +684,7 @@ int bnxt_qplib_enable_rcfw_channel(struct pci_dev *pdev,/* General */ rcfw->seq_num =0 ;set_bit (FIRMWARE_FIRST_FLAG , &rcfw->flags); - bmap_size =BITS_TO_LONGS (rcfw->cmdq_depth * -sizeof (unsigned long )); + bmap_size =BITS_TO_LONGS (rcfw->cmdq_depth) *sizeof (unsigned long ); rcfw->cmdq_bitmap =kzalloc (bmap_size,GFP_KERNEL );if (!rcfw->cmdq_bitmap)return -ENOMEM ; --
Quoted 1 line
2.17.1
Thanks for supplying this fix, it was on my list for my upcoming cleanup series.
Jason Gunthorpe
AppliedQuoted 14 lines · Dan Carpenter
This is from static analysis not from testing. Depending on the value of
rcfw->cmdq_depth, then this might not cause an issue at runtime.The
BITS_TO_LONGS()macro tells us how many longs it take to hold a bitmap. In other words, it divides by the number if bits per long and rounds up. Then we want to take that number and multiple by sizeof(long) to get the number of bytes to allocate.The code here does the multiplication first so the rounding up is done in the wrong place. So imagine we want to allocate 1 bit, then "(1 * 8) / 64 = 1" when we round up. But it should be
"(1 / 64) * 8 = 8". In other words, because of the rounding difference we might allocate up to "sizeof(long) - 1" bytes fewer than intended.
Quoted 6 lines
Fixes 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")Signed-off-by Dan Carpenter <[email protected]>Acked-by Devesh Sharma <[email protected]>--- drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Applied to for-next
Thanks,
Jason