* [PATCH v1] init: Do not select DEBUG_KERNEL by default @ 2019-04-10 21:26 Sinan Kaya 2019-04-10 21:45 ` Kees Cook 0 siblings, 1 reply; 11+ messages in thread From: Sinan Kaya @ 2019-04-10 21:26 UTC (permalink / raw) To: linux-kernel Cc: Sinan Kaya, Masahiro Yamada, Andrew Morton, Kees Cook, Johannes Weiner, Peter Zijlstra (Intel), Nicholas Piggin, Mathieu Desnoyers, Vasily Gorbik, Adrian Reber, Richard Guy Briggs 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. It looks like CONFIG_KALLSYMS_ALL is the only feature that requires CONFIG_DEBUG_KERNEL. Move the CONFIG_DEBUG_KERNEL selection to where it really is needed. Signed-off-by: Sinan Kaya <okaya@kernel.org> --- init/Kconfig | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/init/Kconfig b/init/Kconfig index 4592bf7997c0..0a6346feae8d 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1206,8 +1206,6 @@ config BPF menuconfig EXPERT bool "Configure standard kernel features (expert users)" - # Unhide debug options, to make the on-by-default options visible - select DEBUG_KERNEL help This option allows certain base kernel options and settings to be disabled or tweaked. This is for specialized @@ -1473,7 +1471,8 @@ config KALLSYMS config KALLSYMS_ALL bool "Include all symbols in kallsyms" - depends on DEBUG_KERNEL && KALLSYMS + select DEBUG_KERNEL + depends on KALLSYMS help Normally kallsyms only contains the symbols of functions for nicer OOPS messages and backtraces (i.e., symbols from the text and inittext -- 2.21.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default 2019-04-10 21:26 [PATCH v1] init: Do not select DEBUG_KERNEL by default Sinan Kaya @ 2019-04-10 21:45 ` Kees Cook 2019-04-10 21:53 ` Sinan Kaya 0 siblings, 1 reply; 11+ messages in thread From: Kees Cook @ 2019-04-10 21:45 UTC (permalink / raw) To: Sinan Kaya Cc: LKML, Masahiro Yamada, Andrew Morton, Kees Cook, Johannes Weiner, Peter Zijlstra (Intel), Nicholas Piggin, Mathieu Desnoyers, Vasily Gorbik, Adrian Reber, Richard Guy Briggs On Wed, Apr 10, 2019 at 2:26 PM Sinan Kaya <okaya@kernel.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. -- Kees Cook ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default 2019-04-10 21:45 ` Kees Cook @ 2019-04-10 21:53 ` Sinan Kaya 2019-04-10 22:04 ` Mathieu Desnoyers 2019-04-10 22:04 ` Kees Cook 0 siblings, 2 replies; 11+ messages in thread From: Sinan Kaya @ 2019-04-10 21:53 UTC (permalink / raw) To: Kees Cook Cc: LKML, Masahiro Yamada, Andrew Morton, Johannes Weiner, Peter Zijlstra (Intel), Nicholas Piggin, Mathieu Desnoyers, Vasily Gorbik, Adrian Reber, Richard Guy Briggs On 4/10/2019 5:45 PM, Kees Cook wrote: > On Wed, Apr 10, 2019 at 2:26 PM Sinan Kaya <okaya@kernel.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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default 2019-04-10 21:53 ` Sinan Kaya @ 2019-04-10 22:04 ` Mathieu Desnoyers 2019-04-10 22:07 ` Kees Cook 2019-04-10 22:04 ` Kees Cook 1 sibling, 1 reply; 11+ messages in thread From: Mathieu Desnoyers @ 2019-04-10 22:04 UTC (permalink / raw) To: Sinan Kaya Cc: Kees Cook, linux-kernel, Masahiro Yamada, Andrew Morton, Johannes Weiner, Peter Zijlstra, Nicholas Piggin, gor, Adrian Reber, Richard Guy Briggs ----- On Apr 10, 2019, at 5:53 PM, Sinan Kaya Okaya@kernel.org wrote: > On 4/10/2019 5:45 PM, Kees Cook wrote: >> On Wed, Apr 10, 2019 at 2:26 PM Sinan Kaya <okaya@kernel.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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default 2019-04-10 22:04 ` Mathieu Desnoyers @ 2019-04-10 22:07 ` Kees Cook 0 siblings, 0 replies; 11+ messages in thread From: Kees Cook @ 2019-04-10 22:07 UTC (permalink / raw) To: Mathieu Desnoyers Cc: Sinan Kaya, Kees Cook, linux-kernel, Masahiro Yamada, Andrew Morton, Johannes Weiner, Peter Zijlstra, Nicholas Piggin, gor, Adrian Reber, Richard Guy Briggs On Wed, Apr 10, 2019 at 3:04 PM Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote: > > ----- On Apr 10, 2019, at 5:53 PM, Sinan Kaya Okaya@kernel.org wrote: > > > On 4/10/2019 5:45 PM, Kees Cook wrote: > >> On Wed, Apr 10, 2019 at 2:26 PM Sinan Kaya <okaya@kernel.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 This one is unfortunate for sure. :P > 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 > } This seems best to just always enable, neither caller appears to be fast-path. > > 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) Yeah, that's certainly not how it was intended. But under EXPERT, I think there is still an argument to made that it's the right thing to do. -- Kees Cook ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default 2019-04-10 21:53 ` Sinan Kaya 2019-04-10 22:04 ` Mathieu Desnoyers @ 2019-04-10 22:04 ` Kees Cook 2019-04-10 22:18 ` Sinan Kaya 1 sibling, 1 reply; 11+ messages in thread From: Kees Cook @ 2019-04-10 22:04 UTC (permalink / raw) To: Sinan Kaya Cc: LKML, Masahiro Yamada, Andrew Morton, Johannes Weiner, Peter Zijlstra (Intel), Nicholas Piggin, Mathieu Desnoyers, Vasily Gorbik, Adrian Reber, Richard Guy Briggs On Wed, Apr 10, 2019 at 2:54 PM Sinan Kaya <Okaya@kernel.org> wrote: > > On 4/10/2019 5:45 PM, Kees Cook wrote: > > On Wed, Apr 10, 2019 at 2:26 PM Sinan Kaya <okaya@kernel.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. Production kernels enable it. e.g. Ubuntu: $ grep '\bCONFIG_DEBUG_KERNEL\b' /boot/config-$(uname -r) CONFIG_DEBUG_KERNEL=y > 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. CONFIG_DEBUG_KERNEL mainly only enables the visibility of various other options. I can only find two instances of it controlling a "default", and one is overridden by CONFIG_SMP on alpha. :) $ git grep -B2 'default.*DEBUG_KERNEL' arch/alpha/Kconfig.debug-config MATHEMU arch/alpha/Kconfig.debug- tristate "Kernel FP software completion" if DEBUG_KERNEL && !SMP arch/alpha/Kconfig.debug: default y if !DEBUG_KERNEL || SMP -- kernel/trace/Kconfig-menuconfig FTRACE kernel/trace/Kconfig- bool "Tracers" kernel/trace/Kconfig: default y if DEBUG_KERNEL What do you see enabled that CONFIG_DEBUG_KERNEL enables that you don't want? -- Kees Cook ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default 2019-04-10 22:04 ` Kees Cook @ 2019-04-10 22:18 ` Sinan Kaya 2019-04-10 22:21 ` Kees Cook 0 siblings, 1 reply; 11+ messages in thread From: Sinan Kaya @ 2019-04-10 22:18 UTC (permalink / raw) To: Kees Cook Cc: LKML, Masahiro Yamada, Andrew Morton, Johannes Weiner, Peter Zijlstra (Intel), Nicholas Piggin, Mathieu Desnoyers, Vasily Gorbik, Adrian Reber, Richard Guy Briggs On 4/10/2019 6:04 PM, Kees Cook wrote: >> 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. > > Production kernels enable it. e.g. Ubuntu: > $ grep '\bCONFIG_DEBUG_KERNEL\b' /boot/config-$(uname -r) > CONFIG_DEBUG_KERNEL=y > It makes sense for a general purpose operating system. It doesn't apply to my limited case where I'm very concerned about image size. >> 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. > > CONFIG_DEBUG_KERNEL mainly only enables the visibility of various > other options. I can only find two instances of it controlling a > "default", and one is overridden by CONFIG_SMP on alpha. :) > > $ git grep -B2 'default.*DEBUG_KERNEL' > arch/alpha/Kconfig.debug-config MATHEMU > arch/alpha/Kconfig.debug- tristate "Kernel FP software > completion" if DEBUG_KERNEL && !SMP > arch/alpha/Kconfig.debug: default y if !DEBUG_KERNEL || SMP > -- > kernel/trace/Kconfig-menuconfig FTRACE > kernel/trace/Kconfig- bool "Tracers" > kernel/trace/Kconfig: default y if DEBUG_KERNEL If the idea is to just show, nothing should happen based on DEBUG_KERNEL, right? No default selection as in FTRACE, no c/S file changes in the code path as Mathieu identified. I can go after individual enables if you agree assuming Mathieu will go after the changes in the other email. Let me know otherwise. > > What do you see enabled that CONFIG_DEBUG_KERNEL enables that you don't want? > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default 2019-04-10 22:18 ` Sinan Kaya @ 2019-04-10 22:21 ` Kees Cook 2019-04-10 22:25 ` Sinan Kaya 0 siblings, 1 reply; 11+ messages in thread From: Kees Cook @ 2019-04-10 22:21 UTC (permalink / raw) To: Sinan Kaya Cc: LKML, Masahiro Yamada, Andrew Morton, Johannes Weiner, Peter Zijlstra (Intel), Nicholas Piggin, Mathieu Desnoyers, Vasily Gorbik, Adrian Reber, Richard Guy Briggs On Wed, Apr 10, 2019 at 3:18 PM Sinan Kaya <Okaya@kernel.org> wrote: > > On 4/10/2019 6:04 PM, Kees Cook wrote: > > >> 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. > > > > Production kernels enable it. e.g. Ubuntu: > > $ grep '\bCONFIG_DEBUG_KERNEL\b' /boot/config-$(uname -r) > > CONFIG_DEBUG_KERNEL=y > > > > It makes sense for a general purpose operating system. It doesn't apply > to my limited case where I'm very concerned about image size. Gotcha. > > >> 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. > > > > CONFIG_DEBUG_KERNEL mainly only enables the visibility of various > > other options. I can only find two instances of it controlling a > > "default", and one is overridden by CONFIG_SMP on alpha. :) > > > > $ git grep -B2 'default.*DEBUG_KERNEL' > > arch/alpha/Kconfig.debug-config MATHEMU > > arch/alpha/Kconfig.debug- tristate "Kernel FP software > > completion" if DEBUG_KERNEL && !SMP > > arch/alpha/Kconfig.debug: default y if !DEBUG_KERNEL || SMP > > -- > > kernel/trace/Kconfig-menuconfig FTRACE > > kernel/trace/Kconfig- bool "Tracers" > > kernel/trace/Kconfig: default y if DEBUG_KERNEL > > If the idea is to just show, nothing should happen based on > DEBUG_KERNEL, right? > > No default selection as in FTRACE, no c/S file changes in the code > path as Mathieu identified. Yeah, this is clearly a problem. I hadn't looked for code #ifdefs :( > I can go after individual enables if you agree assuming Mathieu will > go after the changes in the other email. Let me know otherwise. How about you split it, but make DEBUG_KERNEL be "default EXPERT" that way enabling EXPERT will enable DEBUG_KERNEL still in the default case? -- Kees Cook ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default 2019-04-10 22:21 ` Kees Cook @ 2019-04-10 22:25 ` Sinan Kaya 2019-04-10 22:28 ` Kees Cook 0 siblings, 1 reply; 11+ messages in thread From: Sinan Kaya @ 2019-04-10 22:25 UTC (permalink / raw) To: Kees Cook Cc: LKML, Masahiro Yamada, Andrew Morton, Johannes Weiner, Peter Zijlstra (Intel), Nicholas Piggin, Mathieu Desnoyers, Vasily Gorbik, Adrian Reber, Richard Guy Briggs On 4/10/2019 6:21 PM, Kees Cook wrote: >> I can go after individual enables if you agree assuming Mathieu will >> go after the changes in the other email. Let me know otherwise. > How about you split it, but make DEBUG_KERNEL be "default EXPERT" that > way enabling EXPERT will enable DEBUG_KERNEL still in the default > case? Sorry, can you explain what you mean by split? Do you mean move the things I need out of EXPERT? or something else? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default 2019-04-10 22:25 ` Sinan Kaya @ 2019-04-10 22:28 ` Kees Cook 2019-04-10 22:29 ` Sinan Kaya 0 siblings, 1 reply; 11+ messages in thread From: Kees Cook @ 2019-04-10 22:28 UTC (permalink / raw) To: Sinan Kaya Cc: LKML, Masahiro Yamada, Andrew Morton, Johannes Weiner, Peter Zijlstra (Intel), Nicholas Piggin, Mathieu Desnoyers, Vasily Gorbik, Adrian Reber, Richard Guy Briggs On Wed, Apr 10, 2019 at 3:25 PM Sinan Kaya <Okaya@kernel.org> wrote: > > On 4/10/2019 6:21 PM, Kees Cook wrote: > >> I can go after individual enables if you agree assuming Mathieu will > >> go after the changes in the other email. Let me know otherwise. > > How about you split it, but make DEBUG_KERNEL be "default EXPERT" that > > way enabling EXPERT will enable DEBUG_KERNEL still in the default > > case? > > Sorry, can you explain what you mean by split? I mean when you disconnect them (i.e. remove the "select" in EXPERT) > Do you mean move the things I need out of EXPERT? or something else? So, for example, this: diff --git a/init/Kconfig b/init/Kconfig index c9386a365eea..7ce4a60ab3e9 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1188,8 +1188,6 @@ config BPF menuconfig EXPERT bool "Configure standard kernel features (expert users)" - # Unhide debug options, to make the on-by-default options visible - select DEBUG_KERNEL help This option allows certain base kernel options and settings to be disabled or tweaked. This is for specialized diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index d4df5b24d75e..6a9bc118b64a 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -433,6 +433,7 @@ config MAGIC_SYSRQ_SERIAL config DEBUG_KERNEL bool "Kernel debugging" + default EXPERT help Say Y here if you are developing drivers or trying to debug and identify kernel problems. Then you shouldn't need the KALLSYMS_ALL change, yes? But maybe I misunderstood that. -- Kees Cook ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default 2019-04-10 22:28 ` Kees Cook @ 2019-04-10 22:29 ` Sinan Kaya 0 siblings, 0 replies; 11+ messages in thread From: Sinan Kaya @ 2019-04-10 22:29 UTC (permalink / raw) To: Kees Cook Cc: LKML, Masahiro Yamada, Andrew Morton, Johannes Weiner, Peter Zijlstra (Intel), Nicholas Piggin, Mathieu Desnoyers, Vasily Gorbik, Adrian Reber, Richard Guy Briggs On 4/10/2019 6:28 PM, Kees Cook wrote: > diff --git a/init/Kconfig b/init/Kconfig > index c9386a365eea..7ce4a60ab3e9 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -1188,8 +1188,6 @@ config BPF > > menuconfig EXPERT > bool "Configure standard kernel features (expert users)" > - # Unhide debug options, to make the on-by-default options visible > - select DEBUG_KERNEL > help > This option allows certain base kernel options and settings > to be disabled or tweaked. This is for specialized > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index d4df5b24d75e..6a9bc118b64a 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -433,6 +433,7 @@ config MAGIC_SYSRQ_SERIAL > > config DEBUG_KERNEL > bool "Kernel debugging" > + default EXPERT > help > Say Y here if you are developing drivers or trying to debug and > identify kernel problems. Got it. Thanks. I'll test and respin v2. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-04-10 22:29 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-04-10 21:26 [PATCH v1] init: Do not select DEBUG_KERNEL by default Sinan Kaya 2019-04-10 21:45 ` Kees Cook 2019-04-10 21:53 ` Sinan Kaya 2019-04-10 22:04 ` Mathieu Desnoyers 2019-04-10 22:07 ` Kees Cook 2019-04-10 22:04 ` Kees Cook 2019-04-10 22:18 ` Sinan Kaya 2019-04-10 22:21 ` Kees Cook 2019-04-10 22:25 ` Sinan Kaya 2019-04-10 22:28 ` Kees Cook 2019-04-10 22:29 ` Sinan Kaya
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox