Re: [PATCH v8 2/3] mm: add a field to store names for private anonymous memory

[Suren Baghdasaryan]

On Wed, Sep 1, 2021 at 1:10 AM 'Michal Hocko' via kernel-team
<kernel-team@xxxxxxxxxxx> wrote:
>
> On Fri 27-08-21 12:18:57, Suren Baghdasaryan wrote:
> [...]
> > +static void replace_vma_anon_name(struct vm_area_struct *vma, const char *name)
> > +{
> > +     if (!name) {
> > +             free_vma_anon_name(vma);
> > +             return;
> > +     }
> > +
> > +     if (vma->anon_name) {
> > +             /* Should never happen, to dup use dup_vma_anon_name() */
> > +             WARN_ON(vma->anon_name == name);
>
> What is the point of this warning?

I wanted to make sure replace_vma_anon_name() is not used from inside
vm_area_dup() or some similar place (does not exist today but maybe in
the future) where "new" vma is a copy of "orig" vma and
new->anon_name==orig->anon_name. If someone by mistake calls
replace_vma_anon_name(new, orig->anon_name) and
new->anon_name==orig->anon_name then they will keep pointing to the
same name pointer, which breaks an assumption that ->anon_name
pointers are not shared among vmas even if the string is the same.
That would eventually lead to use-after-free error. After the next
patch implementing refcounting, the similar situation would lead to
both new and orig vma pointing to the same anon_vma_name structure
without raising the refcount, which would also lead to use-after-free
error. That's why the above comment asks to use dup_vma_anon_name() if
this warning ever happens.
I can remove the warning but I thought the problem is subtle enough to
put some safeguards.

>
> > +
> > +             /* Same name, nothing to do here */
> > +             if (!strcmp(name, vma->anon_name))
> > +                     return;
> > +
> > +             free_vma_anon_name(vma);
> > +     }
> > +     vma->anon_name = kstrdup(name, GFP_KERNEL);
> > +}
> --
> Michal Hocko
> SUSE Labs
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@xxxxxxxxxxx.
>