Messages in this thread | | | Date | Tue, 6 Mar 2012 05:57:06 +0000 | From | Al Viro <> | Subject | Re: [patches] VM-related fixes |
| |
On Mon, Mar 05, 2012 at 09:25:05PM -0800, Arve Hj?nnev?g wrote: > > Sorry, no. > > ? ? ? ? ? ? ? ?down_write(&mm->mmap_sem); > > ? ? ? ? ? ? ? ?vma = proc->vma; > > ? ? ? ? ? ? ? ?if (vma && mm != vma->vm_mm) { > > does *not* do what you seem to describe; there's nothing to protect you > > from proc->vma getting freed under you right between load from proc->vma > > and check of vma->mm. ?->mmap_sem on the right mm would prevent that, > > but this one doesn't guarantee anything. ?Get preempted after the second > > line quoted above and by the time you get the timeslice back, you might > > have had munmap() done by another thread, with vma freed, its memory > > recycled, etc. > > > > OK, if the memory got freed and then re-used by someone who stored a > value that matched a pointer to the mm struct that was just locked, > this check will fail to catch it. I can check against a cached vm_mm > member from mmap instead, assuming this will not change before > ->close() is called. Does that sound reasonable, or is there a better > way to check this?
Huh? Sorry, I hadn't been able to parse that - what do you want to cache, where and what do you want to check? Again, at that point *(proc->vma) may very well be random garbage, so looking at it would be pointless; the value you had at ->mmap() time would be simply current->mm of mmap(2) caller; if you want to check that it matches that of opener, fine, but then why not do just that in ->mmap()?
| |