linux-kernel - Re: [PATCH] kernel/fork.c: use sizeof() instead of sizeof

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-next>] [day] [month] [year] [list]

Date:	Tue, 2 Feb 2016 17:04:06 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Aleksa Sarai <cyphar@...har.com>
Cc:	xypron.glpk@....de, Tejun Heo <tj@...nel.org>, koct9i@...il.com,
	ebiederm@...ssion.com, Wei Tang <tangwei@...s.chinamobile.com>,
	linux-kernel@...r.kernel.org, oleg@...hat.com,
	josh@...htriplett.org, jason.low2@...com, peterz@...radead.org,
	Ingo Molnar <mingo@...nel.org>, akpm@...ux-foundation.org,
	kirill.shutemov@...ux.intel.com
Subject: Re: [PATCH] kernel/fork.c: use sizeof() instead of sizeof

On Tue, Feb 02, 2016 at 03:56:01PM +0000, Aleksa Sarai wrote:

> I haven't looked at the order of operations for sizeof, but I imagine
> there's cases where it might bind in a different way than is expected. Are
> you sure there'd be no negative downside to removing the check (the whole
> point is to ensure no new code has stuff like that).

I won't comment on the whole point (or lack thereof) of checkpatch.pl, but
the only subtlety with sizeof is that sizeof ( type-name ) <something> is
*never* interpreted as sizeof of something cast to type-name.  IOW, its
priority is higher than that of typecasts.  If <something> starts with {,
it's a sizeof of compound literal, otherwise it's an unary expression
"sizeof ( type-name )" followed by <something>, which might or might not
yield a valid expression (e.g.
	sizeof(int)1
won't parse, while
	sizeof(int)-1
will be treated as (sizeof(int)) - 1).

Potential headache is along the lines of
#define A (int)-1
sizeof A
but that's more of "use enough parentheses in body of a macro to avoid nasty
surprises" - same as e.g.
#define A 2 + 2
A * A
yielding 8 (2 + 2 * 2 + 2) rather than expected 16 ((2 + 2) * (2 + 2))

FWIW, the actual rules are
	unary-expression: postfix-expression |
			  ++ unary-expression |
			  -- unary-expression |
			  - cast-expression |
			  + cast-expression |
			  ! cast-expression |
			  ~ cast-expression |
			  * cast-expression |
			  & cast-expression |
			  sizeof unary-expression |
			  sizeof ( type-name )
	cast-expression: unary-expression |
			 ( type-name ) cast-expression
Note that while e.g.
	++ ++ n
is allowed by grammar, it runs afoul of the constraint for ++ argument, which
must be a modifiable lvalue.  None of the operators above yield that, so
the rules for ++ and -- might as well have been ++ postfix-expression and
-- postfix-expression resp.