Messages in this thread | | | Date | Mon, 21 Apr 2014 08:36:25 -0500 | From | Dimitri Sivanich <> | Subject | Re: [PATCH 5/6] drivers,sgi-gru/grufault.c: call find_vma with the mmap_sem held |
| |
On Sat, Apr 19, 2014 at 07:26:30PM -0700, Davidlohr Bueso wrote: > From: Jonathan Gonzalez V <zeus@gnu.org> > > Performing vma lookups without taking the mm->mmap_sem is asking > for trouble. While doing the search, the vma in question can > be modified or even removed before returning to the caller. > Take the lock in order to avoid races while iterating through > the vmacache and/or rbtree. > > This patch is completely *untested*.
The mmap_sem is already taken in all paths calling gru_vtop().
The gru_intr() function takes it before calling gru_try_dropin(), from which all calls to gru_vtop() originate.
The gru_find_lock_gts() function takes it when called from gru_handle_user_call_os(), which then calls gru_user_dropin()->gru_try_dropin().
Nacked-by: Dimitri Sivanich <sivanich@sgi.com>
> > Signed-off-by: Jonathan Gonzalez V <zeus@gnu.org> > Signed-off-by: Davidlohr Bueso <davidlohr@hp.com> > Cc: Dimitri Sivanich <sivanich@sgi.com > --- > drivers/misc/sgi-gru/grufault.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/misc/sgi-gru/grufault.c b/drivers/misc/sgi-gru/grufault.c > index f74fc0c..15adc84 100644 > --- a/drivers/misc/sgi-gru/grufault.c > +++ b/drivers/misc/sgi-gru/grufault.c > @@ -266,6 +266,7 @@ static int gru_vtop(struct gru_thread_state *gts, unsigned long vaddr, > unsigned long paddr; > int ret, ps; > > + down_write(&mm->mmap_sem); > vma = find_vma(mm, vaddr); > if (!vma) > goto inval; > @@ -277,22 +278,26 @@ static int gru_vtop(struct gru_thread_state *gts, unsigned long vaddr, > rmb(); /* Must/check ms_range_active before loading PTEs */ > ret = atomic_pte_lookup(vma, vaddr, write, &paddr, &ps); > if (ret) { > - if (atomic) > - goto upm; > + if (atomic) { > + up_write(&mm->mmap_sem); > + return VTOP_RETRY; > + } > if (non_atomic_pte_lookup(vma, vaddr, write, &paddr, &ps)) > goto inval; > } > if (is_gru_paddr(paddr)) > goto inval; > + > + up_write(&mm->mmap_sem); > + > paddr = paddr & ~((1UL << ps) - 1); > *gpa = uv_soc_phys_ram_to_gpa(paddr); > *pageshift = ps; > return VTOP_SUCCESS; > > inval: > + up_write(&mm->mmap_sem); > return VTOP_INVALID; > -upm: > - return VTOP_RETRY; > } > > > -- > 1.8.1.4
| |