linux-kernel - Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption

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, 25 Apr 2016 15:33:02 +0100
From:	Robin Murphy <robin.murphy@....com>
To:	Matt Fleming <matt@...eblueprint.co.uk>,
	Ard Biesheuvel <ard.biesheuvel@...aro.org>
CC:	Mark Rutland <mark.rutland@....com>,
	"linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Leif Lindholm <leif.lindholm@...aro.org>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"hpa@...or.com" <hpa@...or.com>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption

On 25/04/16 15:24, Matt Fleming wrote:
> On Mon, 25 Apr, at 04:18:41PM, Ard Biesheuvel wrote:
>> On 25 April 2016 at 16:15, Matt Fleming <matt@...eblueprint.co.uk> wrote:
>>> On Mon, 25 Apr, at 03:12:01PM, Robin Murphy wrote:
>>>>> +static void efi_call_virt_check_flags(unsigned long flags, const char *call)
>>>>> +{
>>>>> +    unsigned long cur_flags;
>>>>> +    bool mismatch;
>>>>> +
>>>>> +    local_save_flags(cur_flags);
>>>>> +
>>>>> +    mismatch = !!((cur_flags ^ flags) & ARCH_EFI_IRQ_FLAGS_MASK);
>>>>
>>>> nit: the assignment itself is already a conversion to bool, so the
>>>> excitement is redundant here.
>>>
>>> This was intentional. I asked Mark to make this change so that it's
>>> explicit for the developer that we're performing the type conversion.
>>
>> But replacing an implicit boolean cast with an explicit one makes
>> little sense, no? Don't we simply want '!= 0' here if you need a
>> boolean expression?
>
> Aha but '!!' is fewer characters to type!!
>
> I'm not that bothered as long as we don't stuff an int into a bool
> without giving the programmer some idea we're doing that. It's not
> about the compiler getting it wrong, more about a developer
> introducing a bug when they change the code in the future.
>
> Unless anyone objects, I'll fix this up to use '!= 0' when I apply it.

Agreed - the belt and braces approach isn't necessarily bad if the cost
of cocking it up is significant, and !=0 is as explicit as you can get.
After all, if Joe Random Hacker can't infer the behaviour from looking 4
lines up to see the variable definition, then I wouldn't count on him
understanding !! either ;)

Thanks,
Robin.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.