Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default

[Mathieu Desnoyers]

----- On Apr 10, 2019, at 5:53 PM, Sinan Kaya Okaya@xxxxxxxxxx wrote:

> On 4/10/2019 5:45 PM, Kees Cook wrote:
>> On Wed, Apr 10, 2019 at 2:26 PM Sinan Kaya <okaya@xxxxxxxxxx> 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