Re: I found a synchronization problem in mm/vmalloc.c

Nick Piggin
2010-01-13T04:55:47Z

On Tue, Jan 12, 2010 at 06:32:09PM +0900, Yongseok Koh wrote:
> Sorry, Mr. Morton.
>
> Even though it is somewhat late, I am doing cc the mailing list.
>
> Thanks.
>
> -----Original Message-----
>
> On Thu, 7 Jan 2010 20:22:30 +0900
> "Yongseok Koh" <yongseok.koh@samsung.com> wrote:
>
> > Dear all,
> >
> > I___m Yongseok Koh in Korea.
> >
>
> Thanks for the report.
>
> Please do cc a mailing list when reporting bugs so that everyone else knows
> what's going on.
>
> >
> > I just got a new message in linux-2.6.28.10 (plz refer to the below)
> >
> > And, one of my colleagues found that there is a synchronization
> > problem in mm/vmalloc.c
> >
> >
> >
> > In free_unmap_area_noflush(), va->flags is marked as VM_LAZY_FREE
> > first, and then vmap_lazy_nr is increased atomically.
> >
> > But, in __purge_vmap_area_lazy(), while traversing of vmap_are_list,
> > nr is counted by checking VM_LAZY_FREE is set to va->flags.
> >
> > After counting the variable nr, kernel reads vmap_lazy_nr atomically
> > and checks a BUG_ON condition whether nr is greater than vmap_lazy_nr.
> >
> >
> >
> > The problem is that, if interrupted right after marking VM_LAZY_FREE,
> > increment of vmap_lazy_nr can be delayed.
> >
> > Consequently, BUG_ON condition can be met because nr is counted more
> > than vmap_lazy_nr.
> >
> >
> >
> > What I mentioned is highly probable when vmalloc/vfree are called
> > frequently.
> >
> > And my colleagues have verified this scenario by adding delay between
> > marking VM_LAZY_FREE and increasing vmap_lazy_nr in
> > free_unmap_area_noflush().
> >
> >
> >
> > Am I right ?
> >
>
> Looks plausible to me and as far as I can tell, current code has the same
> issue.

Yes, I think it's a good catch.

> Wakey wakey, Nick! What makes that BUG_ON() safe? Not purge_lock afacit?

No I think it is a bug. I would say that we can just get rid of the BUG_ON
now. atomic_t is signed, so it should be OK if it momentarily goes negative
(and anyway it's only used in a heuristic).

So, thanks for the report. Would you care to send a patch, or propose
another way to fix the problem?

Thanks,
Nick