On 12/06/2019 15:01, Jason Gunthorpe wrote:
> On Wed, Jun 12, 2019 at 10:28:42AM +0300, Gal Pressman wrote:
>> When inserting a new mmap entry to the xarray we should check for
>> 'mmap_page' overflow as it is limited to 32 bits.
>>
>> While at it, make sure to advance the mmap_page stored on the ucontext
>> only after a successful insertion.
>>
>> Fixes: 40909f664d27 ("RDMA/efa: Add EFA verbs implementation")
>> Signed-off-by: Gal Pressman <galpress@xxxxxxxxxx>
>> drivers/infiniband/hw/efa/efa_verbs.c | 21 ++++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
>> index 0fea5d63fdbe..c463c683ae84 100644
>> +++ b/drivers/infiniband/hw/efa/efa_verbs.c
>> @@ -204,6 +204,7 @@ static u64 mmap_entry_insert(struct efa_dev *dev, struct efa_ucontext *ucontext,
>> void *obj, u64 address, u64 length, u8 mmap_flag)
>> {
>> struct efa_mmap_entry *entry;
>> + u32 next_mmap_page;
>> int err;
>>
>> entry = kmalloc(sizeof(*entry), GFP_KERNEL);
>> @@ -216,15 +217,19 @@ static u64 mmap_entry_insert(struct efa_dev *dev, struct efa_ucontext *ucontext,
>> entry->mmap_flag = mmap_flag;
>>
>> xa_lock(&ucontext->mmap_xa);
>> + if (check_add_overflow(ucontext->mmap_xa_page,
>> + (u32)(length >> PAGE_SHIFT),
>> + &next_mmap_page))
>> + goto err_unlock;
>> +
>> entry->mmap_page = ucontext->mmap_xa_page;
>> - ucontext->mmap_xa_page += DIV_ROUND_UP(length, PAGE_SIZE);
>> err = __xa_insert(&ucontext->mmap_xa, entry->mmap_page, entry,
>> GFP_KERNEL);
>> + if (err)
>> + goto err_unlock;
>> +
>> + ucontext->mmap_xa_page = next_mmap_page;
>
> This is not ordered right anymore, the xa_lock can be released inside
> __xa_insert, so to be atomic you must do everything before calling
> __xa_insert.
Ah, missed the fact that __xa_insert could release the lock :\..
Thanks Jason, will bring back the mmap_xa_page assignment before the __xa_insert
call and unwind it in case of __xa_insert failure.
