| Message ID | 20190618130732.20895-1-galpress@amazon.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 7a5834e456f7fb3eca9b63af2a6bc7f460ae482f |
| Headers | show |
| Series | [for-rc,v2] RDMA/efa: Handle mmap insertions overflow | expand |
On Tue, Jun 18, 2019 at 04:07:32PM +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. > > Fixes: 40909f664d27 ("RDMA/efa: Add EFA verbs implementation") > Signed-off-by: Gal Pressman <galpress@amazon.com> > Changelog: > v1->v2 > * Bring back the ucontext->mmap_xa_page assignment before __xa_insert > 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..fb6115244d4c 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); Why did DIV_ROUND_UP become >> ? Jason
On 18/06/2019 21:48, Jason Gunthorpe wrote: > On Tue, Jun 18, 2019 at 04:07:32PM +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. >> >> Fixes: 40909f664d27 ("RDMA/efa: Add EFA verbs implementation") >> Signed-off-by: Gal Pressman <galpress@amazon.com> >> Changelog: >> v1->v2 >> * Bring back the ucontext->mmap_xa_page assignment before __xa_insert >> 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..fb6115244d4c 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); > > Why did DIV_ROUND_UP become >> ? Since length is guaranteed to be a multiple of PAGE_SIZE.
On Tue, 2019-06-18 at 22:33 +0300, Gal Pressman wrote: > On 18/06/2019 21:48, Jason Gunthorpe wrote: > > On Tue, Jun 18, 2019 at 04:07:32PM +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. > > > > > > Fixes: 40909f664d27 ("RDMA/efa: Add EFA verbs implementation") > > > Signed-off-by: Gal Pressman <galpress@amazon.com> > > > Changelog: > > > v1->v2 > > > * Bring back the ucontext->mmap_xa_page assignment before > > > __xa_insert > > > 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..fb6115244d4c 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); > > > > Why did DIV_ROUND_UP become >> ? > > Since length is guaranteed to be a multiple of PAGE_SIZE. Thanks, applied to for-rc.
diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c index 0fea5d63fdbe..fb6115244d4c 100644 --- a/drivers/infiniband/hw/efa/efa_verbs.c +++ 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); + ucontext->mmap_xa_page = next_mmap_page; err = __xa_insert(&ucontext->mmap_xa, entry->mmap_page, entry, GFP_KERNEL); + if (err) + goto err_unlock; + xa_unlock(&ucontext->mmap_xa); - if (err){ - kfree(entry); - return EFA_MMAP_INVALID; - } ibdev_dbg( &dev->ibdev, @@ -232,6 +237,12 @@ static u64 mmap_entry_insert(struct efa_dev *dev, struct efa_ucontext *ucontext, entry->obj, entry->address, entry->length, get_mmap_key(entry)); return get_mmap_key(entry); + +err_unlock: + xa_unlock(&ucontext->mmap_xa); + kfree(entry); + return EFA_MMAP_INVALID; + } int efa_query_device(struct ib_device *ibdev,
When inserting a new mmap entry to the xarray we should check for 'mmap_page' overflow as it is limited to 32 bits. Fixes: 40909f664d27 ("RDMA/efa: Add EFA verbs implementation") Signed-off-by: Gal Pressman <galpress@amazon.com> --- Changelog: v1->v2 * Bring back the ucontext->mmap_xa_page assignment before __xa_insert --- drivers/infiniband/hw/efa/efa_verbs.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)