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
[off-list ref] wrote:

quoted

On Fri 27-08-21 12:18:57, Suren Baghdasaryan wrote:
[...]

quoted

+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