BPF Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH bpf] bpf: force .BTF section start to zero when dumping from vmlinux
@ 2019-11-27 22:57 Stanislav Fomichev
  2019-11-28  4:37 ` John Fastabend
  0 siblings, 1 reply; 3+ messages in thread
From: Stanislav Fomichev @ 2019-11-27 22:57 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko

While trying to figure out why fentry_fexit selftest doesn't pass for me
(old pahole, broken BTF), I found out that my latest patch can break vmlinux
.BTF generation. objcopy preserves section start when doing --only-section,
so there is a chance (depending on where pahole inserts .BTF section) to
have leading empty zeroes. Let's explicitly force section offset to zero.

Before:
$ objcopy --set-section-flags .BTF=alloc -O binary \
	--only-section=.BTF vmlinux .btf.vmlinux.bin
$ xxd .btf.vmlinux.bin | head -n1
00000000: 0000 0000 0000 0000 0000 0000 0000 0000  ................

After:
$ objcopy --change-section-address .BTF=0 \
	--set-section-flags .BTF=alloc -O binary \
	--only-section=.BTF vmlinux .btf.vmlinux.bin
$ xxd .btf.vmlinux.bin | head -n1
00000000: 9feb 0100 1800 0000 0000 0000 80e1 1c00  ................
          ^BTF magic

As part of this change, I'm also dropping '2>/dev/null' from objcopy
invocation to be able to catch possible other issues (objcopy doesn't
produce any warnings for me anymore, it did before with --dump-section).

Cc: Andrii Nakryiko <andriin@fb.com>
Fixes: da5fb18225b4 ("bpf: Support pre-2.25-binutils objcopy for vmlinux BTF")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 scripts/link-vmlinux.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 2998ddb323e3..436379940356 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -127,8 +127,9 @@ gen_btf()
 		cut -d, -f1 | cut -d' ' -f2)
 	bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \
 		awk '{print $4}')
-	${OBJCOPY} --set-section-flags .BTF=alloc -O binary \
-		--only-section=.BTF ${1} .btf.vmlinux.bin 2>/dev/null
+	${OBJCOPY} --change-section-address .BTF=0 \
+		--set-section-flags .BTF=alloc -O binary \
+		--only-section=.BTF ${1} .btf.vmlinux.bin
 	${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \
 		--rename-section .data=.BTF .btf.vmlinux.bin ${2}
 }
-- 
2.24.0.432.g9d3f5f5b63-goog


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

* RE: [PATCH bpf] bpf: force .BTF section start to zero when dumping from vmlinux
  2019-11-27 22:57 [PATCH bpf] bpf: force .BTF section start to zero when dumping from vmlinux Stanislav Fomichev
@ 2019-11-28  4:37 ` John Fastabend
  2019-11-29 15:16   ` Daniel Borkmann
  0 siblings, 1 reply; 3+ messages in thread
From: John Fastabend @ 2019-11-28  4:37 UTC (permalink / raw)
  To: Stanislav Fomichev, netdev, bpf
  Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko

Stanislav Fomichev wrote:
> While trying to figure out why fentry_fexit selftest doesn't pass for me
> (old pahole, broken BTF), I found out that my latest patch can break vmlinux
> .BTF generation. objcopy preserves section start when doing --only-section,
> so there is a chance (depending on where pahole inserts .BTF section) to
> have leading empty zeroes. Let's explicitly force section offset to zero.
> 
> Before:
> $ objcopy --set-section-flags .BTF=alloc -O binary \
> 	--only-section=.BTF vmlinux .btf.vmlinux.bin
> $ xxd .btf.vmlinux.bin | head -n1
> 00000000: 0000 0000 0000 0000 0000 0000 0000 0000  ................
> 
> After:
> $ objcopy --change-section-address .BTF=0 \
> 	--set-section-flags .BTF=alloc -O binary \
> 	--only-section=.BTF vmlinux .btf.vmlinux.bin
> $ xxd .btf.vmlinux.bin | head -n1
> 00000000: 9feb 0100 1800 0000 0000 0000 80e1 1c00  ................
>           ^BTF magic
> 
> As part of this change, I'm also dropping '2>/dev/null' from objcopy
> invocation to be able to catch possible other issues (objcopy doesn't
> produce any warnings for me anymore, it did before with --dump-section).

Agree dropping /dev/null seems like a good choice. Otherwise seems reasonable
to me.

Acked-by: John Fastabend <john.fastabend@gmail.com>

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

* Re: [PATCH bpf] bpf: force .BTF section start to zero when dumping from vmlinux
  2019-11-28  4:37 ` John Fastabend
