On Sat, Aug 28, 2021 at 2:28 PM Cyrill Gorcunov <gorcunov@xxxxxxxxx> wrote: > > On Fri, Aug 27, 2021 at 12:18:57PM -0700, Suren Baghdasaryan wrote: > > > > The name is stored in a pointer in the shared union in vm_area_struct > > that points to a null terminated string. Anonymous vmas with the same > > name (equivalent strings) and are otherwise mergeable will be merged. > > The name pointers are not shared between vmas even if they contain the > > same name. The name pointer is stored in a union with fields that are > > only used on file-backed mappings, so it does not increase memory usage. > > > > The patch is based on the original patch developed by Colin Cross, more > > specifically on its latest version [1] posted upstream by Sumit Semwal. > > It used a userspace pointer to store vma names. In that design, name > > pointers could be shared between vmas. However during the last upstreaming > > attempt, Kees Cook raised concerns [2] about this approach and suggested > > to copy the name into kernel memory space, perform validity checks [3] > > and store as a string referenced from vm_area_struct. > > One big concern is about fork() performance which would need to strdup > > anonymous vma names. Dave Hansen suggested experimenting with worst-case > > scenario of forking a process with 64k vmas having longest possible names > > [4]. I ran this experiment on an ARM64 Android device and recorded a > > worst-case regression of almost 40% when forking such a process. This > > regression is addressed in the followup patch which replaces the pointer > > to a name with a refcounted structure that allows sharing the name pointer > > between vmas of the same name. Instead of duplicating the string during > > fork() or when splitting a vma it increments the refcount. > > > > [1] https://lore.kernel.org/linux-mm/20200901161459.11772-4-sumit.semwal@xxxxxxxxxx/ > > [2] https://lore.kernel.org/linux-mm/202009031031.D32EF57ED@keescook/ > > [3] https://lore.kernel.org/linux-mm/202009031022.3834F692@keescook/ > > [4] https://lore.kernel.org/linux-mm/5d0358ab-8c47-2f5f-8e43-23b89d6a8e95@xxxxxxxxx/ > ... > > + > > +/* mmap_lock should be read-locked */ > > +static inline bool is_same_vma_anon_name(struct vm_area_struct *vma, > > + const char *name) > > +{ > > + const char *vma_name = vma_anon_name(vma); > > + > > + if (likely(!vma_name)) > > + return name == NULL; > > + > > + return name && !strcmp(name, vma_name); > > +} > > Hi Suren! There is very important moment with this new feature: if > we assign a name to some VMA it won't longer be mergeable even if > near VMA matches by all other attributes such as flags, permissions > and etc. I mean our vma_merge() start considering the vma namings > and names mismatch potentially blocks merging which happens now > without this new feature. Is it known behaviour or I miss something > pretty obvious here? Hi Cyrill, Correct, this is a known drawback of naming an anonymous VMA. I think I'll need to document this in prctl(2) manpage, which I should update to include this new PR_SET_VMA_ANON_NAME option. Thanks for pointing it out! Suren.
