[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1603884360.3426.1554933853589.JavaMail.zimbra@efficios.com>
Date: Wed, 10 Apr 2019 18:04:13 -0400 (EDT)
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Sinan Kaya <Okaya@...nel.org>
Cc: Kees Cook <keescook@...omium.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
Masahiro Yamada <yamada.masahiro@...ionext.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Johannes Weiner <hannes@...xchg.org>,
Peter Zijlstra <peterz@...radead.org>,
Nicholas Piggin <npiggin@...il.com>, gor <gor@...ux.ibm.com>,
Adrian Reber <adrian@...as.de>,
Richard Guy Briggs <rgb@...hat.com>
Subject: Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default
----- On Apr 10, 2019, at 5:53 PM, Sinan Kaya Okaya@...nel.org wrote:
> On 4/10/2019 5:45 PM, Kees Cook wrote:
>> On Wed, Apr 10, 2019 at 2:26 PM Sinan Kaya <okaya@...nel.org> wrote:
>>>
>>> We can't seem to have a kernel with CONFIG_EXPERT set but
>>> CONFIG_DEBUG_KERNEL unset these days.
>>>
>>> While some of the features under the CONFIG_EXPERT require
>>> CONFIG_DEBUG_KERNEL, it doesn't apply for all features.
>>>
>>> The meaning of CONFIG_EXPERT and CONFIG_DEBUG_KERNEL has been
>>> mixed here.
>>
>> I don't agree: the point of EXPERT is to show _everything_, which
>> means DEBUG_KERNEL should be selected to show those options as well. I
>> think this is fine as-is. What is the problem you want to solve?
>>
>> I think of it as low (nothing selected) medium (DEBUG_KERNEL) and high
>> (EXPERT and DEBUG_KERNEL). So EXPERT enables DEBUG_KERNEL too.
>>
>
> Sure, let's see if there is a better option.
>
> I don't want any of the debug features in my kernel but still
> need all the expert features. My kernel is considered a production
> kernel. I don't really want to ship all the good debug enables.
>
> On the other hand, I need the features under CONFIG_EXPERT to have
> a functional system.
>
> Let's take "multiple users" as an example.
>
> What's the point of having a kernel without multiple users? :)
>
> I don't see the relationship between CONFIG_DEBUG and CONFIG_EXPERT
> as none of the features except KALLSYMS depend on it. If there was
> a compile time dependency, I'd say move it to the things that need
> it as this patch suggests.
>
> P.S. I found a circular dependency now. I can respin the patch based
> on feedback.
I think part of the issue here is that a few .c/.S files use CONFIG_DEBUG_KERNEL
as #ifdef directly, which I'm not sure was meant to be. For instance:
arch/powerpc/kernel/sysfs.c:
#ifdef CONFIG_DEBUG_KERNEL
SYSFS_SPRSETUP(hid0, SPRN_HID0);
SYSFS_SPRSETUP(hid1, SPRN_HID1);
SYSFS_SPRSETUP(hid4, SPRN_HID4);
SYSFS_SPRSETUP(hid5, SPRN_HID5);
SYSFS_SPRSETUP(ima0, SPRN_PA6T_IMA0);
SYSFS_SPRSETUP(ima1, SPRN_PA6T_IMA1);
SYSFS_SPRSETUP(ima2, SPRN_PA6T_IMA2);
SYSFS_SPRSETUP(ima3, SPRN_PA6T_IMA3);
SYSFS_SPRSETUP(ima4, SPRN_PA6T_IMA4);
SYSFS_SPRSETUP(ima5, SPRN_PA6T_IMA5);
SYSFS_SPRSETUP(ima6, SPRN_PA6T_IMA6);
SYSFS_SPRSETUP(ima7, SPRN_PA6T_IMA7);
SYSFS_SPRSETUP(ima8, SPRN_PA6T_IMA8);
SYSFS_SPRSETUP(ima9, SPRN_PA6T_IMA9);
SYSFS_SPRSETUP(imaat, SPRN_PA6T_IMAAT);
SYSFS_SPRSETUP(btcr, SPRN_PA6T_BTCR);
SYSFS_SPRSETUP(pccr, SPRN_PA6T_PCCR);
SYSFS_SPRSETUP(rpccr, SPRN_PA6T_RPCCR);
SYSFS_SPRSETUP(der, SPRN_PA6T_DER);
SYSFS_SPRSETUP(mer, SPRN_PA6T_MER);
SYSFS_SPRSETUP(ber, SPRN_PA6T_BER);
SYSFS_SPRSETUP(ier, SPRN_PA6T_IER);
SYSFS_SPRSETUP(sier, SPRN_PA6T_SIER);
SYSFS_SPRSETUP(siar, SPRN_PA6T_SIAR);
SYSFS_SPRSETUP(tsr0, SPRN_PA6T_TSR0);
SYSFS_SPRSETUP(tsr1, SPRN_PA6T_TSR1);
SYSFS_SPRSETUP(tsr2, SPRN_PA6T_TSR2);
SYSFS_SPRSETUP(tsr3, SPRN_PA6T_TSR3);
#endif /* CONFIG_DEBUG_KERNEL */
arch/mips/kernel/setup.c:
#if defined(CONFIG_DEBUG_KERNEL) && defined(CONFIG_DEBUG_INFO)
/*
* This information is necessary when debugging the kernel
* But is a security vulnerability otherwise!
*/
show_kernel_relocation(KERN_INFO);
#endif
net/netfilter/core.c:
static void hooks_validate(const struct nf_hook_entries *hooks)
{
#ifdef CONFIG_DEBUG_KERNEL
struct nf_hook_ops **orig_ops;
int prio = INT_MIN;
size_t i = 0;
orig_ops = nf_hook_entries_get_hook_ops(hooks);
for (i = 0; i < hooks->num_hook_entries; i++) {
if (orig_ops[i] == &dummy_ops)
continue;
WARN_ON(orig_ops[i]->priority < prio);
if (orig_ops[i]->priority > prio)
prio = orig_ops[i]->priority;
}
#endif
}
and also:
arch/xtensa/kernel/smp.c
arch/xtensa/kernel/entry.S
I was under the impression that config DEBUG_KERNEL was only making a
"group" of menu entries visible without any direct impact on the code,
but it does not appear to be the case for a few exceptions. Perhaps this
is the actual issue ? (and lack of documentation of this Kconfig entry)
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Powered by blists - more mailing lists