2019-04-11 00:57:44

by Sinan Kaya

[permalink] [raw]
Subject: [PATCH v3] init: Do not select DEBUG_KERNEL by default

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.

It looks like CONFIG_KALLSYMS_ALL is the only feature that
requires CONFIG_DEBUG_KERNEL.

Select CONFIG_EXPERT when CONFIG_DEBUG_KERNEL is chosen but
you can still choose CONFIG_EXPERT without CONFIG_DEBUG_KERNEL.

Signed-off-by: Sinan Kaya <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
---
init/Kconfig | 2 --
lib/Kconfig.debug | 1 +
2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 4592bf7997c0..37e10a8391a3 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
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 0d9e81779e37..9fbf3499ec8d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -434,6 +434,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.
--
2.21.0


2019-04-11 02:48:15

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default

On Wed, Apr 10, 2019 at 5:56 PM Sinan Kaya <[email protected]> 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.
>
> It looks like CONFIG_KALLSYMS_ALL is the only feature that
> requires CONFIG_DEBUG_KERNEL.
>
> Select CONFIG_EXPERT when CONFIG_DEBUG_KERNEL is chosen but
> you can still choose CONFIG_EXPERT without CONFIG_DEBUG_KERNEL.
>
> Signed-off-by: Sinan Kaya <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>

Masahiro, should this go via your tree, or somewhere else?

Thanks!

-Kees

> ---
> init/Kconfig | 2 --
> lib/Kconfig.debug | 1 +
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 4592bf7997c0..37e10a8391a3 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
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 0d9e81779e37..9fbf3499ec8d 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -434,6 +434,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.
> --
> 2.21.0
>


--
Kees Cook

2019-04-11 05:33:40

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default

On Thu, Apr 11, 2019 at 9:59 AM Sinan Kaya <[email protected]> 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.
>
> It looks like CONFIG_KALLSYMS_ALL is the only feature that
> requires CONFIG_DEBUG_KERNEL.

Which part of KALLSYMS_ALL code requires CONFIG_DEBUG_KERNEL?



> Select CONFIG_EXPERT when CONFIG_DEBUG_KERNEL is chosen but
> you can still choose CONFIG_EXPERT without CONFIG_DEBUG_KERNEL.
>
> Signed-off-by: Sinan Kaya <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> ---
> init/Kconfig | 2 --
> lib/Kconfig.debug | 1 +
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 4592bf7997c0..37e10a8391a3 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
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 0d9e81779e37..9fbf3499ec8d 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -434,6 +434,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.
> --
> 2.21.0
>


--
Best Regards

Masahiro Yamada

2019-04-11 05:36:48

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default

On Thu, Apr 11, 2019 at 11:47 AM Kees Cook <[email protected]> wrote:
>
> On Wed, Apr 10, 2019 at 5:56 PM Sinan Kaya <[email protected]> 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.
> >
> > It looks like CONFIG_KALLSYMS_ALL is the only feature that
> > requires CONFIG_DEBUG_KERNEL.
> >
> > Select CONFIG_EXPERT when CONFIG_DEBUG_KERNEL is chosen but
> > you can still choose CONFIG_EXPERT without CONFIG_DEBUG_KERNEL.
> >
> > Signed-off-by: Sinan Kaya <[email protected]>
> > Reviewed-by: Kees Cook <[email protected]>
>
> Masahiro, should this go via your tree, or somewhere else?


I think somewhere else.


--
Best Regards
Masahiro Yamada

2019-04-11 05:44:46

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default

On 4/11/2019 1:31 AM, Masahiro Yamada wrote:
>> t looks like CONFIG_KALLSYMS_ALL is the only feature that
>> requires CONFIG_DEBUG_KERNEL.
> Which part of KALLSYMS_ALL code requires CONFIG_DEBUG_KERNEL?
>

I was going by what Kconfig tells me

Symbol: KALLSYMS_ALL [=n]
Depends on: DEBUG_KERNEL [=n] && KALLSYMS [=y]



2019-04-11 05:50:20

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default

On Thu, Apr 11, 2019 at 2:44 PM Sinan Kaya <[email protected]> wrote:
>
> On 4/11/2019 1:31 AM, Masahiro Yamada wrote:
> >> t looks like CONFIG_KALLSYMS_ALL is the only feature that
> >> requires CONFIG_DEBUG_KERNEL.
> > Which part of KALLSYMS_ALL code requires CONFIG_DEBUG_KERNEL?
> >
>
> I was going by what Kconfig tells me
>
> Symbol: KALLSYMS_ALL [=n]
> Depends on: DEBUG_KERNEL [=n] && KALLSYMS [=y]

