[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c36689df-cfad-46f5-835e-54073b6454a4@redhat.com>
Date: Fri, 10 May 2024 11:15:59 +0200
From: David Hildenbrand <david@...hat.com>
To: Vincent Donnefort <vdonnefort@...gle.com>,
Steven Rostedt <rostedt@...dmis.org>
Cc: mhiramat@...nel.org, linux-kernel@...r.kernel.org,
linux-trace-kernel@...r.kernel.org, mathieu.desnoyers@...icios.com,
kernel-team@...roid.com, rdunlap@...radead.org, rppt@...nel.org,
linux-mm@...ck.org
Subject: Re: [PATCH v22 2/5] ring-buffer: Introducing ring-buffer mapping
functions
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 <vdonnefort@...gle.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
Powered by blists - more mailing lists