bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
@ 2020-03-31 21:55 Slava Bacherikov
  2020-03-31 22:40 ` KP Singh
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Slava Bacherikov @ 2020-03-31 21:55 UTC (permalink / raw)
  To: andriin
  Cc: keescook, bpf, linux-kernel, jannh, alexei.starovoitov, daniel,
	kernel-hardening, Slava Bacherikov, Liu Yiding

Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
enabled will produce invalid btf file, since gen_btf function in
link-vmlinux.sh script doesn't handle *.dwo files.

Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.

Signed-off-by: Slava Bacherikov <slava@bacher09.org>
Reported-by: Jann Horn <jannh@google.com>
Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
---
 lib/Kconfig.debug | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f61d834e02fe..9ae288e2a6c0 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -223,6 +223,7 @@ config DEBUG_INFO_DWARF4
 config DEBUG_INFO_BTF
 	bool "Generate BTF typeinfo"
 	depends on DEBUG_INFO
+	depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED && !GCC_PLUGIN_RANDSTRUCT
 	help
 	  Generate deduplicated BTF type information from DWARF debug info.
 	  Turning this on expects presence of pahole tool, which will convert
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-03-31 21:55 [PATCH v2 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF Slava Bacherikov
@ 2020-03-31 22:40 ` KP Singh
  2020-04-01  0:03 ` Andrii Nakryiko
  2020-04-01  7:34 ` Kees Cook
  2 siblings, 0 replies; 17+ messages in thread
From: KP Singh @ 2020-03-31 22:40 UTC (permalink / raw)
  To: Slava Bacherikov
  Cc: andriin, keescook, bpf, linux-kernel, jannh, alexei.starovoitov,
	daniel, kernel-hardening, Liu Yiding

On 01-Apr 00:55, Slava Bacherikov wrote:
> Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
> enabled will produce invalid btf file, since gen_btf function in
> link-vmlinux.sh script doesn't handle *.dwo files.
> 
> Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
> using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
> 
> Signed-off-by: Slava Bacherikov <slava@bacher09.org>
> Reported-by: Jann Horn <jannh@google.com>
> Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")

I can say that I also got invalid BTF when I tried using
DEBUG_INFO_REDUCED.

So here's a first one from of these from me :)

Acked-by: KP Singh <kpsingh@google.com>

