linux-kernel - Re: [PATCH 07/14] proc/kcore: hide a harmless warning

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, 18 Jul 2017 21:07:29 +0100
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jiri Olsa <jolsa@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Tejun Heo <tj@...nel.org>, Guenter Roeck <linux@...ck-us.net>,
        IDE-ML <linux-ide@...r.kernel.org>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Kees Cook <keescook@...omium.org>,
        Ingo Molnar <mingo@...nel.org>,
        Laura Abbott <labbott@...hat.com>,
        Pratyush Anand <panand@...hat.com>
Subject: Re: [PATCH 07/14] proc/kcore: hide a harmless warning

On 18 July 2017 at 21:01, Arnd Bergmann <arnd@...db.de> wrote:
> On Tue, Jul 18, 2017 at 9:55 PM, Ard Biesheuvel
> <ard.biesheuvel@...aro.org> wrote:
>> On 18 July 2017 at 20:53, Arnd Bergmann <arnd@...db.de> wrote:
>>> On Fri, Jul 14, 2017 at 2:28 PM, Ard Biesheuvel
>>> <ard.biesheuvel@...aro.org> wrote:
>>>> On 14 July 2017 at 10:25, Arnd Bergmann <arnd@...db.de> wrote:
>>>>> gcc warns when MODULES_VADDR/END is defined to the same value as
>>>>> VMALLOC_START/VMALLOC_END, e.g. on x86-32:
>>>>>
>>>>> fs/proc/kcore.c: In function ‘add_modules_range’:
>>>>> fs/proc/kcore.c:622:161: error: self-comparison always evaluates to false [-Werror=tautological-compare]
>>>>>   if (/*MODULES_VADDR != VMALLOC_START && */MODULES_END != VMALLOC_END) {
>>>>>
>>>>
>>>> Does it occur for subtraction as well? Or only for comparison?
>>>
>>> This replacement patch would also address the warning:
>>>
>>> diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
>>> index 45629f4b5402..35824e986c2c 100644
>>> --- a/fs/proc/kcore.c
>>> +++ b/fs/proc/kcore.c
>>> @@ -623,7 +623,7 @@ static void __init proc_kcore_text_init(void)
>>>  struct kcore_list kcore_modules;
>>>  static void __init add_modules_range(void)
>>>  {
>>> -       if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
>>> +       if (MODULES_VADDR - VMALLOC_START && MODULES_END - VMALLOC_END) {
>>>                 kclist_add(&kcore_modules, (void *)MODULES_VADDR,
>>>                         MODULES_END - MODULES_VADDR, KCORE_VMALLOC);
>>>         }
>>>
>>> I have also verified that four of the 14 patches are not needed when building
>>> without ccache, this is one of them:
>>>
>>>  acpi: thermal: fix gcc-6/ccache warning
>>>  proc/kcore: hide a harmless warning
>>>  SFI: fix tautological-compare warning
>>>  [media] fix warning on v4l2_subdev_call() result interpreted as bool
>>>
>>> Not sure what to do with those, we could either ignore them all and
>>> not care about ccache, or we try to address them all in some way.
>>>
>>
>> Any idea why ccache makes a difference here? It is not obvious (not to
>> me at least)
>
> When ccache is used, the compilation stage is apparently always done on
> the preprocessed source file. So instead of parsing (with the integrated
> preprocessor)
>
>           if (MODULES_VADDR != VMALLOC_START ...)
>
> the compiler sees
>
>           if (((unsigned long)high_memory + (8 * 1024 * 1024))  !=
>               ((unsigned long)high_memory + (8 * 1024 * 1024))  ...)
>
> and it correctly considers the first expression something that one
> would write in source code, while -Wtautological-compare
> is intended to warn about the second version being always true,
> which makes the 'if()' pointless.
>

Ah, now it makes sense. I was a bit surprised that
-Wtautological-compare complains about symbolic constants that resolve
to the same expression, but apparently it doesn't.

I see how ccache needs to preprocess first: that is how it notices
changes, by hashing the preprocessed input and comparing it to the
stored hash. I'd still expect it to go back to letting the compiler
preprocess for the actual build, but apparently it doesn't.

A quick google search didn't produce anything useful, but I'd expect
other ccache users to run into the same issue.