On 12/06/2019 16:28, Gal Pressman wrote:
> 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.
On second thought, unwinding the mmap_xa_page will cause other issues.. Will
drop this part.