> ---
>  lib/Kconfig.debug | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f61d834e02fe..9ae288e2a6c0 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -223,6 +223,7 @@ config DEBUG_INFO_DWARF4
>  config DEBUG_INFO_BTF
>  	bool "Generate BTF typeinfo"
>  	depends on DEBUG_INFO
> +	depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED && !GCC_PLUGIN_RANDSTRUCT
>  	help
>  	  Generate deduplicated BTF type information from DWARF debug info.
>  	  Turning this on expects presence of pahole tool, which will convert
> -- 
> 2.24.1
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-03-31 21:55 [PATCH v2 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF Slava Bacherikov
  2020-03-31 22:40 ` KP Singh
@ 2020-04-01  0:03 ` Andrii Nakryiko
  2020-04-01  7:34 ` Kees Cook
  2 siblings, 0 replies; 17+ messages in thread
From: Andrii Nakryiko @ 2020-04-01  0:03 UTC (permalink / raw)
  To: Slava Bacherikov
  Cc: Andrii Nakryiko, Kees Cook, bpf, open list, Jann Horn,
	Alexei Starovoitov, Daniel Borkmann, kernel-hardening,
	Liu Yiding

On Tue, Mar 31, 2020 at 2:57 PM Slava Bacherikov <slava@bacher09.org> wrote:
>
> Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
> enabled will produce invalid btf file, since gen_btf function in
> link-vmlinux.sh script doesn't handle *.dwo files.
>
> Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
> using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
>
> Signed-off-by: Slava Bacherikov <slava@bacher09.org>
> Reported-by: Jann Horn <jannh@google.com>
> Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
> ---

LGTM, but let's wait on Kees about COMPILE_TEST dependency...

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  lib/Kconfig.debug | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f61d834e02fe..9ae288e2a6c0 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -223,6 +223,7 @@ config DEBUG_INFO_DWARF4
>  config DEBUG_INFO_BTF
>         bool "Generate BTF typeinfo"
>         depends on DEBUG_INFO
> +       depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED && !GCC_PLUGIN_RANDSTRUCT
>         help
>           Generate deduplicated BTF type information from DWARF debug info.
>           Turning this on expects presence of pahole tool, which will convert
> --
> 2.24.1
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-03-31 21:55 [PATCH v2 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF Slava Bacherikov
  2020-03-31 22:40 ` KP Singh
  2020-04-01  0:03 ` Andrii Nakryiko
@ 2020-04-01  7:34 ` Kees Cook
  2020-04-01 14:20   ` [PATCH v3 " Slava Bacherikov
  2 siblings, 1 reply; 17+ messages in thread
From: Kees Cook @ 2020-04-01  7:34 UTC (permalink / raw)
  To: Slava Bacherikov
  Cc: andriin, bpf, linux-kernel, jannh, alexei.starovoitov, daniel,
	kernel-hardening, Liu Yiding

On Wed, Apr 01, 2020 at 12:55:37AM +0300, Slava Bacherikov wrote:
> Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
> enabled will produce invalid btf file, since gen_btf function in
> link-vmlinux.sh script doesn't handle *.dwo files.
> 
> Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
> using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
> 
> Signed-off-by: Slava Bacherikov <slava@bacher09.org>
> Reported-by: Jann Horn <jannh@google.com>
> Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
> ---
>  lib/Kconfig.debug | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f61d834e02fe..9ae288e2a6c0 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -223,6 +223,7 @@ config DEBUG_INFO_DWARF4
>  config DEBUG_INFO_BTF
>  	bool "Generate BTF typeinfo"
>  	depends on DEBUG_INFO
> +	depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED && !GCC_PLUGIN_RANDSTRUCT
>  	help
>  	  Generate deduplicated BTF type information from DWARF debug info.
>  	  Turning this on expects presence of pahole tool, which will convert

Please make this:

depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
depends on COMPILE_TEST || !GCC_PLUGIN_RANDSTRUCT

-Kees

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-01  7:34 ` Kees Cook
@ 2020-04-01 14:20   ` Slava Bacherikov
  2020-04-01 14:37     ` Slava Bacherikov
  2020-04-01 15:49     ` Kees Cook
  0 siblings, 2 replies; 17+ messages in thread
From: Slava Bacherikov @ 2020-04-01 14:20 UTC (permalink / raw)
  To: keescook
  Cc: andriin, bpf, linux-kernel, jannh, alexei.starovoitov, daniel,
	kernel-hardening, liuyd.fnst, Slava Bacherikov, KP Singh

Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
enabled will produce invalid btf file, since gen_btf function in
link-vmlinux.sh script doesn't handle *.dwo files.

Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.

Signed-off-by: Slava Bacherikov <slava@bacher09.org>
Reported-by: Jann Horn <jannh@google.com>
Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
Acked-by: KP Singh <kpsingh@google.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
---
 lib/Kconfig.debug | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f61d834e02fe..b94227be2d62 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -222,7 +222,9 @@ config DEBUG_INFO_DWARF4
 
 config DEBUG_INFO_BTF
 	bool "Generate BTF typeinfo"
-	depends on DEBUG_INFO
+	depends on DEBUG_INFO || COMPILE_TEST
+	depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
+	depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
 	help
 	  Generate deduplicated BTF type information from DWARF debug info.
 	  Turning this on expects presence of pahole tool, which will convert

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-01 14:20   ` [PATCH v3 " Slava Bacherikov
@ 2020-04-01 14:37     ` Slava Bacherikov
  2020-04-01 17:46       ` Andrii Nakryiko
  2020-04-01 15:49     ` Kees Cook
  1 sibling, 1 reply; 17+ messages in thread
From: Slava Bacherikov @ 2020-04-01 14:37 UTC (permalink / raw)
  To: keescook
  Cc: andriin, bpf, linux-kernel, jannh, alexei.starovoitov, daniel,
	kernel-hardening, liuyd.fnst, KP Singh



01.04.2020 17:20, Slava Bacherikov wrotes:
> Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
> enabled will produce invalid btf file, since gen_btf function in
> link-vmlinux.sh script doesn't handle *.dwo files.
> 
> Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
> using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
> 
> Signed-off-by: Slava Bacherikov <slava@bacher09.org>
> Reported-by: Jann Horn <jannh@google.com>
> Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
> Acked-by: KP Singh <kpsingh@google.com>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
> ---
>  lib/Kconfig.debug | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f61d834e02fe..b94227be2d62 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -222,7 +222,9 @@ config DEBUG_INFO_DWARF4
>  
>  config DEBUG_INFO_BTF
>  	bool "Generate BTF typeinfo"
> -	depends on DEBUG_INFO
> +	depends on DEBUG_INFO || COMPILE_TEST
I had to add this, since DEBUG_INFO which depends on:

	DEBUG_KERNEL && !COMPILE_TEST

would block DEBUG_INFO_BTF when COMPILE_TEST is turned on.

In that case allyesconfig will emit both:

CONFIG_DEBUG_INFO_BTF=y
CONFIG_GCC_PLUGIN_RANDSTRUCT=y



> +	depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
> +	depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
>  	help
>  	  Generate deduplicated BTF type information from DWARF debug info.
>  	  Turning this on expects presence of pahole tool, which will convert
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-01 14:20   ` [PATCH v3 " Slava Bacherikov
  2020-04-01 14:37     ` Slava Bacherikov
@ 2020-04-01 15:49     ` Kees Cook
  2020-04-02 15:33       ` [PATCH v4 " Slava Bacherikov
  1 sibling, 1 reply; 17+ messages in thread
From: Kees Cook @ 2020-04-01 15:49 UTC (permalink / raw)
  To: Slava Bacherikov
  Cc: andriin, bpf, linux-kernel, jannh, alexei.starovoitov, daniel,
	kernel-hardening, liuyd.fnst, KP Singh

On Wed, Apr 01, 2020 at 05:20:58PM +0300, Slava Bacherikov wrote:
> Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
> enabled will produce invalid btf file, since gen_btf function in
> link-vmlinux.sh script doesn't handle *.dwo files.
> 
> Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
> using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
> 
> Signed-off-by: Slava Bacherikov <slava@bacher09.org>

Very cool; thanks! :)

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> Reported-by: Jann Horn <jannh@google.com>
> Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
> Acked-by: KP Singh <kpsingh@google.com>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
> ---
>  lib/Kconfig.debug | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f61d834e02fe..b94227be2d62 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -222,7 +222,9 @@ config DEBUG_INFO_DWARF4
>  
>  config DEBUG_INFO_BTF
>  	bool "Generate BTF typeinfo"
> -	depends on DEBUG_INFO
> +	depends on DEBUG_INFO || COMPILE_TEST
> +	depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
> +	depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
>  	help
>  	  Generate deduplicated BTF type information from DWARF debug info.
>  	  Turning this on expects presence of pahole tool, which will convert

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-01 14:37     ` Slava Bacherikov
@ 2020-04-01 17:46       ` Andrii Nakryiko
  2020-04-01 18:24         ` Slava Bacherikov
  0 siblings, 1 reply; 17+ messages in thread
From: Andrii Nakryiko @ 2020-04-01 17:46 UTC (permalink / raw)
  To: Slava Bacherikov
  Cc: Kees Cook, Andrii Nakryiko, bpf, open list, Jann Horn,
	Alexei Starovoitov, Daniel Borkmann, Kernel Hardening,
	Liu Yiding, KP Singh

On Wed, Apr 1, 2020 at 7:38 AM Slava Bacherikov <slava@bacher09.org> wrote:
>
>
>
> 01.04.2020 17:20, Slava Bacherikov wrotes:
> > Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
> > enabled will produce invalid btf file, since gen_btf function in
> > link-vmlinux.sh script doesn't handle *.dwo files.
> >
> > Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
> > using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
> >
> > Signed-off-by: Slava Bacherikov <slava@bacher09.org>
> > Reported-by: Jann Horn <jannh@google.com>
> > Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
> > Acked-by: KP Singh <kpsingh@google.com>
> > Acked-by: Andrii Nakryiko <andriin@fb.com>
> > Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
> > ---
> >  lib/Kconfig.debug | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index f61d834e02fe..b94227be2d62 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -222,7 +222,9 @@ config DEBUG_INFO_DWARF4
> >
> >  config DEBUG_INFO_BTF
> >       bool "Generate BTF typeinfo"
> > -     depends on DEBUG_INFO
> > +     depends on DEBUG_INFO || COMPILE_TEST
> I had to add this, since DEBUG_INFO which depends on:
>
>         DEBUG_KERNEL && !COMPILE_TEST
>
> would block DEBUG_INFO_BTF when COMPILE_TEST is turned on.
>

Sorry if I'm being dense here. But what's the point in enabling
DEBUG_INFO_BTF if there is no *valid* DWARF info available for
DWARF-to-BTF conversion?


> In that case allyesconfig will emit both:
>
> CONFIG_DEBUG_INFO_BTF=y
> CONFIG_GCC_PLUGIN_RANDSTRUCT=y

Which I thought is exactly what we wanted to avoid. Not sure what's
the point of compiling kernel (even if it's the one that is not
supposed to ever run) that apriori has broken BTF? If it was
acceptable to not have DEBUG_INFO for COMPILE_TEST, why it's not
acceptable to not have DEBUG_INFO_BTF in that situation as well?

>
>
>
> > +     depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
> > +     depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
> >       help
> >         Generate deduplicated BTF type information from DWARF debug info.
> >         Turning this on expects presence of pahole tool, which will convert
> >

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-01 17:46       ` Andrii Nakryiko
@ 2020-04-01 18:24         ` Slava Bacherikov
  0 siblings, 0 replies; 17+ messages in thread
From: Slava Bacherikov @ 2020-04-01 18:24 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Kees Cook, Andrii Nakryiko, bpf, open list, Jann Horn,
	Alexei Starovoitov, Daniel Borkmann, Kernel Hardening,
	Liu Yiding, KP Singh



01.04.2020 20:46, Andrii Nakryiko пишет:
> On Wed, Apr 1, 2020 at 7:38 AM Slava Bacherikov <slava@bacher09.org> wrote:
>>
>>
>>
>> 01.04.2020 17:20, Slava Bacherikov wrotes:
>>> Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
>>> enabled will produce invalid btf file, since gen_btf function in
>>> link-vmlinux.sh script doesn't handle *.dwo files.
>>>
>>> Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
>>> using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
>>>
>>> Signed-off-by: Slava Bacherikov <slava@bacher09.org>
>>> Reported-by: Jann Horn <jannh@google.com>
>>> Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
>>> Acked-by: KP Singh <kpsingh@google.com>
>>> Acked-by: Andrii Nakryiko <andriin@fb.com>
>>> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
>>> ---
>>>  lib/Kconfig.debug | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>>> index f61d834e02fe..b94227be2d62 100644
>>> --- a/lib/Kconfig.debug
>>> +++ b/lib/Kconfig.debug
>>> @@ -222,7 +222,9 @@ config DEBUG_INFO_DWARF4
>>>
>>>  config DEBUG_INFO_BTF
>>>       bool "Generate BTF typeinfo"
>>> -     depends on DEBUG_INFO
>>> +     depends on DEBUG_INFO || COMPILE_TEST
>> I had to add this, since DEBUG_INFO which depends on:
>>
>>         DEBUG_KERNEL && !COMPILE_TEST
>>
>> would block DEBUG_INFO_BTF when COMPILE_TEST is turned on.
>>
> 
> Sorry if I'm being dense here. But what's the point in enabling
> DEBUG_INFO_BTF if there is no *valid* DWARF info available for
> DWARF-to-BTF conversion?

As I mention in [0] there is no point in having `!GCC_PLUGIN_RANDSTRUCT
|| COMPILE_TEST` without `DEBUG_INFO || COMPILE_TEST`, since without it
COMPILE_TEST would block DEBUG_INFO_BTF and because of that
GCC_PLUGIN_RANDSTRUCT would be never blocked by BTF.

As far as I understood from [1] main point for all these these things
with COMPILE_TEST is to be able to check if kernel could be compiled
with all these options (e.g. check syntax, build scripts, etc).

I can rollback `DEBUG_INFO || COMPILE_TEST`, but in that case there is
no point in `GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST` since COMPILE_TEST
in that case will not affect anything here, regardless from it's value.

[0]:
https://lore.kernel.org/bpf/202004010029.167BA4AA1F@keescook/T/#m2f493902d6aed09d30e5c4144a0164459386339d
[1]:
https://lore.kernel.org/bpf/202004010029.167BA4AA1F@keescook/T/#m8f25fab3476c9619249fee9ae692acb98c02cdc7
> 
> 
>> In that case allyesconfig will emit both:
>>
>> CONFIG_DEBUG_INFO_BTF=y
>> CONFIG_GCC_PLUGIN_RANDSTRUCT=y
> 
> Which I thought is exactly what we wanted to avoid. Not sure what's
> the point of compiling kernel (even if it's the one that is not
> supposed to ever run) that apriori has broken BTF? If it was
> acceptable to not have DEBUG_INFO for COMPILE_TEST, why it's not
> acceptable to not have DEBUG_INFO_BTF in that situation as well?
> 
>>
>>
>>
>>> +     depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
>>> +     depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
>>>       help
>>>         Generate deduplicated BTF type information from DWARF debug info.
>>>         Turning this on expects presence of pahole tool, which will convert
>>>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-01 15:49     ` Kees Cook
@ 2020-04-02 15:33       ` Slava Bacherikov
  2020-04-02 15:40         ` Slava Bacherikov
  0 siblings, 1 reply; 17+ messages in thread
From: Slava Bacherikov @ 2020-04-02 15:33 UTC (permalink / raw)
  To: keescook
  Cc: andriin, bpf, linux-kernel, jannh, alexei.starovoitov, daniel,
	kernel-hardening, liuyd.fnst, kpsingh, Slava Bacherikov

Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
enabled will produce invalid btf file, since gen_btf function in
link-vmlinux.sh script doesn't handle *.dwo files.

Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.

Signed-off-by: Slava Bacherikov <slava@bacher09.org>
Reported-by: Jann Horn <jannh@google.com>
Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
Acked-by: KP Singh <kpsingh@google.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
---
 lib/Kconfig.debug | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f61d834e02fe..b94227be2d62 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -222,7 +222,9 @@ config DEBUG_INFO_DWARF4
 
 config DEBUG_INFO_BTF
 	bool "Generate BTF typeinfo"
-	depends on DEBUG_INFO
+	depends on DEBUG_INFO || COMPILE_TEST
+	depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
+	depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
 	help
 	  Generate deduplicated BTF type information from DWARF debug info.
 	  Turning this on expects presence of pahole tool, which will convert

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-02 15:33       ` [PATCH v4 " Slava Bacherikov
@ 2020-04-02 15:40         ` Slava Bacherikov
  2020-04-02 19:31           ` Andrii Nakryiko
  0 siblings, 1 reply; 17+ messages in thread
From: Slava Bacherikov @ 2020-04-02 15:40 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Kees Cook, bpf, linux-kernel, jannh, alexei.starovoitov, daniel,
	kernel-hardening, liuyd.fnst, kpsingh



02.04.2020 18:33, Slava Bacherikov wrote:
> +	depends on DEBUG_INFO || COMPILE_TEST

Andrii are you fine by this ?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-02 15:40         ` Slava Bacherikov
@ 2020-04-02 19:31           ` Andrii Nakryiko
  2020-04-02 20:34             ` Kees Cook
  0 siblings, 1 reply; 17+ messages in thread
From: Andrii Nakryiko @ 2020-04-02 19:31 UTC (permalink / raw)
  To: Slava Bacherikov
  Cc: Andrii Nakryiko, Kees Cook, bpf, open list, Jann Horn,
	Alexei Starovoitov, Daniel Borkmann, Kernel Hardening,
	Liu Yiding, kpsingh

On Thu, Apr 2, 2020 at 8:40 AM Slava Bacherikov <slava@bacher09.org> wrote:
>
>
>
> 02.04.2020 18:33, Slava Bacherikov wrote:
> > +     depends on DEBUG_INFO || COMPILE_TEST
>
> Andrii are you fine by this ?

I think it needs a good comment explaining this weirdness, at least.
As I said, if there is no DEBUG_INFO, there is not point in doing
DWARF-to-BTF conversion, even more -- it actually might fail, I
haven't checked what pahole does in that case. So I'd rather drop
GCC_PLUGIN_RANDSTRUCT is that's the issue here. DEBUG_INFO_SPLIT and
DEBUG_INFO_REDUCED look good.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-02 19:31           ` Andrii Nakryiko
@ 2020-04-02 20:34             ` Kees Cook
  2020-04-02 20:38               ` Slava Bacherikov
  2020-04-02 20:41               ` [PATCH v5 " Slava Bacherikov
  0 siblings, 2 replies; 17+ messages in thread
From: Kees Cook @ 2020-04-02 20:34 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Slava Bacherikov, Andrii Nakryiko, bpf, open list, Jann Horn,
	Alexei Starovoitov, Daniel Borkmann, Kernel Hardening,
	Liu Yiding, kpsingh

On Thu, Apr 02, 2020 at 12:31:36PM -0700, Andrii Nakryiko wrote:
> On Thu, Apr 2, 2020 at 8:40 AM Slava Bacherikov <slava@bacher09.org> wrote:
> >
> >
> >
> > 02.04.2020 18:33, Slava Bacherikov wrote:
> > > +     depends on DEBUG_INFO || COMPILE_TEST
> >
> > Andrii are you fine by this ?
> 
> I think it needs a good comment explaining this weirdness, at least.
> As I said, if there is no DEBUG_INFO, there is not point in doing
> DWARF-to-BTF conversion, even more -- it actually might fail, I
> haven't checked what pahole does in that case. So I'd rather drop
> GCC_PLUGIN_RANDSTRUCT is that's the issue here. DEBUG_INFO_SPLIT and
> DEBUG_INFO_REDUCED look good.

The DEBUG_INFO is separate, AIUI -- it sounds like BTF may entirely
break on a compile with weird DWARF configs.

The GCC_PLUGIN_RANDSTRUCT issue is separate: it doesn't make sense to
run a kernel built with BTF and GCC_PLUGIN_RANDSTRUCT. But they should
have nothing to do with each other with regard to compilation. So, to
keep GCC_PLUGIN_RANDSTRUCT disable for "real" builds but leave it on for
all*config, randconfig, etc, I'd like to keep the || COMPILE_TEST,
otherwise GCC_PLUGIN_RANDSTRUCT won't be part of the many CIs doing
compilation testing.

And FWIW, I'm fine to let GCC_PLUGIN_RANDSTRUCT and BTF build together.
But if they want to be depends-conflicted, I wanted to keep the test
compile trap door.

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-02 20:34             ` Kees Cook
@ 2020-04-02 20:38               ` Slava Bacherikov
  2020-04-02 20:41               ` [PATCH v5 " Slava Bacherikov
  1 sibling, 0 replies; 17+ messages in thread
From: Slava Bacherikov @ 2020-04-02 20:38 UTC (permalink / raw)
  To: Kees Cook, Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, open list, Jann Horn, Alexei Starovoitov,
	Daniel Borkmann, Kernel Hardening, Liu Yiding, kpsingh



02.04.2020 23:34, Kees Cook wrote:
> On Thu, Apr 02, 2020 at 12:31:36PM -0700, Andrii Nakryiko wrote:
>> On Thu, Apr 2, 2020 at 8:40 AM Slava Bacherikov <slava@bacher09.org> wrote:
>>>
>>>
>>>
>>> 02.04.2020 18:33, Slava Bacherikov wrote:
>>>> +     depends on DEBUG_INFO || COMPILE_TEST
>>>
>>> Andrii are you fine by this ?
>>
>> I think it needs a good comment explaining this weirdness, at least.
>> As I said, if there is no DEBUG_INFO, there is not point in doing
>> DWARF-to-BTF conversion, even more -- it actually might fail, I
>> haven't checked what pahole does in that case. So I'd rather drop
>> GCC_PLUGIN_RANDSTRUCT is that's the issue here. DEBUG_INFO_SPLIT and
>> DEBUG_INFO_REDUCED look good.

Yesterday before sending it I tested it against latest bpf git with
allyesconfig and it compiled fine, even worked in vm ;)
> 
> The DEBUG_INFO is separate, AIUI -- it sounds like BTF may entirely
> break on a compile with weird DWARF configs.
> 
> The GCC_PLUGIN_RANDSTRUCT issue is separate: it doesn't make sense to
> run a kernel built with BTF and GCC_PLUGIN_RANDSTRUCT. But they should
> have nothing to do with each other with regard to compilation. So, to
> keep GCC_PLUGIN_RANDSTRUCT disable for "real" builds but leave it on for
> all*config, randconfig, etc, I'd like to keep the || COMPILE_TEST,
> otherwise GCC_PLUGIN_RANDSTRUCT won't be part of the many CIs doing
> compilation testing.
> 
> And FWIW, I'm fine to let GCC_PLUGIN_RANDSTRUCT and BTF build together.
> But if they want to be depends-conflicted, I wanted to keep the test
> compile trap door.
> 

Oh, seems I misunderstood you, if everyone agree I'll drop it.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v5 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-02 20:34             ` Kees Cook
  2020-04-02 20:38               ` Slava Bacherikov
@ 2020-04-02 20:41               ` Slava Bacherikov
  2020-04-02 21:02                 ` Andrii Nakryiko
  2020-04-02 22:49                 ` Daniel Borkmann
  1 sibling, 2 replies; 17+ messages in thread
From: Slava Bacherikov @ 2020-04-02 20:41 UTC (permalink / raw)
  To: andriin
  Cc: keescook, bpf, linux-kernel, jannh, alexei.starovoitov, daniel,
	kernel-hardening, liuyd.fnst, kpsingh, Slava Bacherikov

Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
enabled will produce invalid btf file, since gen_btf function in
link-vmlinux.sh script doesn't handle *.dwo files.

Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.

Signed-off-by: Slava Bacherikov <slava@bacher09.org>
Reported-by: Jann Horn <jannh@google.com>
Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
Acked-by: KP Singh <kpsingh@google.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
---
 lib/Kconfig.debug | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f61d834e02fe..6118d99117da 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -223,6 +223,8 @@ config DEBUG_INFO_DWARF4
 config DEBUG_INFO_BTF
 	bool "Generate BTF typeinfo"
 	depends on DEBUG_INFO
+	depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
+	depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
 	help
 	  Generate deduplicated BTF type information from DWARF debug info.
 	  Turning this on expects presence of pahole tool, which will convert

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-02 20:41               ` [PATCH v5 " Slava Bacherikov
@ 2020-04-02 21:02                 ` Andrii Nakryiko
  2020-04-02 22:49                 ` Daniel Borkmann
  1 sibling, 0 replies; 17+ messages in thread
From: Andrii Nakryiko @ 2020-04-02 21:02 UTC (permalink / raw)
  To: Slava Bacherikov
  Cc: Andrii Nakryiko, Kees Cook, bpf, open list, Jann Horn,
	Alexei Starovoitov, Daniel Borkmann, Kernel Hardening,
	Liu Yiding, KP Singh

On Thu, Apr 2, 2020 at 1:44 PM Slava Bacherikov <slava@bacher09.org> wrote:
>
> Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
> enabled will produce invalid btf file, since gen_btf function in
> link-vmlinux.sh script doesn't handle *.dwo files.
>
> Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
> using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
>
> Signed-off-by: Slava Bacherikov <slava@bacher09.org>
> Reported-by: Jann Horn <jannh@google.com>
> Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
> Acked-by: KP Singh <kpsingh@google.com>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
> ---
>  lib/Kconfig.debug | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f61d834e02fe..6118d99117da 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -223,6 +223,8 @@ config DEBUG_INFO_DWARF4
>  config DEBUG_INFO_BTF
>         bool "Generate BTF typeinfo"
>         depends on DEBUG_INFO
> +       depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
> +       depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST

Given what Kees explained, I think this looks good. Thanks!

>         help
>           Generate deduplicated BTF type information from DWARF debug info.
>           Turning this on expects presence of pahole tool, which will convert

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF
  2020-04-02 20:41               ` [PATCH v5 " Slava Bacherikov
  2020-04-02 21:02                 ` Andrii Nakryiko
@ 2020-04-02 22:49                 ` Daniel Borkmann
  1 sibling, 0 replies; 17+ messages in thread
From: Daniel Borkmann @ 2020-04-02 22:49 UTC (permalink / raw)
  To: Slava Bacherikov, andriin
  Cc: keescook, bpf, linux-kernel, jannh, alexei.starovoitov,
	kernel-hardening, liuyd.fnst, kpsingh

On 4/2/20 10:41 PM, Slava Bacherikov wrote:
> Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
> enabled will produce invalid btf file, since gen_btf function in
> link-vmlinux.sh script doesn't handle *.dwo files.
> 
> Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
> using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
> 
> Signed-off-by: Slava Bacherikov <slava@bacher09.org>
> Reported-by: Jann Horn <jannh@google.com>
> Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
> Acked-by: KP Singh <kpsingh@google.com>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")

Applied, thanks!

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-04-02 22:50 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-31 21:55 [PATCH v2 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF Slava Bacherikov
2020-03-31 22:40 ` KP Singh
2020-04-01  0:03 ` Andrii Nakryiko
2020-04-01  7:34 ` Kees Cook
2020-04-01 14:20   ` [PATCH v3 " Slava Bacherikov
2020-04-01 14:37     ` Slava Bacherikov
2020-04-01 17:46       ` Andrii Nakryiko
2020-04-01 18:24         ` Slava Bacherikov
2020-04-01 15:49     ` Kees Cook
2020-04-02 15:33       ` [PATCH v4 " Slava Bacherikov
2020-04-02 15:40         ` Slava Bacherikov
2020-04-02 19:31           ` Andrii Nakryiko
2020-04-02 20:34             ` Kees Cook
2020-04-02 20:38               ` Slava Bacherikov
2020-04-02 20:41               ` [PATCH v5 " Slava Bacherikov
2020-04-02 21:02                 ` Andrii Nakryiko
2020-04-02 22:49                 ` Daniel Borkmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).