On 09.05.24 13:05, Vincent Donnefort wrote:
On Tue, May 07, 2024 at 10:34:02PM -0400, Steven Rostedt wrote:
On Tue, 30 Apr 2024 12:13:51 +0100
Vincent Donnefort <vdonnef...@google.com> wrote:

+#ifdef CONFIG_MMU
+static int __rb_map_vma(struct ring_buffer_per_cpu *cpu_buffer,
+                       struct vm_area_struct *vma)
+{
+       unsigned long nr_subbufs, nr_pages, vma_pages, pgoff = vma->vm_pgoff;
+       unsigned int subbuf_pages, subbuf_order;
+       struct page **pages;
+       int p = 0, s = 0;
+       int err;
+
+       /* Refuse MP_PRIVATE or writable mappings */
+       if (vma->vm_flags & VM_WRITE || vma->vm_flags & VM_EXEC ||
+           !(vma->vm_flags & VM_MAYSHARE))
+               return -EPERM;
+
+       /*
+        * Make sure the mapping cannot become writable later. Also tell the VM
+        * to not touch these pages (VM_DONTCOPY | VM_DONTEXPAND). Finally,
+        * prevent migration, GUP and dump (VM_IO).
+        */
+       vm_flags_mod(vma, VM_DONTCOPY | VM_DONTEXPAND | VM_IO, VM_MAYWRITE);

Do we really need the VM_IO?

When testing this in gdb, I would get:

(gdb) p tmap->map->subbuf_size
Cannot access memory at address 0x7ffff7fc2008

It appears that you can't ptrace IO memory. When I removed that flag,
gdb has no problem reading that memory.

Yeah, VM_IO indeed implies DONTDUMP. VM_IO was part of Linus recommendations.

Yes, the VM should recognize that memory to some degree as being special already due to VM_MIXEDMAP and VM_DONTEXPAND.

#define VM_SPECIAL (VM_IO | VM_DONTEXPAND | VM_PFNMAP | VM_MIXEDMAP)

So any of these flag achieve that (e.g., mlock_fixup() checks VM_SPECIAL). KSM similarly skips VM_DONTEXPAND and VM_MIXEDMAP (likely we should be using VM_SPECIAL in vma_ksm_compatible()). Not sure about page migration, likely its fine.

Thinking about MADV_DONTNEED, I can spot in madvise_dontneed_free_valid_vma() only that we disallow primarily VM_PFNMAP.

... I assume if user space MADV_DONTNEED's some pages we'll simply get a page fault later on access that will SIGBUS, handling that gracefully (we should double-check!).


But perhaps, VM_DONTEXPAND and MIXEDMAP (implicitely set by vm_insert_pages) are
enough protection?

Do we want to dump these pages? VM_DONTDUMP might be reasonabe then.


I don't see how anything could use GUP there and as David pointed-out on the
previous version, it doesn't event prevent the GUP-fast path.

Yes, GUP-fast would still have worked under some conditions.

--
Cheers,

David / dhildenb


Reply via email to