On Mon, Aug 26, 2019 at 04:55:45PM +0000, Jason Gunthorpe wrote:
> On Mon, Aug 26, 2019 at 09:42:23AM -0700, Nathan Chancellor wrote:
> > On Mon, Aug 19, 2019 at 02:17:06PM +0300, Leon Romanovsky wrote:
> > > From: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> > >
> > > Since the page size can be extended in the ODP case by IB_ACCESS_HUGETLB
> > > the existing overflow checks done by ib_umem_get() are not
> > > sufficient. Check for overflow again.
> > >
> > > Further, remove the unchecked math from the inlines and just use the
> > > precomputed value stored in the interval_tree_node.
> > >
> > > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > drivers/infiniband/core/umem_odp.c | 25 +++++++++++++++++++------
> > > include/rdma/ib_umem_odp.h | 5 ++---
> > > 2 files changed, 21 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c
> > > index 2575dd783196..46ae9962fae3 100644
> > > +++ b/drivers/infiniband/core/umem_odp.c
> > > @@ -294,19 +294,32 @@ static inline int ib_init_umem_odp(struct ib_umem_odp *umem_odp,
> > >
> > > umem_odp->umem.is_odp = 1;
> > > if (!umem_odp->is_implicit_odp) {
> > > - size_t pages = ib_umem_odp_num_pages(umem_odp);
> > > -
> > > + size_t page_size = 1UL << umem_odp->page_shift;
> > > + size_t pages;
> > > +
> > > + umem_odp->interval_tree.start =
> > > + ALIGN_DOWN(umem_odp->umem.address, page_size);
> > > + if (check_add_overflow(umem_odp->umem.address,
> > > + umem_odp->umem.length,
> > > + &umem_odp->interval_tree.last))
> > > + return -EOVERFLOW;
> >
> > This if statement causes a warning on 32-bit ARM:
> >
> > drivers/infiniband/core/umem_odp.c:295:7: warning: comparison of distinct
> > pointer types ('typeof (umem_odp->umem.address) *' (aka 'unsigned long *')
> > and 'typeof (umem_odp->umem.length) *' (aka 'unsigned int *'))
> > [-Wcompare-distinct-pointer-types]
> > if (check_add_overflow(umem_odp->umem.address,
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > include/linux/overflow.h:59:15: note: expanded from macro 'check_add_overflow'
> > (void) (&__a == &__b); \
> > ~~~~ ^ ~~~~
> > 1 warning generated.
>
> Hum, I'm pretty sure 0-day has stopped running 32 bit builds or
> something :\
>
> Jason
My report was with clang but GCC reports the same type of warning:
In file included from ../include/linux/slab.h:16,
from ../drivers/infiniband/core/umem_odp.c:38:
../drivers/infiniband/core/umem_odp.c: In function 'ib_init_umem_odp':
../include/linux/overflow.h:59:15: warning: comparison of distinct pointer types lacks a cast
59 | (void) (&__a == &__b); \
| ^~
../drivers/infiniband/core/umem_odp.c:220:7: note: in expansion of macro 'check_add_overflow'
220 | if (check_add_overflow(umem_odp->umem.address,
| ^~~~~~~~~~~~~~~~~~
Adding Philip and Rong as I believe that they are the current 0-day
maintainers.
Cheers,
Nathan
