Re: [PATCH rdma-next 08/12] RDMA/odp: Check for overflow when computing the umem_odp end

From: Nathan Chancellor <natechancellor@gmail.com>
To: Leon Romanovsky <leon@kernel.org>
Cc: Doug Ledford <dledford@redhat.com>,
	Jason Gunthorpe <jgg@mellanox.com>,
	Leon Romanovsky <leonro@mellanox.com>,
	RDMA mailing list <linux-rdma@vger.kernel.org>,
	Guy Levi <guyle@mellanox.com>, Moni Shoua <monis@mellanox.com>
Subject: Re: [PATCH rdma-next 08/12] RDMA/odp: Check for overflow when computing the umem_odp end
Date: Mon, 26 Aug 2019 09:42:23 -0700	[thread overview]
Message-ID: <20190826164223.GA122752@archlinux-threadripper> (raw)
In-Reply-To: <20190819111710.18440-9-leon@kernel.org>

On Mon, Aug 19, 2019 at 02:17:06PM +0300, Leon Romanovsky wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
> 
> 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@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
>  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
> --- a/drivers/infiniband/core/umem_odp.c
> +++ 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.

Cheers,
Nathan