Re: [PATCH v2] init: Do not select DEBUG_KERNEL by default
On Thu, Apr 11, 2019 at 06:27:04PM -0400, Sinan Kaya wrote:
> On 4/11/2019 6:21 PM, Kees Cook wrote:
> > > Proposed alternative plan: let's add a new symbol, something like
> > > DEBUG_MISC ("Miscellaneous debug code that should be under a more
> > > specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
> > > "default DEBUG_KERNEL" but allow itself to be turned off, and then
> > > mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
> > > "#ifdef CONFIG_DEBUG_MISC".
> > >
> > > Does that sound like an appropriately rapid solution for this bug?
> > Sure, that sounds fine to me. Sinan can you take care of that for v4?
>
> Sure, let me work on this.
Thank you both! I really appreciate that.
Re: [PATCH v2] init: Do not select DEBUG_KERNEL by default
On 4/11/2019 6:21 PM, Kees Cook wrote:
Proposed alternative plan: let's add a new symbol, something like
DEBUG_MISC ("Miscellaneous debug code that should be under a more
specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
"default DEBUG_KERNEL" but allow itself to be turned off, and then
mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
"#ifdef CONFIG_DEBUG_MISC".
Does that sound like an appropriately rapid solution for this bug?
Sure, that sounds fine to me. Sinan can you take care of that for v4?
Sure, let me work on this.
Re: [PATCH v2] init: Do not select DEBUG_KERNEL by default
On Thu, Apr 11, 2019 at 3:16 PM Josh Triplett wrote:
>
> On Wed, Apr 10, 2019 at 11:13:52PM -0400, Sinan Kaya wrote:
> > On 4/10/2019 11:02 PM, Josh Triplett wrote:
> > > Then let's fix*that*, and get checkpatch to help enforce it in the
> > > future. EXPERT doesn't affect code generation, and neither should this.
> >
> > I think we have to do both. We need to go after the users as well as
> > solve the immediate problem per this patch.
> >
> > As Mathieu identified, CONFIG_DEBUG_KERNEL is being used all over the
> > place and getting subsystem owners to remove let alone add a check
> > to checkpatch is just going to take time.
> >
> > Please let us know if you are OK with this plan.
>
> I'm not OK with this plan. Turning on EXPERT should make the options
> under DEBUG_KERNEL visible; it's a bug that DEBUG_KERNEL affects code
> generation as well.
>
> Proposed alternative plan: let's add a new symbol, something like
> DEBUG_MISC ("Miscellaneous debug code that should be under a more
> specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
> "default DEBUG_KERNEL" but allow itself to be turned off, and then
> mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
> "#ifdef CONFIG_DEBUG_MISC".
>
> Does that sound like an appropriately rapid solution for this bug?
Sure, that sounds fine to me. Sinan can you take care of that for v4?
--
Kees Cook
Re: [PATCH v2] init: Do not select DEBUG_KERNEL by default
On Wed, Apr 10, 2019 at 11:13:52PM -0400, Sinan Kaya wrote:
> On 4/10/2019 11:02 PM, Josh Triplett wrote:
> > Then let's fix*that*, and get checkpatch to help enforce it in the future.
> > EXPERT doesn't affect code generation, and neither should this.
>
> I think we have to do both. We need to go after the users as well as
> solve the immediate problem per this patch.
>
> As Mathieu identified, CONFIG_DEBUG_KERNEL is being used all over the
> place and getting subsystem owners to remove let alone add a check
> to checkpatch is just going to take time.
>
> Please let us know if you are OK with this plan.
I'm not OK with this plan. Turning on EXPERT should make the options
under DEBUG_KERNEL visible; it's a bug that DEBUG_KERNEL affects code
generation as well.
Proposed alternative plan: let's add a new symbol, something like
DEBUG_MISC ("Miscellaneous debug code that should be under a more
specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
"default DEBUG_KERNEL" but allow itself to be turned off, and then
mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
"#ifdef CONFIG_DEBUG_MISC".
Does that sound like an appropriately rapid solution for this bug?
Re: [PATCH v2] init: Do not select DEBUG_KERNEL by default
On 4/10/19 8:02 PM, Josh Triplett wrote: > On April 10, 2019 4:24:18 PM PDT, Kees Cook wrote: >> On Wed, Apr 10, 2019 at 4:22 PM Josh Triplett >> wrote: >>> >>> On April 10, 2019 3:58:55 PM PDT, Kees Cook >> wrote: On Wed, Apr 10, 2019 at 3:42 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 is chosen but you can Typo: CONFIG_DEBUG_KERNEL > still choose CONFIG_EXPERT without CONFIG_DEBUG. same. > > Signed-off-by: Sinan Kaya But with those fixed, looks good to me. Adding Josh (and others) to >> CC since he originally added the linkage to EXPERT in commit f505c553dbe2. >>> >>> CONFIG_DEBUG_KERNEL shouldn't affect code generation in any way; it >> should only make more options appear in kconfig. I originally added >> this to ensure that features you might want to *disable* aren't hidden, >> as part of the tinification effort. >>> >>> What specific problem does having CONFIG_DEBUG_KERNEL enabled cause >> for you? I'd still prefer to have a single switch for "don't hide >> things I might want to disable", rather than several. >> >> See earlier in the thread: code generation depends on >> CONFIG_DEBUG_KERNEL now unfortunately. > > Then let's fix *that*, and get checkpatch to help enforce it in the future. > EXPERT doesn't affect code generation, and neither should this. > checkpatch is not an enforcer. It takes maintainers to do that. -- ~Randy
Re: [PATCH v2] init: Do not select DEBUG_KERNEL by default
On 4/10/2019 11:02 PM, Josh Triplett wrote: Then let's fix*that*, and get checkpatch to help enforce it in the future. EXPERT doesn't affect code generation, and neither should this. I think we have to do both. We need to go after the users as well as solve the immediate problem per this patch. As Mathieu identified, CONFIG_DEBUG_KERNEL is being used all over the place and getting subsystem owners to remove let alone add a check to checkpatch is just going to take time. Please let us know if you are OK with this plan.
Re: [PATCH v2] init: Do not select DEBUG_KERNEL by default
On April 10, 2019 4:24:18 PM PDT, Kees Cook wrote: >On Wed, Apr 10, 2019 at 4:22 PM Josh Triplett >wrote: >> >> On April 10, 2019 3:58:55 PM PDT, Kees Cook >wrote: >> >On Wed, Apr 10, 2019 at 3:42 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 is chosen but you can >> > >> >Typo: CONFIG_DEBUG_KERNEL >> > >> >> still choose CONFIG_EXPERT without CONFIG_DEBUG. >> > >> >same. >> > >> >> >> >> Signed-off-by: Sinan Kaya >> > >> >But with those fixed, looks good to me. Adding Josh (and others) to >CC >> >since he originally added the linkage to EXPERT in commit >> >f505c553dbe2. >> >> CONFIG_DEBUG_KERNEL shouldn't affect code generation in any way; it >should only make more options appear in kconfig. I originally added >this to ensure that features you might want to *disable* aren't hidden, >as part of the tinification effort. >> >> What specific problem does having CONFIG_DEBUG_KERNEL enabled cause >for you? I'd still prefer to have a single switch for "don't hide >things I might want to disable", rather than several. > >See earlier in the thread: code generation depends on >CONFIG_DEBUG_KERNEL now unfortunately. Then let's fix *that*, and get checkpatch to help enforce it in the future. EXPERT doesn't affect code generation, and neither should this.
Re: [PATCH v2] init: Do not select DEBUG_KERNEL by default
On Wed, Apr 10, 2019 at 4:22 PM Josh Triplett wrote: > > On April 10, 2019 3:58:55 PM PDT, Kees Cook wrote: > >On Wed, Apr 10, 2019 at 3:42 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 is chosen but you can > > > >Typo: CONFIG_DEBUG_KERNEL > > > >> still choose CONFIG_EXPERT without CONFIG_DEBUG. > > > >same. > > > >> > >> Signed-off-by: Sinan Kaya > > > >But with those fixed, looks good to me. Adding Josh (and others) to CC > >since he originally added the linkage to EXPERT in commit > >f505c553dbe2. > > CONFIG_DEBUG_KERNEL shouldn't affect code generation in any way; it should > only make more options appear in kconfig. I originally added this to ensure > that features you might want to *disable* aren't hidden, as part of the > tinification effort. > > What specific problem does having CONFIG_DEBUG_KERNEL enabled cause for you? > I'd still prefer to have a single switch for "don't hide things I might want > to disable", rather than several. See earlier in the thread: code generation depends on CONFIG_DEBUG_KERNEL now unfortunately. -- Kees Cook
Re: [PATCH v2] init: Do not select DEBUG_KERNEL by default
On April 10, 2019 3:58:55 PM PDT, Kees Cook wrote: >On Wed, Apr 10, 2019 at 3:42 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 is chosen but you can > >Typo: CONFIG_DEBUG_KERNEL > >> still choose CONFIG_EXPERT without CONFIG_DEBUG. > >same. > >> >> Signed-off-by: Sinan Kaya > >But with those fixed, looks good to me. Adding Josh (and others) to CC >since he originally added the linkage to EXPERT in commit >f505c553dbe2. CONFIG_DEBUG_KERNEL shouldn't affect code generation in any way; it should only make more options appear in kconfig. I originally added this to ensure that features you might want to *disable* aren't hidden, as part of the tinification effort. What specific problem does having CONFIG_DEBUG_KERNEL enabled cause for you? I'd still prefer to have a single switch for "don't hide things I might want to disable", rather than several. This would also need checking to make sure it doesn't grow tinyconfig.
Re: [PATCH v2] init: Do not select DEBUG_KERNEL by default
On Wed, Apr 10, 2019 at 3:42 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 is chosen but you can Typo: CONFIG_DEBUG_KERNEL > still choose CONFIG_EXPERT without CONFIG_DEBUG. same. > > Signed-off-by: Sinan Kaya But with those fixed, looks good to me. Adding Josh (and others) to CC since he originally added the linkage to EXPERT in commit f505c553dbe2. Reviewed-by: Kees Cook -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
[PATCH v2] 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 is chosen but you can still choose CONFIG_EXPERT without CONFIG_DEBUG. Signed-off-by: Sinan Kaya --- 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
