linux-kernel - Re: [PATCH 3/5] lib: lockless generic and arch independent page table (gpt) 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, 10 Nov 2014 20:29:15 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Jerome Glisse <j.glisse@...il.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-mm <linux-mm@...ck.org>, Joerg Roedel <joro@...tes.org>,
	Mel Gorman <mgorman@...e.de>, "H. Peter Anvin" <hpa@...or.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Johannes Weiner <jweiner@...hat.com>,
	Larry Woodman <lwoodman@...hat.com>,
	Rik van Riel <riel@...hat.com>,
	Dave Airlie <airlied@...hat.com>,
	Brendan Conoboy <blc@...hat.com>,
	Joe Donohue <jdonohue@...hat.com>,
	Duncan Poole <dpoole@...dia.com>,
	Sherry Cheung <SCheung@...dia.com>,
	Subhash Gutti <sgutti@...dia.com>,
	John Hubbard <jhubbard@...dia.com>,
	Mark Hairgrove <mhairgrove@...dia.com>,
	Lucien Dunning <ldunning@...dia.com>,
	Cameron Buschardt <cabuschardt@...dia.com>,
	Arvind Gopalakrishnan <arvindg@...dia.com>,
	Shachar Raindel <raindel@...lanox.com>,
	Liran Liss <liranl@...lanox.com>,
	Roland Dreier <roland@...estorage.com>,
	Ben Sander <ben.sander@....com>,
	Greg Stoner <Greg.Stoner@....com>,
	John Bridgman <John.Bridgman@....com>,
	Michael Mantor <Michael.Mantor@....com>,
	Paul Blinzer <Paul.Blinzer@....com>,
	Laurent Morichetti <Laurent.Morichetti@....com>,
	Alexander Deucher <Alexander.Deucher@....com>,
	Oded Gabbay <Oded.Gabbay@....com>,
	Jérôme Glisse <jglisse@...hat.com>
Subject: Re: [PATCH 3/5] lib: lockless generic and arch independent page table
 (gpt) v2.

On Mon, Nov 10, 2014 at 7:16 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> There's no reason for a "u64 cast". The value of "1 << pd_shift" is
> going to be an "int" regardless of what type pd_shift is. The type of
> a shift expression is the type of the left-hand side (with the C
> promotion rules forcing it to at least "int"), the right-hand
> expression type has absolutely no relevance.

Btw, for that exact reason, code like this:

+                  (uint64_t)(pdp->index +
+                  (1UL << (gpt_pdp_shift(gpt, pdp) + gpt->pd_shift)) - 1UL));

is likely buggy if you actually care about the uint64_t part.

On 32-bit, 1ul will be 32-bit. And so will "(1ul << .. ) -1UL",
regardless of the type of the right hand of the shift. So the fact
that gpt->pd_shift and gpt_pdp_shift() are both u64, the actual end
result is u32 (page->index is a 32-bit entity on 32-bit architectures,
since pgoff_t is an "unsigned long" too). So you're doing the shifts
in 32-bit, the addition in 32-bit, and then just casting the resulting
32-bit thing to a 64-bit entity.  The high 32 bits are guaranteed to
be zero, in other words.

This just highlights how wrong it is to make those shifts be u64. That
gpt_pdp_shift() helper similarly should at no point be returning u64.
It doesn't help, it only hurts. It makes the structure bigger for no
gain, and apparently it confuses people into thinking those shifts are
done in 64 bit.

When you do "a+b" or similar operations, the end result is the biggest
type size of 'a' and 'b' respectively (with the normal promotion to at
least 'int'). But that's not true of shifts, the type of the shift
expression is the (integer-promoted) left-hand side. The right-hand
side just gives the amount that value is shifted by, it doesn't affect
the type of the result.

                       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/