Lots of features have 'depends on DEBUG_KERNEL'.
What is special about KALLSYMS_ALL here?


./drivers/gpio/Kconfig:52: depends on DEBUG_KERNEL
./drivers/pci/Kconfig:69: depends on DEBUG_KERNEL
./drivers/usb/gadget/Kconfig:51: depends on DEBUG_KERNEL
./drivers/base/Kconfig:119: depends on DEBUG_KERNEL
./drivers/base/Kconfig:130: depends on DEBUG_KERNEL
./drivers/base/Kconfig:142: depends on DEBUG_KERNEL
./drivers/spi/Kconfig:29: depends on DEBUG_KERNEL
./drivers/pinctrl/Kconfig:29: depends on DEBUG_KERNEL
./drivers/gpu/drm/Kconfig:55: depends on DEBUG_KERNEL
./kernel/rcu/Kconfig.debug:16: depends on DEBUG_KERNEL
./kernel/rcu/Kconfig.debug:33: depends on DEBUG_KERNEL
./kernel/rcu/Kconfig.debug:61: depends on DEBUG_KERNEL
./kernel/rcu/Kconfig.debug:73: depends on DEBUG_KERNEL
./net/dccp/Kconfig:30: depends on DEBUG_KERNEL=y
./crypto/Kconfig:173: depends on DEBUG_KERNEL && !CRYPTO_MANAGER_DISABLE_TESTS
./init/Kconfig:951: depends on DEBUG_KERNEL
./init/Kconfig:1476: depends on DEBUG_KERNEL && KALLSYMS
./mm/Kconfig.debug:12: depends on DEBUG_KERNEL
./mm/Kconfig.debug:44: depends on DEBUG_KERNEL && STACKTRACE_SUPPORT
./mm/Kconfig.debug:99: depends on DEBUG_KERNEL
./mm/Kconfig:494: depends on DEBUG_KERNEL && CMA
./lib/Kconfig.kgdb:8: depends on DEBUG_KERNEL
./lib/Kconfig.debug:80: depends on DEBUG_KERNEL && PRINTK &&
GENERIC_CALIBRATE_DELAY
./lib/Kconfig.debug:172: depends on DEBUG_KERNEL && !COMPILE_TEST
./lib/Kconfig.debug:264: depends on DEBUG_KERNEL
./lib/Kconfig.debug:363: depends on DEBUG_KERNEL && (M68K || UML ||
SUPERH) || ARCH_WANT_FRAME_POINTERS
./lib/Kconfig.debug:387: depends on DEBUG_KERNEL
./lib/Kconfig.debug:447: depends on DEBUG_KERNEL
./lib/Kconfig.debug:508: depends on DEBUG_KERNEL && SLAB
./lib/Kconfig.debug:549: depends on DEBUG_KERNEL && HAVE_DEBUG_KMEMLEAK
./lib/Kconfig.debug:614: depends on DEBUG_KERNEL && !IA64
./lib/Kconfig.debug:623: depends on DEBUG_KERNEL
./lib/Kconfig.debug:661: depends on DEBUG_KERNEL && ARCH_HAS_DEBUG_VIRTUAL
./lib/Kconfig.debug:670: depends on DEBUG_KERNEL && !MMU
./lib/Kconfig.debug:712: depends on DEBUG_KERNEL
./lib/Kconfig.debug:723: depends on DEBUG_KERNEL && HIGHMEM
./lib/Kconfig.debug:733: depends on DEBUG_KERNEL && HAVE_DEBUG_STACKOVERFLOW
./lib/Kconfig.debug:802: depends on DEBUG_KERNEL
./lib/Kconfig.debug:816: depends on DEBUG_KERNEL && !S390
./lib/Kconfig.debug:868: depends on DEBUG_KERNEL && !S390
./lib/Kconfig.debug:902: depends on DEBUG_KERNEL
./lib/Kconfig.debug:956: depends on DEBUG_KERNEL
./lib/Kconfig.debug:997: depends on DEBUG_KERNEL && PROC_FS
./lib/Kconfig.debug:1010: depends on DEBUG_KERNEL && PROC_FS
./lib/Kconfig.debug:1023: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1048: depends on DEBUG_KERNEL && PREEMPT &&
TRACE_IRQFLAGS_SUPPORT
./lib/Kconfig.debug:1065: depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
./lib/Kconfig.debug:1111: depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
./lib/Kconfig.debug:1133: depends on DEBUG_KERNEL && RT_MUTEXES
./lib/Kconfig.debug:1140: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1150: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1157: depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
./lib/Kconfig.debug:1174: depends on DEBUG_KERNEL && RWSEM_SPIN_ON_OWNER
./lib/Kconfig.debug:1181: depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
./lib/Kconfig.debug:1196: depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
./lib/Kconfig.debug:1207: depends on DEBUG_KERNEL && LOCKDEP
./lib/Kconfig.debug:1216: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1226: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1237: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1308: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1346: depends on DEBUG_KERNEL || BUG_ON_DATA_CORRUPTION
./lib/Kconfig.debug:1355: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1365: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1375: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1385: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1402: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1417: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1444: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1457: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1536: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1615: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1680: depends on DEBUG_KERNEL || m
./lib/Kconfig.debug:1690: depends on DEBUG_KERNEL || m
./lib/Kconfig.debug:1699: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1710: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1724: depends on DEBUG_KERNEL
./lib/Kconfig.debug:1731: depends on DEBUG_KERNEL
./arch/xtensa/Kconfig.debug:5: depends on DEBUG_KERNEL && MMU
./arch/x86/Kconfig.debug:70: depends on DEBUG_KERNEL
./arch/x86/Kconfig.debug:129: depends on DEBUG_KERNEL
./arch/x86/Kconfig.debug:174: depends on DEBUG_KERNEL && KPROBES
./arch/x86/Kconfig.debug:258: depends on DEBUG_KERNEL
./arch/x86/Kconfig.debug:265: depends on DEBUG_KERNEL
./arch/x86/Kconfig.debug:285: depends on DEBUG_KERNEL
./arch/x86/Kconfig.debug:295: depends on DEBUG_KERNEL && X86_LOCAL_APIC
./arch/x86/Kconfig.debug:319: depends on DEBUG_KERNEL
./arch/ia64/Kconfig.debug:23: depends on DEBUG_KERNEL
./arch/ia64/Kconfig.debug:33: depends on DEBUG_KERNEL
./arch/ia64/Kconfig.debug:42: depends on DEBUG_KERNEL
./arch/ia64/Kconfig.debug:51: depends on DEBUG_KERNEL
./arch/s390/Kconfig.debug:8: depends on DEBUG_KERNEL
./arch/powerpc/Kconfig.debug:57: depends on DEBUG_KERNEL
./arch/powerpc/Kconfig.debug:70: depends on DEBUG_KERNEL &&
JUMP_LABEL_FEATURE_CHECKS
./arch/powerpc/Kconfig.debug:79: depends on DEBUG_KERNEL
./arch/powerpc/Kconfig.debug:83: depends on DEBUG_KERNEL
./arch/powerpc/Kconfig.debug:90: depends on DEBUG_KERNEL
./arch/powerpc/Kconfig.debug:127: depends on DEBUG_KERNEL && PPC32
./arch/powerpc/Kconfig.debug:355: depends on DEBUG_KERNEL && DEBUG_FS
./arch/powerpc/Kconfig.debug:366: depends on DEBUG_KERNEL &&
PPC_BOOK3S_64
./arch/arm/Kconfig.debug:8: depends on DEBUG_KERNEL
./arch/arm/Kconfig.debug:105: depends on DEBUG_KERNEL
./arch/arm64/Kconfig.debug:7: depends on DEBUG_KERNEL
./arch/unicore32/Kconfig.debug:17: depends on DEBUG_KERNEL
./arch/mips/Kconfig.debug:90: depends on DEBUG_KERNEL && SYS_SUPPORTS_ZBOOT
./arch/sh/Kconfig.debug:22: depends on DEBUG_KERNEL && SUPERH32
./arch/sh/Kconfig.debug:31: depends on DEBUG_KERNEL && (MMU || BROKEN)
&& !PAGE_SIZE_64KB
./arch/sh/Kconfig.debug:41: depends on DEBUG_KERNEL && SUPERH32 && BROKEN
./arch/sh/Kconfig.debug:49: depends on DEBUG_KERNEL && SUPERH32
./arch/sh/Kconfig.debug:70: depends on DEBUG_KERNEL



