[PATCH] um: protect VMA iteration

Johannes Berg johannes at sipsolutions.net
Tue Oct 18 02:44:50 PDT 2022


On Mon, 2022-10-17 at 15:50 +0100, Matthew Wilcox wrote:
> On Mon, Oct 17, 2022 at 11:06:30AM +0200, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg at intel.com>
> > 
> > Looks like this is needed now, otherwise we get RCU
> > splats from lockdep. But I don't know anything about
> > this code ...
> 
> You're getting lockdep splats now because there was no checking before.

Yeah that's what I was assuming, and seeing in your change anyway since
it just was doing a manual iteration.

> I assumed that the locking was correct before when making this change,
> and lockdep shows it wasn't ;-) 
> 

:-)

> So, you were traversing the list of
> VMAs with no protection against modification (ie a simulataneous call to
> mmap()/munmap() would lead to a UAF).  I imagine you really want a call
> to mmap_read_lock() / mmap_read_unlock(), and you'll want to backport
> it as far as you care about.

Well, to be fair, ARCH=um is !SMP && !PREEMPT, so I don't think anything
can actually happen here? At least I don't really see how anything
would. Even userspace can't run while here, I believe.

> Maybe this RCU locking is good enough; it'll let you walk the VMAs
> without any locking, but you have to know that (a) fix_range() doesn't
> sleep and (b) that if new VMAs are added during the walk or old ones are
> removed, it doesn't matter whether fix_range() is called on them or not.
> 
> From the tone of the email, I infer that you probably haven't done that
> depth of analysis ;-)  Taking the mmap_lock for read is the safe route.
> 

Indeed, thanks :)

johannes



More information about the linux-um mailing list