linux-kernel - Re: [PATCH] Kbuild: Check for CONFIG_READABLE

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: <20141007121356.GA18003@sepie.suse.cz>
Date:	Tue, 7 Oct 2014 14:13:56 +0200
From:	Michal Marek <mmarek@...e.cz>
To:	Borislav Petkov <bp@...en8.de>, linux-kbuild@...r.kernel.org
Cc:	LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>
Subject: Re: [PATCH] Kbuild: Check for CONFIG_READABLE_ASM when building .s
 targets

On 2014-10-05 17:58, Borislav Petkov wrote:
> On Sun, Oct 05, 2014 at 05:32:34PM +0200, Borislav Petkov wrote:
>> From: Borislav Petkov <bp@...e.de>
>>
>> We do need to look at the asm output of c files for various reasons. In
>> order to do so, we do make <path-to-filename>.s.
>>
>> However, it can happen that the Kconfig option which enables the
>> creation of readable asm CONFIG_READABLE_ASM is disabled. What is more,
>> we want this option enabled because it indirectly enables the issue of
>> line numbers in the asm done by gcc's switch -g.
> 
> Not really: so this is enabled by CONFIG_DEBUG_INFO and there are two
> options: we make CONFIG_READABLE_ASM depend on it which makes people
> answer a couple more questions or we simply add it to the command
> building the .s file.
> 
> I say we do the second thing:
> 
> --
> From: Borislav Petkov <bp@...e.de>
> Subject: [PATCH -v1.1] Kbuild: Check for CONFIG_READABLE_ASM when building .s targets
> 
> We do need to look at the asm output of c files for various reasons. In
> order to do so, we do make <path-to-filename>.s.
> 
> However, it can happen that the Kconfig option which enables the
> creation of readable asm CONFIG_READABLE_ASM is disabled.
> 
> So issue a warning that the asm output might not be optimally readable
> if that option is disabled.
> 
> Additionally, add the -g switch to the .s build command so that gcc
> issues line numbers too.

This violates the principle of least surprise:

make $file.s
as -o $file.o $file.s

should be equivalent to

make $file.o

Why not simply check both READABLE_ASM and DEBUG_INFO?  Also, it's more
straightforward to print the warning in the top-level Makefile rule than
to add a conditional to the generic rule, like this:

diff --git a/Makefile b/Makefile
index 106f300..2b4f62f 100644
--- a/Makefile
+++ b/Makefile
@@ -1505,6 +1505,9 @@ else
 endif
 
 %.s: %.c prepare scripts FORCE
+ifeq ($(CONFIG_READABLE_ASM)$(CONFIG_DEBUG_INFO),)
+       $(warning Enable CONFIG_READABLE_ASM and CONFIG_DEBUG_INFO more helpful asm)
+endif
        $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
 %.i: %.c prepare scripts FORCE
        $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)

I wonder if the whole check shouldn't be wrapped in an ifdef
CONFIG_DEBUG_KERNEL.

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