--
Best Regards
Masahiro Yamada

2019-04-11 17:01:46

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default

On Wed, Apr 10, 2019 at 10:34 PM Masahiro Yamada
<[email protected]> wrote:
>
> On Thu, Apr 11, 2019 at 11:47 AM Kees Cook <[email protected]> wrote:
> >
> > On Wed, Apr 10, 2019 at 5:56 PM Sinan Kaya <[email protected]> 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.
> > >
> > > It looks like CONFIG_KALLSYMS_ALL is the only feature that
> > > requires CONFIG_DEBUG_KERNEL.
> > >
> > > Select CONFIG_EXPERT when CONFIG_DEBUG_KERNEL is chosen but
> > > you can still choose CONFIG_EXPERT without CONFIG_DEBUG_KERNEL.
> > >
> > > Signed-off-by: Sinan Kaya <[email protected]>
> > > Reviewed-by: Kees Cook <[email protected]>
> >
> > Masahiro, should this go via your tree, or somewhere else?
>
>
> I think somewhere else.

Okay. Andrew, can you take this?

--
Kees Cook

2019-04-11 18:18:00

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default

On 4/11/2019 1:48 AM, Masahiro Yamada wrote:
>> I was going by what Kconfig tells me
>>
>> Symbol: KALLSYMS_ALL [=n]
>> Depends on: DEBUG_KERNEL [=n] && KALLSYMS [=y]
> Lots of features have 'depends on DEBUG_KERNEL'.
> What is special about KALLSYMS_ALL here?

