From 9660dcbe0d9186976917c94bce4e69dbd8d7a974 Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Wed, 8 Sep 2021 10:18:48 +0200 Subject: RDMA/mlx5: Fix number of allocated XLT entries In commit 8010d74b9965b ("RDMA/mlx5: Split the WR setup out of mlx5_ib_update_xlt()") the allocation logic was split out of mlx5_ib_update_xlt() and the logic was changed to enable better OOM handling. Sadly this change introduced a miscalculation of the number of entries that were actually allocated when under memory pressure where it can actually become 0 which on s390 lets dma_map_single() fail. It can also lead to corruption of the free pages list when the wrong number of entries is used in the calculation of sg->length which is used as argument for free_pages(). Fix this by using the allocation size instead of misusing get_order(size). Cc: stable@vger.kernel.org Fixes: 8010d74b9965 ("RDMA/mlx5: Split the WR setup out of mlx5_ib_update_xlt()") Link: https://lore.kernel.org/r/20210908081849.7948-1-schnelle@linux.ibm.com Signed-off-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe --- drivers/infiniband/hw/mlx5/mr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c index a520ac8ab68c..b2dd1173c98a 100644 --- a/drivers/infiniband/hw/mlx5/mr.c +++ b/drivers/infiniband/hw/mlx5/mr.c @@ -1024,7 +1024,7 @@ static void *mlx5_ib_alloc_xlt(size_t *nents, size_t ent_size, gfp_t gfp_mask) if (size > MLX5_SPARE_UMR_CHUNK) { size = MLX5_SPARE_UMR_CHUNK; - *nents = get_order(size) / ent_size; + *nents = size / ent_size; res = (void *)__get_free_pages(gfp_mask | __GFP_NOWARN, get_order(size)); if (res) -- cgit v1.2.3 From f4c6f31011eafe027abddf6cee1288a1b5a05b73 Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Wed, 8 Sep 2021 10:18:49 +0200 Subject: RDMA/mlx5: Fix xlt_chunk_align calculation The XLT chunk alignment depends on ent_size not sizeof(ent_size) aka sizeof(size_t). The incoming ent_size is either 8 or 16, so the miscalculation when 16 is required is only an over-alignment and functional harmless. Fixes: 8010d74b9965 ("RDMA/mlx5: Split the WR setup out of mlx5_ib_update_xlt()") Link: https://lore.kernel.org/r/20210908081849.7948-2-schnelle@linux.ibm.com Signed-off-by: Niklas Schnelle Signed-off-by: Jason Gunthorpe --- drivers/infiniband/hw/mlx5/mr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c index b2dd1173c98a..3be36ebbf67a 100644 --- a/drivers/infiniband/hw/mlx5/mr.c +++ b/drivers/infiniband/hw/mlx5/mr.c @@ -995,7 +995,7 @@ static struct mlx5_ib_mr *alloc_cacheable_mr(struct ib_pd *pd, static void *mlx5_ib_alloc_xlt(size_t *nents, size_t ent_size, gfp_t gfp_mask) { const size_t xlt_chunk_align = - MLX5_UMR_MTT_ALIGNMENT / sizeof(ent_size); + MLX5_UMR_MTT_ALIGNMENT / ent_size; size_t size; void *res = NULL; -- cgit v1.2.3 From 84f969e1c48ed3825986e91a0786e363d57f69d1 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Fri, 3 Sep 2021 10:07:28 -0300 Subject: IB/qib: Fix null pointer subtraction compiler warning >> drivers/infiniband/hw/qib/qib_sysfs.c:411:1: warning: performing pointer subtraction with a null pointer has undefined behavior +[-Wnull-pointer-subtraction] QIB_DIAGC_ATTR(rc_resends); ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/infiniband/hw/qib/qib_sysfs.c:408:51: note: expanded from macro 'QIB_DIAGC_ATTR' .counter = &((struct qib_ibport *)0)->rvp.n_##N - (u64 *)0, \ Use offsetof and accomplish the type check using static_assert. Fixes: 4a7aaf88c89f ("RDMA/qib: Use attributes for the port sysfs") Link: https://lore.kernel.org/r/0-v1-43ae3c759177+65-qib_type_jgg@nvidia.com Reported-by: kernel test robot Acked-by: Dennis Dalessandro Signed-off-by: Jason Gunthorpe --- drivers/infiniband/hw/qib/qib_sysfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/infiniband/hw/qib/qib_sysfs.c b/drivers/infiniband/hw/qib/qib_sysfs.c index d57e49de6650..452e2355d24e 100644 --- a/drivers/infiniband/hw/qib/qib_sysfs.c +++ b/drivers/infiniband/hw/qib/qib_sysfs.c @@ -403,9 +403,11 @@ static ssize_t diagc_attr_store(struct ib_device *ibdev, u32 port_num, } #define QIB_DIAGC_ATTR(N) \ + static_assert(&((struct qib_ibport *)0)->rvp.n_##N != (u64 *)NULL); \ static struct qib_diagc_attr qib_diagc_attr_##N = { \ .attr = __ATTR(N, 0664, diagc_attr_show, diagc_attr_store), \ - .counter = &((struct qib_ibport *)0)->rvp.n_##N - (u64 *)0, \ + .counter = \ + offsetof(struct qib_ibport, rvp.n_##N) / sizeof(u64) \ } QIB_DIAGC_ATTR(rc_resends); -- cgit v1.2.3 From f1b195ce81ad31618e9f28894a343fd62debb9ab Mon Sep 17 00:00:00 2001 From: Len Baker Date: Sun, 5 Sep 2021 10:18:12 +0200 Subject: RDMA/bnxt_re: Prefer kcalloc over open coded arithmetic As noted in the "Deprecated Interfaces, Language Features, Attributes, and Conventions" documentation [1], size calculations (especially multiplication) should not be performed in memory allocator (or similar) function arguments due to the risk of them overflowing. This could lead to values wrapping around and a smaller allocation being made than the caller was expecting. Using those allocations could lead to linear overflows of heap memory and other misbehaviors. In this case this is not actually dynamic sizes: both sides of the multiplication are constant values. However it is best to refactor this anyway, just to keep the open-coded math idiom out of code. So, use the purpose specific kcalloc() function instead of the argument size * count in the kzalloc() function. Also, remove the unnecessary initialization of the sqp_tbl variable since it is set a few lines later. [1] https://www.kernel.org/doc/html/v5.14/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments Link: https://lore.kernel.org/r/20210905081812.17113-1-len.baker@gmx.com Signed-off-by: Len Baker Reviewed-by: Leon Romanovsky Acked-by: Selvin Xavier Signed-off-by: Jason Gunthorpe --- drivers/infiniband/hw/bnxt_re/ib_verbs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c index 634d1586a1fa..525791a27a86 100644 --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c @@ -1309,7 +1309,7 @@ out: static int bnxt_re_create_shadow_gsi(struct bnxt_re_qp *qp, struct bnxt_re_pd *pd) { - struct bnxt_re_sqp_entries *sqp_tbl = NULL; + struct bnxt_re_sqp_entries *sqp_tbl; struct bnxt_re_dev *rdev; struct bnxt_re_qp *sqp; struct bnxt_re_ah *sah; @@ -1317,7 +1317,7 @@ static int bnxt_re_create_shadow_gsi(struct bnxt_re_qp *qp, rdev = qp->rdev; /* Create a shadow QP to handle the QP1 traffic */ - sqp_tbl = kzalloc(sizeof(*sqp_tbl) * BNXT_RE_MAX_GSI_SQP_ENTRIES, + sqp_tbl = kcalloc(BNXT_RE_MAX_GSI_SQP_ENTRIES, sizeof(*sqp_tbl), GFP_KERNEL); if (!sqp_tbl) return -ENOMEM; -- cgit v1.2.3 From 2169b908894df2ce83e7eb4a399d3224b2635126 Mon Sep 17 00:00:00 2001 From: chongjiapeng Date: Mon, 6 Sep 2021 17:48:43 +0800 Subject: IB/hfi1: make hist static This symbol is not used outside of trace.c, so marks it static. Fix the following sparse warning: drivers/infiniband/hw/hfi1/trace.c:491:23: warning: symbol 'hist' was not declared. Should it be static? Link: https://lore.kernel.org/r/1630921723-21545-1-git-send-email-jiapeng.chong@linux.alibaba.com Reported-by: Abaci Robot Signed-off-by: chongjiapeng Signed-off-by: Jason Gunthorpe --- drivers/infiniband/hw/hfi1/trace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/infiniband/hw/hfi1/trace.c b/drivers/infiniband/hw/hfi1/trace.c index d9b5bbb2d011..8302469582c6 100644 --- a/drivers/infiniband/hw/hfi1/trace.c +++ b/drivers/infiniband/hw/hfi1/trace.c @@ -488,7 +488,7 @@ struct hfi1_ctxt_hist { atomic_t data[255]; }; -struct hfi1_ctxt_hist hist = { +static struct hfi1_ctxt_hist hist = { .count = ATOMIC_INIT(0) }; -- cgit v1.2.3