[PATCH dwarves] btf_encoder: verify 0 address DWARF variables are really in ELF section

[PATCH dwarves] btf_encoder: verify 0 address DWARF variables are really in ELF section

[Alan Maguire]

We use the DWARF location information to match a variable with its
associated ELF section.  In the case of per-CPU variables their
ELF section address range starts at 0, so any 0 address variables will
appear to belong in that ELF section.  However, for "discard" sections
DWARF encodes the associated variables with address location 0 so
we need to double-check that address 0 variables really are in the
associated section by checking the ELF symbol table.

This resolves an issue exposed by CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y
kernel builds where __pcpu_* dummary variables in a .discard section
get misclassified as belonging in the per-CPU variable section since
they specify location address 0.

Reported-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 btf_encoder.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/btf_encoder.c b/btf_encoder.c
index 3754884..04f547c 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -2189,6 +2189,26 @@ static bool filter_variable_name(const char *name)
 	return false;
 }
 
+bool variable_in_sec(struct btf_encoder *encoder, const char *name, size_t shndx)
+{
+	uint32_t sym_sec_idx;
+	uint32_t core_id;
+	GElf_Sym sym;
+
+	elf_symtab__for_each_symbol_index(encoder->symtab, core_id, sym, sym_sec_idx) {
+		const char *sym_name;
+
+		if (sym_sec_idx != shndx || elf_sym__type(&sym) != STT_OBJECT)
+			continue;
+		sym_name = elf_sym__name(&sym, encoder->symtab);
+		if (!sym_name)
+			continue;
+		if (strcmp(name, sym_name) == 0)
+			return true;
+	}
+	return false;
+}
+
 static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 {
 	struct cu *cu = encoder->cu;
@@ -2258,6 +2278,13 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 		if (filter_variable_name(name))
 			continue;
 
+		/* A 0 address may be in a "discard" section; DWARF provides
+		 * location information with address 0 for such variables.
+		 * Ensure the variable really is in this section by checking
+		 * the ELF symtab.
+		 */
+		if (addr == 0 && !variable_in_sec(encoder, name, shndx))
+			continue;
 		/* Check for invalid BTF names */
 		if (!btf_name_valid(name)) {
 			dump_invalid_symbol("Found invalid variable name when encoding btf",
-- 
2.31.1

Re: [PATCH dwarves] btf_encoder: verify 0 address DWARF variables are really in ELF section

[Jiri Olsa]

On Tue, Dec 17, 2024 at 10:36:29AM +0000, Alan Maguire wrote:
> We use the DWARF location information to match a variable with its
> associated ELF section.  In the case of per-CPU variables their
> ELF section address range starts at 0, so any 0 address variables will
> appear to belong in that ELF section.  However, for "discard" sections
> DWARF encodes the associated variables with address location 0 so
> we need to double-check that address 0 variables really are in the
> associated section by checking the ELF symbol table.
> 
> This resolves an issue exposed by CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y
> kernel builds where __pcpu_* dummary variables in a .discard section
> get misclassified as belonging in the per-CPU variable section since
> they specify location address 0.
> 
> Reported-by: Cong Wang <xiyou.wangcong@gmail.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>

Tested/Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka


> ---
>  btf_encoder.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 3754884..04f547c 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -2189,6 +2189,26 @@ static bool filter_variable_name(const char *name)
>  	return false;
>  }
>  
> +bool variable_in_sec(struct btf_encoder *encoder, const char *name, size_t shndx)
> +{
> +	uint32_t sym_sec_idx;
> +	uint32_t core_id;
> +	GElf_Sym sym;
> +
> +	elf_symtab__for_each_symbol_index(encoder->symtab, core_id, sym, sym_sec_idx) {
> +		const char *sym_name;
> +
> +		if (sym_sec_idx != shndx || elf_sym__type(&sym) != STT_OBJECT)
> +			continue;
> +		sym_name = elf_sym__name(&sym, encoder->symtab);
> +		if (!sym_name)
> +			continue;
> +		if (strcmp(name, sym_name) == 0)
> +			return true;
> +	}
> +	return false;
> +}
> +
>  static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
>  {
>  	struct cu *cu = encoder->cu;
> @@ -2258,6 +2278,13 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
>  		if (filter_variable_name(name))
>  			continue;
>  
> +		/* A 0 address may be in a "discard" section; DWARF provides
> +		 * location information with address 0 for such variables.
> +		 * Ensure the variable really is in this section by checking
> +		 * the ELF symtab.
> +		 */
> +		if (addr == 0 && !variable_in_sec(encoder, name, shndx))
> +			continue;
>  		/* Check for invalid BTF names */
>  		if (!btf_name_valid(name)) {
>  			dump_invalid_symbol("Found invalid variable name when encoding btf",
> -- 
> 2.31.1
> 

Re: [PATCH dwarves] btf_encoder: verify 0 address DWARF variables are really in ELF section

[Stephen Brennan]

Alan Maguire <alan.maguire@oracle.com> writes:

> We use the DWARF location information to match a variable with its
> associated ELF section.  In the case of per-CPU variables their
> ELF section address range starts at 0, so any 0 address variables will
> appear to belong in that ELF section.  However, for "discard" sections
> DWARF encodes the associated variables with address location 0 so
> we need to double-check that address 0 variables really are in the
> associated section by checking the ELF symbol table.
>
> This resolves an issue exposed by CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y
> kernel builds where __pcpu_* dummary variables in a .discard section
> get misclassified as belonging in the per-CPU variable section since
> they specify location address 0.
>
> Reported-by: Cong Wang <xiyou.wangcong@gmail.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>

Reviewed-by: Stephen Brennan <stephen.s.brennan@oracle.com>

> ---
>  btf_encoder.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 3754884..04f547c 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -2189,6 +2189,26 @@ static bool filter_variable_name(const char *name)
>  	return false;
>  }
>  
> +bool variable_in_sec(struct btf_encoder *encoder, const char *name, size_t shndx)
> +{
> +	uint32_t sym_sec_idx;
> +	uint32_t core_id;
> +	GElf_Sym sym;
> +
> +	elf_symtab__for_each_symbol_index(encoder->symtab, core_id, sym, sym_sec_idx) {
> +		const char *sym_name;
> +
> +		if (sym_sec_idx != shndx || elf_sym__type(&sym) != STT_OBJECT)
> +			continue;
> +		sym_name = elf_sym__name(&sym, encoder->symtab);
> +		if (!sym_name)
> +			continue;
> +		if (strcmp(name, sym_name) == 0)
> +			return true;
> +	}
> +	return false;
> +}
> +
>  static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
>  {
>  	struct cu *cu = encoder->cu;
> @@ -2258,6 +2278,13 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
>  		if (filter_variable_name(name))
>  			continue;
>  
> +		/* A 0 address may be in a "discard" section; DWARF provides
> +		 * location information with address 0 for such variables.
> +		 * Ensure the variable really is in this section by checking
> +		 * the ELF symtab.
> +		 */
> +		if (addr == 0 && !variable_in_sec(encoder, name, shndx))
> +			continue;
>  		/* Check for invalid BTF names */
>  		if (!btf_name_valid(name)) {
>  			dump_invalid_symbol("Found invalid variable name when encoding btf",
> -- 
> 2.31.1

Re: [PATCH dwarves] btf_encoder: verify 0 address DWARF variables are really in ELF section

[Cong Wang]

Hi Alan,

On Tue, Dec 17, 2024 at 2:36 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> We use the DWARF location information to match a variable with its
> associated ELF section.  In the case of per-CPU variables their
> ELF section address range starts at 0, so any 0 address variables will
> appear to belong in that ELF section.  However, for "discard" sections
> DWARF encodes the associated variables with address location 0 so
> we need to double-check that address 0 variables really are in the
> associated section by checking the ELF symbol table.
>
> This resolves an issue exposed by CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y
> kernel builds where __pcpu_* dummary variables in a .discard section
> get misclassified as belonging in the per-CPU variable section since
> they specify location address 0.

It is _not_ your patch's fault, but I got this segfault which prevents me from
testing this patch. (It also happens after reverting your patch.)

  INSTALL libsubcmd_headers
  INSTALL libsubcmd_headers
  CALL    scripts/checksyscalls.sh
  UPD     include/generated/utsversion.h
  CC      init/version-timestamp.o
  KSYMS   .tmp_vmlinux0.kallsyms.S
  AS      .tmp_vmlinux0.kallsyms.o
  LD      .tmp_vmlinux1
  BTF     .tmp_vmlinux1.btf.o
Segmentation fault (core dumped)
  NM      .tmp_vmlinux1.syms
  KSYMS   .tmp_vmlinux1.kallsyms.S
  AS      .tmp_vmlinux1.kallsyms.o
  LD      .tmp_vmlinux2
  NM      .tmp_vmlinux2.syms
  KSYMS   .tmp_vmlinux2.kallsyms.S
  AS      .tmp_vmlinux2.kallsyms.o
  LD      vmlinux
  BTFIDS  vmlinux
libbpf: failed to find '.BTF' ELF section in vmlinux
FAILED: load BTF from vmlinux: No data available
make[2]: *** [scripts/Makefile.vmlinux:77: vmlinux] Error 255
make[2]: *** Deleting file 'vmlinux'

Thanks!

Re: [PATCH dwarves] btf_encoder: verify 0 address DWARF variables are really in ELF section

[Cong Wang]

On Sat, Jan 25, 2025 at 8:55 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> Hi Alan,
>
> On Tue, Dec 17, 2024 at 2:36 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > We use the DWARF location information to match a variable with its
> > associated ELF section.  In the case of per-CPU variables their
> > ELF section address range starts at 0, so any 0 address variables will
> > appear to belong in that ELF section.  However, for "discard" sections
> > DWARF encodes the associated variables with address location 0 so
> > we need to double-check that address 0 variables really are in the
> > associated section by checking the ELF symbol table.
> >
> > This resolves an issue exposed by CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y
> > kernel builds where __pcpu_* dummary variables in a .discard section
> > get misclassified as belonging in the per-CPU variable section since
> > they specify location address 0.
>
> It is _not_ your patch's fault, but I got this segfault which prevents me from
> testing this patch. (It also happens after reverting your patch.)

Never mind, I managed to workaround this issue by a clean build.

And I tested your patch, it works for me with CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y.

Tested-by: Cong Wang <cong.wang@bytedance.com>

Thanks a lot!

Re: [PATCH dwarves] btf_encoder: verify 0 address DWARF variables are really in ELF section

[Alan Maguire]

On 26/01/2025 20:04, Cong Wang wrote:
> On Sat, Jan 25, 2025 at 8:55 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>
>> Hi Alan,
>>
>> On Tue, Dec 17, 2024 at 2:36 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>>
>>> We use the DWARF location information to match a variable with its
>>> associated ELF section.  In the case of per-CPU variables their
>>> ELF section address range starts at 0, so any 0 address variables will
>>> appear to belong in that ELF section.  However, for "discard" sections
>>> DWARF encodes the associated variables with address location 0 so
>>> we need to double-check that address 0 variables really are in the
>>> associated section by checking the ELF symbol table.
>>>
>>> This resolves an issue exposed by CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y
>>> kernel builds where __pcpu_* dummary variables in a .discard section
>>> get misclassified as belonging in the per-CPU variable section since
>>> they specify location address 0.
>>
>> It is _not_ your patch's fault, but I got this segfault which prevents me from
>> testing this patch. (It also happens after reverting your patch.)
> 
> Never mind, I managed to workaround this issue by a clean build.
> 
> And I tested your patch, it works for me with CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y.
> 
> Tested-by: Cong Wang <cong.wang@bytedance.com>
> 
> Thanks a lot!

Thanks for verifying the fix! You didn't happen to get a coredump or
backtrace for the earlier segmentation fault by any chance? Just want to
make sure there aren't other issues lurking here. Thanks again!

Alan

Re: [PATCH dwarves] btf_encoder: verify 0 address DWARF variables are really in ELF section

[Cong Wang]

On Mon, Jan 27, 2025 at 3:17 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 26/01/2025 20:04, Cong Wang wrote:
> > On Sat, Jan 25, 2025 at 8:55 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >>
> >> Hi Alan,
> >>
> >> On Tue, Dec 17, 2024 at 2:36 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >>>
> >>> We use the DWARF location information to match a variable with its
> >>> associated ELF section.  In the case of per-CPU variables their
> >>> ELF section address range starts at 0, so any 0 address variables will
> >>> appear to belong in that ELF section.  However, for "discard" sections
> >>> DWARF encodes the associated variables with address location 0 so
> >>> we need to double-check that address 0 variables really are in the
> >>> associated section by checking the ELF symbol table.
> >>>
> >>> This resolves an issue exposed by CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y
> >>> kernel builds where __pcpu_* dummary variables in a .discard section
> >>> get misclassified as belonging in the per-CPU variable section since
> >>> they specify location address 0.
> >>
> >> It is _not_ your patch's fault, but I got this segfault which prevents me from
> >> testing this patch. (It also happens after reverting your patch.)
> >
> > Never mind, I managed to workaround this issue by a clean build.
> >
> > And I tested your patch, it works for me with CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y.
> >
> > Tested-by: Cong Wang <cong.wang@bytedance.com>
> >
> > Thanks a lot!
>
> Thanks for verifying the fix! You didn't happen to get a coredump or
> backtrace for the earlier segmentation fault by any chance? Just want to
> make sure there aren't other issues lurking here. Thanks again!

I didn't. I guess there was some mess in my build env, since a clean
build just worked.

Thanks!

Re: [PATCH dwarves] btf_encoder: verify 0 address DWARF variables are really in ELF section

[Alan Maguire]

On 17/12/2024 10:36, Alan Maguire wrote:
> We use the DWARF location information to match a variable with its
> associated ELF section.  In the case of per-CPU variables their
> ELF section address range starts at 0, so any 0 address variables will
> appear to belong in that ELF section.  However, for "discard" sections
> DWARF encodes the associated variables with address location 0 so
> we need to double-check that address 0 variables really are in the
> associated section by checking the ELF symbol table.
> 
> This resolves an issue exposed by CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y
> kernel builds where __pcpu_* dummary variables in a .discard section
> get misclassified as belonging in the per-CPU variable section since
> they specify location address 0.
> 
> Reported-by: Cong Wang <xiyou.wangcong@gmail.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>

applied to the next branch of

https://git.kernel.org/pub/scm/devel/pahole/pahole.git/

> ---
>  btf_encoder.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 3754884..04f547c 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -2189,6 +2189,26 @@ static bool filter_variable_name(const char *name)
>  	return false;
>  }
>  
> +bool variable_in_sec(struct btf_encoder *encoder, const char *name, size_t shndx)
> +{
> +	uint32_t sym_sec_idx;
> +	uint32_t core_id;
> +	GElf_Sym sym;
> +
> +	elf_symtab__for_each_symbol_index(encoder->symtab, core_id, sym, sym_sec_idx) {
> +		const char *sym_name;
> +
> +		if (sym_sec_idx != shndx || elf_sym__type(&sym) != STT_OBJECT)
> +			continue;
> +		sym_name = elf_sym__name(&sym, encoder->symtab);
> +		if (!sym_name)
> +			continue;
> +		if (strcmp(name, sym_name) == 0)
> +			return true;
> +	}
> +	return false;
> +}
> +
>  static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
>  {
>  	struct cu *cu = encoder->cu;
> @@ -2258,6 +2278,13 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
>  		if (filter_variable_name(name))
>  			continue;
>  
> +		/* A 0 address may be in a "discard" section; DWARF provides
> +		 * location information with address 0 for such variables.
> +		 * Ensure the variable really is in this section by checking
> +		 * the ELF symtab.
> +		 */
> +		if (addr == 0 && !variable_in_sec(encoder, name, shndx))
> +			continue;
>  		/* Check for invalid BTF names */
>  		if (!btf_name_valid(name)) {
>  			dump_invalid_symbol("Found invalid variable name when encoding btf",