linux-kernel - Re: [PATCH] debug: fix BUILD_BUG_ON() for non-constant expressions

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:	Tue, 19 Aug 2008 19:33:59 +0300
From:	Boaz Harrosh <bharrosh@...asas.com>
To:	Ingo Molnar <mingo@...e.hu>
CC:	Rusty Russell <rusty@...tcorp.com.au>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Alexey Dobriyan <adobriyan@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Sam Ravnborg <sam@...nborg.org>
Subject: Re: [PATCH] debug: fix BUILD_BUG_ON() for non-constant expressions

Ingo Molnar wrote:
> * Boaz Harrosh <bharrosh@...asas.com> wrote:
> 
>>> Link time warnings are easy enough to miss.
>>>
>>> So unless there's a better way of doing it all at compile time (i'd 
>>> really prefer that!) i'd prefer the link time error about botched 
>>> BUILD_BUG_ON() conditions - as my commits introduce.
>>>
>>> 	Ingo
>>> --
>> #define BUILD_BUG_ON(condition)						\
>> do {
>> 	enum { bad = !!(condition)}; 									\
>> 	static struct { char arr[1 - 2*bad]; } x __maybe_unused;	\
>> } while(0)
>>
>> the enum definition will not let in anything not compile-time constant.
> 
> nice trick!
> 
>> But then I fail on: (include/linux/virtio_config.h:99)
>>
>> 	if (__builtin_constant_p(fbit))
>> 		BUILD_BUG_ON(fbit >= 32);
>>
>> is that code broken?
> 
> hmm ... that's a bit sad, gcc ought to have been able to figure this 
> out. Can this be fixed somehow, without losing the strength of the 
> checking here? I think we should not change BUILD_BUG_ON() to make it 
> less useful.
> 
> 	Ingo

The BUILD_BUG_ON I proposed is not less useful. Actually with current
BUILD_BUG_ON above code does nothing, since fbit is a function parameter
it will translate to nothing. Certainly it will not check the size of passed
parameter, since that was converted on the stack to unsigned int. So my
definition will catch the bad programing here.

If the user of virtio_has_feature() must pass a compile-time constant then
it must be converted to a MACRO, and then the BUILD_BUG_ON will work.
Or it should be changed to a BUG_ON() if fbit is a runtime variable.

>From what I understood this is what you wanted, some way to make sure
a BUILD_BUG_ON will check that the check is actually useful (compile-time)
and is not silently ignored.

Same thing at the other place I sent.

Boaz
--
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/