@ 2019-11-29 15:16   ` Daniel Borkmann
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Borkmann @ 2019-11-29 15:16 UTC (permalink / raw)
  To: John Fastabend, Stanislav Fomichev, netdev, bpf
  Cc: davem, ast, Andrii Nakryiko

On 11/28/19 5:37 AM, John Fastabend wrote:
> Stanislav Fomichev wrote:
>> While trying to figure out why fentry_fexit selftest doesn't pass for me
>> (old pahole, broken BTF), I found out that my latest patch can break vmlinux
>> .BTF generation. objcopy preserves section start when doing --only-section,
>> so there is a chance (depending on where pahole inserts .BTF section) to
>> have leading empty zeroes. Let's explicitly force section offset to zero.
>>
>> Before:
>> $ objcopy --set-section-flags .BTF=alloc -O binary \
>> 	--only-section=.BTF vmlinux .btf.vmlinux.bin
>> $ xxd .btf.vmlinux.bin | head -n1
>> 00000000: 0000 0000 0000 0000 0000 0000 0000 0000  ................
>>
>> After:
>> $ objcopy --change-section-address .BTF=0 \
>> 	--set-section-flags .BTF=alloc -O binary \
>> 	--only-section=.BTF vmlinux .btf.vmlinux.bin
>> $ xxd .btf.vmlinux.bin | head -n1
>> 00000000: 9feb 0100 1800 0000 0000 0000 80e1 1c00  ................
>>            ^BTF magic
>>
>> As part of this change, I'm also dropping '2>/dev/null' from objcopy
>> invocation to be able to catch possible other issues (objcopy doesn't
>> produce any warnings for me anymore, it did before with --dump-section).
> 
> Agree dropping /dev/null seems like a good choice. Otherwise seems reasonable
> to me.
> 
> Acked-by: John Fastabend <john.fastabend@gmail.com>

Could reproduce as well:

root@apoc:~/bpf# xxd .btf.vmlinux.bin.old | head -n1                   (original)
00000000: 9feb 0100 1800 0000 0000 0000 5088 2000  ............P. .

root@apoc:~/bpf# ls -l .btf.vmlinux.bin.old
-rw-r--r-- 1 root root 3439882 Nov 29 15:59 .btf.vmlinux.bin.old


root@apoc:~/bpf# xxd .btf.vmlinux.bin.new-buggy | head -n1             ('bpf: Support pre-2.25-binutils objcopy for vmlinux BTF')
00000000: 0000 0000 0000 0000 0000 0000 0000 0000  ................

root@apoc:~/bpf# ls -l .btf.vmlinux.bin.new-buggy
-rwxr-xr-x 1 root root 45705482 Nov 29 16:01 .btf.vmlinux.bin.new-buggy


root@apoc:~/bpf# xxd .btf.vmlinux.bin.new-fixed | head -n1             ('bpf: Force .BTF section start to zero when dumping from vmlinux')
00000000: 9feb 0100 1800 0000 0000 0000 5088 2000  ............P. .

root@apoc:~/bpf# ls -l .btf.vmlinux.bin.new-fixed
-rwxr-xr-x 1 root root 3439882 Nov 29 16:02 .btf.vmlinux.bin.new-fixed


root@apoc:~/bpf# diff .btf.vmlinux.bin.old .btf.vmlinux.bin.new-buggy
Binary files .btf.vmlinux.bin.old and .btf.vmlinux.bin.new-buggy differ

root@apoc:~/bpf# diff .btf.vmlinux.bin.old .btf.vmlinux.bin.new-fixed
root@apoc:~/bpf#


Applied, thanks!

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27 22:57 [PATCH bpf] bpf: force .BTF section start to zero when dumping from vmlinux Stanislav Fomichev
2019-11-28  4:37 ` John Fastabend
2019-11-29 15:16   ` Daniel Borkmann

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git