linux-kernel - Re: [PATCH v22 2/5] ring-buffer: Introducing ring-buffer mapping functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zjyt-nX3el5nnDPm@google.com>
Date: Thu, 9 May 2024 12:05:30 +0100
From: Vincent Donnefort <vdonnefort@...gle.com>
To: 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,
	david@...hat.com, linux-mm@...ck.org
Subject: Re: [PATCH v22 2/5] ring-buffer: Introducing ring-buffer mapping
 functions

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.
But perhaps, VM_DONTEXPAND and MIXEDMAP (implicitely set by vm_insert_pages) are
enough protection?

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.

> 
> I think we should drop that flag.
> 
> Can you send a v23 with that removed, Shuah's update, and also the
> change below:

Ack.

[...]