linux-kernel - Re: [PATCH 3/6] mmu_notifier: add event information to address invalidation v2

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]

Date:	Mon, 30 Jun 2014 18:57:25 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Jérôme Glisse <j.glisse@...il.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-mm <linux-mm@...ck.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Mel Gorman <mgorman@...e.de>, Peter Anvin <hpa@...or.com>,
	peterz@...raread.org, Andrea Arcangeli <aarcange@...hat.com>,
	Rik van Riel <riel@...hat.com>,
	Johannes Weiner <jweiner@...hat.com>,
	Mark Hairgrove <mhairgrove@...dia.com>,
	Jatin Kumar <jakumar@...dia.com>,
	Subhash Gutti <sgutti@...dia.com>,
	Lucien Dunning <ldunning@...dia.com>,
	Cameron Buschardt <cabuschardt@...dia.com>,
	Arvind Gopalakrishnan <arvindg@...dia.com>,
	John Hubbard <jhubbard@...dia.com>,
	Sherry Cheung <SCheung@...dia.com>,
	Duncan Poole <dpoole@...dia.com>,
	Oded Gabbay <Oded.Gabbay@....com>,
	Alexander Deucher <Alexander.Deucher@....com>,
	Andrew Lewycky <Andrew.Lewycky@....com>,
	Jérôme Glisse <jglisse@...hat.com>
Subject: Re: [PATCH 3/6] mmu_notifier: add event information to address
 invalidation v2

On Fri, Jun 27, 2014 at 7:00 PM, Jérôme Glisse <j.glisse@...il.com> wrote:
> From: Jérôme Glisse <jglisse@...hat.com>
>
> The event information will be useful [...]

That needs to be cleaned up, though.

Why the heck are you making up ew and stupid event types? Now you make
the generic VM code do stupid things like this:

+       if ((vma->vm_flags & VM_READ) && (vma->vm_flags & VM_WRITE))
+               event = MMU_MPROT_RANDW;
+       else if (vma->vm_flags & VM_WRITE)
+               event = MMU_MPROT_WONLY;
+       else if (vma->vm_flags & VM_READ)
+               event = MMU_MPROT_RONLY;

which makes no sense at all. The names are some horrible abortion too
("RANDW"? That sounds like "random write" to me, not "read-and-write",
which is commonly shortened RW or perhaps RDWR. Same foes for
RONLY/WONLY - what kind of crazy names are those?

But more importantly, afaik none of that is needed. Instead, tell us
why you need particular flags, and don't make up crazy names like
this. As far as I can tell, you're already passing in the new
protection information (thanks to passing in the vma), so all those
badly named states you've made up seem to be totally pointless. They
add no actual information, but they *do* add crazy code like the above
to generic code that doesn't even WANT any of this crap. The only
thing this should need is a MMU_MPROT event, and just use that. Then
anybody who wants to look at whether the protections are being changed
to read-only, they can just look at the vma->vm_flags themselves.

So things like this need to be tightened up and made sane before any
chance of merging it.

So NAK NAK NAK in the meantime.

            Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/