I had to do some learning about KALLSYSM_ALL. Based on my limited
searching, KALLSYMS_ALL allows you to locate the symbol location
at runtime from the kernel.

Without KALLSYM_ALL, you can only locate the kernel code only. With
KALLSYMS_ALL, you can locate the symbols for any data structure
including kernel modules.

I'm not a KALLSYMS person but based on my search, I'd consider
KALLSYMS_ALL a debug feature as it is today.

kernel/kallsyms.c: else if (IS_ENABLED(CONFIG_KALLSYMS_ALL))
kernel/livepatch/Kconfig: depends on KALLSYMS_ALL
kernel/module.c:#ifdef CONFIG_KALLSYMS_ALL
kernel/module.c:#ifndef CONFIG_KALLSYMS_ALL

lib/Kconfig.debug: select KALLSYMS_ALL
config LOCKDEP
bool
depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
select KALLSYMS
select KALLSYMS_ALL

lib/Kconfig.debug: select KALLSYMS_ALL
config LATENCYTOP
bool "Latency measuring infrastructure"
select KALLSYMS
select KALLSYMS_ALL

scripts/link-vmlinux.sh: if [ -n "${CONFIG_KALLSYMS_ALL}" ]; then

2019-04-11 23:07:08

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default

On Thu, Apr 11, 2019 at 10:00:24AM -0700, Kees Cook wrote:
> On Wed, Apr 10, 2019 at 10:34 PM Masahiro Yamada
> <[email protected]> wrote:
> >
> > On Thu, Apr 11, 2019 at 11:47 AM Kees Cook <[email protected]> wrote:
> > >
> > > On Wed, Apr 10, 2019 at 5:56 PM Sinan Kaya <[email protected]> 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.
> > > >
> > > > It looks like CONFIG_KALLSYMS_ALL is the only feature that
> > > > requires CONFIG_DEBUG_KERNEL.
> > > >
> > > > Select CONFIG_EXPERT when CONFIG_DEBUG_KERNEL is chosen but
> > > > you can still choose CONFIG_EXPERT without CONFIG_DEBUG_KERNEL.
> > > >
> > > > Signed-off-by: Sinan Kaya <[email protected]>
> > > > Reviewed-by: Kees Cook <[email protected]>
> > >
> > > Masahiro, should this go via your tree, or somewhere else?
> >
> >
> > I think somewhere else.
>
> Okay. Andrew, can you take this?

Echoing my comment from the previous thread: this does not seem like the
right solution to me and it introduces a new problem. Please see my
response to v2 for a quick alternative that would address the underlying
bug, which is that some code generation depends on DEBUG_KERNEL.