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

On Wed, Sep 1, 2021 at 1:10 AM 'Michal Hocko' via kernel-team
[off-list ref] wrote:

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.

quoted

+
+             /* 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@android.com.