On Tue, Aug 6, 2024 at 6:14 PM Suren Baghdasaryan <surenb@xxxxxxxxxx> wrote:
> On Mon, Aug 5, 2024 at 5:52 AM Jann Horn <jannh@xxxxxxxxxx> wrote:
> >
> > There is a (harmless) type confusion in lock_vma_under_rcu():
> > After vma_start_read(), we have taken the VMA lock but don't know yet
> > whether the VMA has already been detached and scheduled for RCU freeing.
> > At this point, ->vm_start and ->vm_end are accessed.
> >
> > vm_area_struct contains a union such that ->vm_rcu uses the same memory as
> > ->vm_start and ->vm_end; so accessing ->vm_start and ->vm_end of a detached
> > VMA is illegal and leads to type confusion between union members.
> >
> > Fix it by reordering the vma->detached check above the address checks, and
> > document the rules for RCU readers accessing VMAs.
> >
> > This will probably change the number of observed VMA_LOCK_MISS events
> > (since previously, trying to access a detached VMA whose ->vm_rcu has been
> > scheduled would bail out when checking the fault address against the
> > rcu_head members reinterpreted as VMA bounds).
> >
> > Fixes: 50ee32537206 ("mm: introduce lock_vma_under_rcu to be used from arch-specific code")
> > Signed-off-by: Jann Horn <jannh@xxxxxxxxxx>
>
> Thanks for fixing this subtle issue and clearly documenting the rules!
> Not sure if we should CC stable? It is harmless but it's still a bug...
Yeah, I'm not sure - I guess it kinda depends on how much we care
about VMA_LOCK_MISS being accurate?
> Acked-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
Thanks!
