linux-kernel - Re: [PATCH] genirq: Make - vs ?: precedence explicit

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]

Message-ID: <a3b338a1-95c5-1e7e-9b5c-d924a760078e@prevas.dk>
Date:   Tue, 28 Nov 2017 13:19:50 +0100
From:   Rasmus Villemoes <rasmus.villemoes@...vas.dk>
To:     Kees Cook <keescook@...omium.org>,
        Thomas Gleixner <tglx@...utronix.de>
CC:     <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] genirq: Make - vs ?: precedence explicit

On 2017-11-22 21:56, Kees Cook wrote:
> Noticed with a Clang build. This improves the readability of the ?:
> expression, as it has lower precedence than the - expression. Show
> explicitly that - is evaluated first.
> 
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Signed-off-by: Kees Cook <keescook@...omium.org>
> ---
>  kernel/irq/matrix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/matrix.c b/kernel/irq/matrix.c
> index a3cbbc8191c5..7df2480005f8 100644
> --- a/kernel/irq/matrix.c
> +++ b/kernel/irq/matrix.c
> @@ -384,7 +384,7 @@ unsigned int irq_matrix_available(struct irq_matrix *m, bool cpudown)
>  {
>  	struct cpumap *cm = this_cpu_ptr(m->maps);
>  
> -	return m->global_available - cpudown ? cm->available : 0;
> +	return (m->global_available - cpudown) ? cm->available : 0;
>  }

I see that this got applied, and that doesn't change the semantics of
the code. But surely the code is and was buggy, right? From the kernel
doc, I'm pretty sure the idea is to subtract cm->available if cpudown is
true, otherwise subtract 0, i.e.

  return m->global_available - (cpudown ? cm->available : 0);

Rasmus