* [PATCH] IB/mlx4: prevent undefined shift in set_user_sq_size()
@ 2019-06-08 9:22 Dan Carpenter
2019-06-10 13:28 ` Jason Gunthorpe
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2019-06-08 9:22 UTC (permalink / raw)
To: Yishai Hadas
Cc: Doug Ledford, Jason Gunthorpe, linux-rdma, linux-kernel,
kernel-janitors
The ucmd->log_sq_bb_count is a u8 that comes from the user. If it's
larger than the number of bits in an int then that's undefined behavior.
It turns out this doesn't really cause an issue at runtime but it's
still nice to clean it up.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/infiniband/hw/mlx4/qp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 5221c0794d1d..9f6eb23e8044 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -439,7 +439,8 @@ static int set_user_sq_size(struct mlx4_ib_dev *dev,
struct mlx4_ib_create_qp *ucmd)
{
/* Sanity check SQ size before proceeding */
- if ((1 << ucmd->log_sq_bb_count) > dev->dev->caps.max_wqes ||
+ if (ucmd->log_sq_bb_count > 31 ||
+ (1 << ucmd->log_sq_bb_count) > dev->dev->caps.max_wqes ||
ucmd->log_sq_stride >
ilog2(roundup_pow_of_two(dev->dev->caps.max_sq_desc_sz)) ||
ucmd->log_sq_stride < MLX4_IB_MIN_SQ_STRIDE)
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] IB/mlx4: prevent undefined shift in set_user_sq_size()
2019-06-08 9:22 [PATCH] IB/mlx4: prevent undefined shift in set_user_sq_size() Dan Carpenter
@ 2019-06-10 13:28 ` Jason Gunthorpe
2019-06-10 14:08 ` Leon Romanovsky
2019-06-11 10:07 ` Dan Carpenter
0 siblings, 2 replies; 4+ messages in thread
From: Jason Gunthorpe @ 2019-06-10 13:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: Yishai Hadas, Doug Ledford, linux-rdma, linux-kernel,
kernel-janitors
On Sat, Jun 08, 2019 at 12:22:31PM +0300, Dan Carpenter wrote:
> The ucmd->log_sq_bb_count is a u8 that comes from the user. If it's
> larger than the number of bits in an int then that's undefined behavior.
> It turns out this doesn't really cause an issue at runtime but it's
> still nice to clean it up.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/infiniband/hw/mlx4/qp.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
> index 5221c0794d1d..9f6eb23e8044 100644
> --- a/drivers/infiniband/hw/mlx4/qp.c
> +++ b/drivers/infiniband/hw/mlx4/qp.c
> @@ -439,7 +439,8 @@ static int set_user_sq_size(struct mlx4_ib_dev *dev,
> struct mlx4_ib_create_qp *ucmd)
> {
> /* Sanity check SQ size before proceeding */
> - if ((1 << ucmd->log_sq_bb_count) > dev->dev->caps.max_wqes ||
> + if (ucmd->log_sq_bb_count > 31 ||
> + (1 << ucmd->log_sq_bb_count) > dev->dev->caps.max_wqes ||
Surely this should use check_shl_overflow() ?
Jason
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] IB/mlx4: prevent undefined shift in set_user_sq_size()
2019-06-10 13:28 ` Jason Gunthorpe
@ 2019-06-10 14:08 ` Leon Romanovsky
2019-06-11 10:07 ` Dan Carpenter
1 sibling, 0 replies; 4+ messages in thread
From: Leon Romanovsky @ 2019-06-10 14:08 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Dan Carpenter, Yishai Hadas, Doug Ledford, linux-rdma,
linux-kernel, kernel-janitors
On Mon, Jun 10, 2019 at 10:28:49AM -0300, Jason Gunthorpe wrote:
> On Sat, Jun 08, 2019 at 12:22:31PM +0300, Dan Carpenter wrote:
> > The ucmd->log_sq_bb_count is a u8 that comes from the user. If it's
> > larger than the number of bits in an int then that's undefined behavior.
> > It turns out this doesn't really cause an issue at runtime but it's
> > still nice to clean it up.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > drivers/infiniband/hw/mlx4/qp.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
> > index 5221c0794d1d..9f6eb23e8044 100644
> > --- a/drivers/infiniband/hw/mlx4/qp.c
> > +++ b/drivers/infiniband/hw/mlx4/qp.c
> > @@ -439,7 +439,8 @@ static int set_user_sq_size(struct mlx4_ib_dev *dev,
> > struct mlx4_ib_create_qp *ucmd)
> > {
> > /* Sanity check SQ size before proceeding */
> > - if ((1 << ucmd->log_sq_bb_count) > dev->dev->caps.max_wqes ||
> > + if (ucmd->log_sq_bb_count > 31 ||
> > + (1 << ucmd->log_sq_bb_count) > dev->dev->caps.max_wqes ||
>
> Surely this should use check_shl_overflow() ?
Yes
>
> Jason
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] IB/mlx4: prevent undefined shift in set_user_sq_size()
2019-06-10 13:28 ` Jason Gunthorpe
2019-06-10 14:08 ` Leon Romanovsky
@ 2019-06-11 10:07 ` Dan Carpenter
1 sibling, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2019-06-11 10:07 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Yishai Hadas, Doug Ledford, linux-rdma, linux-kernel,
kernel-janitors
On Mon, Jun 10, 2019 at 10:28:49AM -0300, Jason Gunthorpe wrote:
> On Sat, Jun 08, 2019 at 12:22:31PM +0300, Dan Carpenter wrote:
> > The ucmd->log_sq_bb_count is a u8 that comes from the user. If it's
> > larger than the number of bits in an int then that's undefined behavior.
> > It turns out this doesn't really cause an issue at runtime but it's
> > still nice to clean it up.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > drivers/infiniband/hw/mlx4/qp.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
> > index 5221c0794d1d..9f6eb23e8044 100644
> > --- a/drivers/infiniband/hw/mlx4/qp.c
> > +++ b/drivers/infiniband/hw/mlx4/qp.c
> > @@ -439,7 +439,8 @@ static int set_user_sq_size(struct mlx4_ib_dev *dev,
> > struct mlx4_ib_create_qp *ucmd)
> > {
> > /* Sanity check SQ size before proceeding */
> > - if ((1 << ucmd->log_sq_bb_count) > dev->dev->caps.max_wqes ||
> > + if (ucmd->log_sq_bb_count > 31 ||
> > + (1 << ucmd->log_sq_bb_count) > dev->dev->caps.max_wqes ||
>
> Surely this should use check_shl_overflow() ?
>
Same for the other one I sent. I'll resend in a couple days. No rush.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-11 10:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-08 9:22 [PATCH] IB/mlx4: prevent undefined shift in set_user_sq_size() Dan Carpenter
2019-06-10 13:28 ` Jason Gunthorpe
2019-06-10 14:08 ` Leon Romanovsky
2019-06-11 10:07 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox