On Wed, Jun 27, 2018 at 09:27:37PM +0300, Leon Romanovsky wrote:
> > > index 5471b57b873d..6b0034c063f4 100644
> > > +++ b/drivers/infiniband/hw/mlx5/qp.c
> > > @@ -653,6 +653,9 @@ int bfregn_to_uar_index(struct mlx5_ib_dev *dev,
> > > int index_of_sys_page;
> > > int offset;
> > >
> > > + if (bfregn > MLX5_MAX_BFREGS)
> > > + return -EINVAL;
> >
> > This is not enough, you should make 'int bfregn' into 'unsigned int
> > bfregn'.
> >
> > Because here we do:
> > uar_index = bfregn_to_uar_index(dev, &context->bfregi,
> > ucmd.bfreg_index, true);
> >
> > And
> > __u32 bfreg_index;
> >
> > So, userspace could pass in 1<<31 and still pass (bfregn > MLX5_MAX_BFREGS)
> > after all the casting is worked out.
> >
>
> How? MLX5_MAX_BFREGS is 1 << 9 in mlx5
Beacuse this prints success?
void func(int arg)
{
if (arg > 9)
return;
printf("Success! %d\n", arg);
}
int main(int argc, const char *argv[])
{
uint32_t arg = -1;
func(arg);
}
> only negative value slipped through the cracks and passed the check
> mentioned in commit message.
I didn't understand that part of the commit message either. Aren't the
existing tests just fine if you make the argument and other variables
unsigned:
index_of_sys_page = bfregn / bfregs_per_sys_page;
if (index_of_sys_page >= bfregi->num_sys_pages)
return -EINVAL;
??
The issue is nothing is checking for negative values.
That is solved correctly by not using signed types - not by adding
more range checking.
So this is the fix, as it seems the existing 'index_of_sys_page >=
num_sys_pages' restriction is a perfectly fine array bound??
@@ -642,12 +642,12 @@ static void mlx5_ib_unlock_cqs(struct mlx5_ib_cq *send_cq,
struct mlx5_ib_cq *recv_cq);
int bfregn_to_uar_index(struct mlx5_ib_dev *dev,
- struct mlx5_bfreg_info *bfregi, int bfregn,
+ struct mlx5_bfreg_info *bfregi, u32 bfregn,
bool dyn_bfreg)
{
- int bfregs_per_sys_page;
- int index_of_sys_page;
- int offset;
+ unsigned int bfregs_per_sys_page;
+ u32 index_of_sys_page;
+ u32 offset;
bfregs_per_sys_page = get_uars_per_sys_page(dev, bfregi->lib_uar_4k) *
MLX5_NON_FP_BFREGS_PER_UAR;
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
