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 > wrote: > > > > On Thu, Apr 11, 2019 at 11:47 AM Kees Cook wrote: > > > > > > On Wed, Apr 10, 2019 at 5:56 PM Sinan Kaya 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 > > > > Reviewed-by: Kees Cook > > > > > > 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.
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
Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default
On Wed, Apr 10, 2019 at 10:34 PM Masahiro Yamada wrote: > > On Thu, Apr 11, 2019 at 11:47 AM Kees Cook wrote: > > > > On Wed, Apr 10, 2019 at 5:56 PM Sinan Kaya 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 > > > Reviewed-by: Kees Cook > > > > Masahiro, should this go via your tree, or somewhere else? > > > I think somewhere else. Okay. Andrew, can you take this? -- Kees Cook
Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default
On Thu, Apr 11, 2019 at 2:44 PM Sinan Kaya 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:: 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
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]
Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default
On Thu, Apr 11, 2019 at 11:47 AM Kees Cook wrote: > > On Wed, Apr 10, 2019 at 5:56 PM Sinan Kaya 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 > > Reviewed-by: Kees Cook > > Masahiro, should this go via your tree, or somewhere else? I think somewhere else. -- Best Regards Masahiro Yamada
Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default
On Thu, Apr 11, 2019 at 9:59 AM Sinan Kaya 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 > Reviewed-by: Kees Cook > --- > 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
Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default
On Wed, Apr 10, 2019 at 5:56 PM Sinan Kaya 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 > Reviewed-by: Kees Cook 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
