linux-kernel - Re: linux-4.20-rc1/arch/x86/mm/pageattr.c:366: logical error ?

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

Message-ID: <alpine.DEB.2.21.1811052143320.1659@nanos.tec.linutronix.de>
Date:   Mon, 5 Nov 2018 22:30:45 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     David Binderman <dcb314@...mail.com>
cc:     "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "luto@...nel.org" <luto@...nel.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>, "hpa@...or.com" <hpa@...or.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: linux-4.20-rc1/arch/x86/mm/pageattr.c:366: logical error ?

David,

On Mon, 5 Nov 2018, David Binderman wrote:

> 
> linux-4.20-rc1/arch/x86/mm/pageattr.c:366]: (style) Same expression on both sides of '||' because 'r1_start<=r2_end&&r1_end>=r2_start' and 'r2_start<=r1_end&&r2_end>=r1_start' represent the same value.
>

The compiler is actually right.

The moron who wrote that code obviously thought that checking the same
thing twice makes sure that the result is more correct. :)

> Source code is
> 
> static bool overlaps(unsigned long r1_start, unsigned long r1_end,
>                      unsigned long r2_start, unsigned long r2_end)
> {
>         return (r1_start <= r2_end && r1_end >= r2_start) ||
>                 (r2_start <= r1_end && r2_end >= r1_start);
> }
> 
> Maybe better code
> 
> static bool overlaps(unsigned long r1_start, unsigned long r1_end,
>                      unsigned long r2_start, unsigned long r2_end)
> {
>         /* r1_start inside r2_start .. r2_end
>             or r1_end inside r2_start .. r2_end
>           */
>         return (r1_start >= r2_start && r1_start <= r2_end) ||
>                 (r1_end >= r2_start && r1_end <= r2_end);
> }

That maybe better code fails with these ranges:

r1 0x0100 - 0x01ff
r2 0x0110 - 0x011f

It claims that there is no overlap between the two ranges. I leave it to
you to figure out why it fails.

But the function which the compiler complains about is actually correct for
all variants of input pairs.

It's just having the same condition twice. The second part after the || is
just the reverse expressed, but identical variant of the first.

So the obvious solution is to remove that redundant part.

Thanks,

	tglx