On Wed 01-09-21 08:42:29, Suren Baghdasaryan wrote:
> 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.
This to me sounds very much like a debugging code that shouldn't make it
to the final patch to be merged. I do see your point of an early
diagnostic but we are talking about an internal MM code and that is not
really designed to be robust against its own failures so I do not see
why this should be any special.
--
Michal Hocko
SUSE Labs
