On Tue, Mar 11, 2025 at 10:05:47AM +0000, Konstantin Taranov wrote:
> > > Subject: [PATCH rdma-next 1/1] RDMA/mana_ib: Fix integer overflow
> > > during queue creation
> > >
> > > From: Konstantin Taranov <kotaranov@xxxxxxxxxxxxx>
> > >
> > > Use size_t instead of u32 in helpers for queue creations to detect
> > > overflow of queue size. The queue size cannot exceed size of u32.
> > >
> > > Fixes: bd4ee700870a ("RDMA/mana_ib: UD/GSI QP creation for kernel")
> > > Signed-off-by: Konstantin Taranov <kotaranov@xxxxxxxxxxxxx>
> > > ---
> > > drivers/infiniband/hw/mana/cq.c | 9 +++++----
> > > drivers/infiniband/hw/mana/main.c | 15 +++++++++++++--
> > > drivers/infiniband/hw/mana/mana_ib.h | 4 ++--
> > > drivers/infiniband/hw/mana/qp.c | 11 ++++++-----
> > > 4 files changed, 26 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/hw/mana/cq.c
> > > b/drivers/infiniband/hw/mana/cq.c index 5c325ef..07b97da 100644
> > > --- a/drivers/infiniband/hw/mana/cq.c
> > > +++ b/drivers/infiniband/hw/mana/cq.c
> > > @@ -18,7 +18,7 @@ int mana_ib_create_cq(struct ib_cq *ibcq, const
> > > struct ib_cq_init_attr *attr,
> > > struct gdma_context *gc;
> > > bool is_rnic_cq;
> > > u32 doorbell;
> > > - u32 buf_size;
> > > + size_t buf_size;
> > > int err;
> > >
> > > mdev = container_of(ibdev, struct mana_ib_dev, ib_dev); @@ -45,7
> > > +45,8 @@ int mana_ib_create_cq(struct ib_cq *ibcq, const struct
> > > +ib_cq_init_attr
> > > *attr,
> > > }
> > >
> > > cq->cqe = attr->cqe;
> > > - err = mana_ib_create_queue(mdev, ucmd.buf_addr, cq->cqe
> > *
> > > COMP_ENTRY_SIZE,
> > > + buf_size = (size_t)cq->cqe * COMP_ENTRY_SIZE;
> > > + err = mana_ib_create_queue(mdev, ucmd.buf_addr, buf_size,
> > > &cq->queue);
> > > if (err) {
> > > ibdev_dbg(ibdev, "Failed to create queue for create
> > cq, %d\n",
> > > err); @@ -57,8 +58,8 @@ int mana_ib_create_cq(struct ib_cq *ibcq,
> > > const struct ib_cq_init_attr *attr,
> > > doorbell = mana_ucontext->doorbell;
> > > } else {
> > > is_rnic_cq = true;
> > > - buf_size = MANA_PAGE_ALIGN(roundup_pow_of_two(attr-
> > >cqe
> > > * COMP_ENTRY_SIZE));
> > > - cq->cqe = buf_size / COMP_ENTRY_SIZE;
> > > + cq->cqe = attr->cqe;
> > > + buf_size =
> > > MANA_PAGE_ALIGN(roundup_pow_of_two((size_t)attr->cqe *
> > > +COMP_ENTRY_SIZE));
> >
> > Why not do a check like:
> > If (attr->cqe > U32_MAX/COMP_ENTRY_SIZE)
> > return -EINVAL;
> >
> > And you don’t need to check them in mana_ib_create_kernel_queue() and
> > mana_ib_create_queue().
> >
>
> Yes, I was initially thinking about the small fix as you proposed and then ended up
> adding checks to all paths. As I see the same can happen if a user asks for a large WQ of RC.
> I believe a kernel client can also cause this overflow. We plan to add kernel RC soon and,
> as far as I understand, a kernel user can also ask to create a large CQ resulting in similar overflow.
The expectation that kernel users are using in-kernel API correctly and
kernel shouldn't have extra protection for API misuse.
Thanks
>
> - Konstantin
