Re: [PATCH] mm/mempolicy: fix a race between offset_il_node and mpol_rebind_task

[Matthew Wilcox <willy@infradead.org>]

On Sat, Aug 14, 2021 at 10:37:23AM +0800, Muchun Song wrote:
> On Sat, Aug 14, 2021 at 12:58 AM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Sat, Aug 14, 2021 at 12:40:53AM +0800, yanghui wrote:
> > > +++ b/mm/mempolicy.c
> > > @@ -193,7 +193,7 @@ static int mpol_new_interleave(struct mempolicy *pol, const nodemask_t *nodes)
> > >  {
> > >       if (nodes_empty(*nodes))
> > >               return -EINVAL;
> > > -     pol->nodes = *nodes;
> > > +     WRITE_ONCE(pol->nodes, *nodes);
> >
> > typedef struct { DECLARE_BITMAP(bits, MAX_NUMNODES); } nodemask_t;
> >
> > If MAX_NUMNODES is large enough, is WRITE_ONCE going to work?  It could
> > be 128 bits, and few architectures have an atomic 128-bit store
> > instruction.
> >
> 
> Hi Matthew,
> 
> In my memory, the WRITE_ONCE will become memcpy if
> the size is greater than sizeof(long). But I found that it didn't
> support the size over sizeof(long) nowadays. So you are
> right. It doesn't work when MAX_NUMNODES is large enough.
> 
> If we don't use WRITE_ONCE/READ_ONCE here, it seems
> like no problem. At least we can sure offset_il_node returns
> a valid node id. What is your opinion about this fix?

Yes, I think that's a reasonable fix.  Include a comment about how
copying pol->nodes to the stack might race with a change to pol->nodes,
but that doesn't matter as long as we don't look at pol->nodes later?