bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tools: bpftool: fix struct_ops command invalid pointer free
@ 2020-04-10  2:06 Daniel T. Lee
  2020-04-10  5:03 ` Martin KaFai Lau
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel T. Lee @ 2020-04-10  2:06 UTC (permalink / raw)
  To: Daniel Borkmann, Alexei Starovoitov; +Cc: netdev, bpf, Martin KaFai Lau

From commit 65c93628599d ("bpftool: Add struct_ops support"),
a new type of command struct_ops has been added.

This command requires kernel CONFIG_DEBUG_INFO_BTF=y, and for retrieving
btf info, get_btf_vmlinux() is used.

When running this command on kernel without BTF debug info, this will
lead to 'btf_vmlinux' variable contains invalid(error) pointer. And by
this, btf_free() causes a segfault when executing 'bpftool struct_ops'.

This commit adds pointer validation with IS_ERR not to free invalid
pointer, and this will fix the segfault issue.

Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
---
 tools/bpf/bpftool/struct_ops.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/struct_ops.c b/tools/bpf/bpftool/struct_ops.c
index 2a7befbd11ad..0fe0d584c57e 100644
--- a/tools/bpf/bpftool/struct_ops.c
+++ b/tools/bpf/bpftool/struct_ops.c
@@ -591,6 +591,8 @@ int do_struct_ops(int argc, char **argv)
 
 	err = cmd_select(cmds, argc, argv, do_help);
 
-	btf__free(btf_vmlinux);
+	if (!IS_ERR(btf_vmlinux))
+		btf__free(btf_vmlinux);
+
 	return err;
 }
-- 
2.26.0


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

* Re: [PATCH] tools: bpftool: fix struct_ops command invalid pointer free
  2020-04-10  2:06 [PATCH] tools: bpftool: fix struct_ops command invalid pointer free Daniel T. Lee
@ 2020-04-10  5:03 ` Martin KaFai Lau
  2020-04-10  5:34   ` Daniel T. Lee
  2020-04-14 19:44   ` Daniel Borkmann
  0 siblings, 2 replies; 4+ messages in thread
From: Martin KaFai Lau @ 2020-04-10  5:03 UTC (permalink / raw)
  To: Daniel T. Lee; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev, bpf

On Fri, Apr 10, 2020 at 11:06:12AM +0900, Daniel T. Lee wrote:
> From commit 65c93628599d ("bpftool: Add struct_ops support"),
> a new type of command struct_ops has been added.
> 
> This command requires kernel CONFIG_DEBUG_INFO_BTF=y, and for retrieving
> btf info, get_btf_vmlinux() is used.
> 
> When running this command on kernel without BTF debug info, this will
> lead to 'btf_vmlinux' variable contains invalid(error) pointer. And by
> this, btf_free() causes a segfault when executing 'bpftool struct_ops'.
> 
> This commit adds pointer validation with IS_ERR not to free invalid
> pointer, and this will fix the segfault issue.
> 
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
Fixes: 65c93628599d ("bpftool: Add struct_ops support")
Acked-by: Martin KaFai Lau

Thanks for the fix!  Please add the Fixes tag in the future.

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

* Re: [PATCH] tools: bpftool: fix struct_ops command invalid pointer free
  2020-04-10  5:03 ` Martin KaFai Lau
@ 2020-04-10  5:34   ` Daniel T. Lee
  2020-04-14 19:44   ` Daniel Borkmann
  1 sibling, 0 replies; 4+ messages in thread
From: Daniel T. Lee @ 2020-04-10  5:34 UTC (permalink / raw)
  To: Martin KaFai Lau; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev, bpf

Will do, thanks for letting me know.
Thank you for your time and effort for the review.

Best,
Daniel

On Fri, Apr 10, 2020 at 2:03 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Fri, Apr 10, 2020 at 11:06:12AM +0900, Daniel T. Lee wrote:
> > From commit 65c93628599d ("bpftool: Add struct_ops support"),
> > a new type of command struct_ops has been added.
> >
> > This command requires kernel CONFIG_DEBUG_INFO_BTF=y, and for retrieving
> > btf info, get_btf_vmlinux() is used.
> >
> > When running this command on kernel without BTF debug info, this will
> > lead to 'btf_vmlinux' variable contains invalid(error) pointer. And by
> > this, btf_free() causes a segfault when executing 'bpftool struct_ops'.
> >
> > This commit adds pointer validation with IS_ERR not to free invalid
> > pointer, and this will fix the segfault issue.
> >
> > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> Fixes: 65c93628599d ("bpftool: Add struct_ops support")
> Acked-by: Martin KaFai Lau
>
> Thanks for the fix!  Please add the Fixes tag in the future.

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

* Re: [PATCH] tools: bpftool: fix struct_ops command invalid pointer free
  2020-04-10  5:03 ` Martin KaFai Lau
  2020-04-10  5:34   ` Daniel T. Lee
@ 2020-04-14 19:44   ` Daniel Borkmann
  1 sibling, 0 replies; 4+ messages in thread
From: Daniel Borkmann @ 2020-04-14 19:44 UTC (permalink / raw)
  To: Martin KaFai Lau, Daniel T. Lee; +Cc: Alexei Starovoitov, netdev, bpf

On 4/10/20 7:03 AM, Martin KaFai Lau wrote:
> On Fri, Apr 10, 2020 at 11:06:12AM +0900, Daniel T. Lee wrote:
>>  From commit 65c93628599d ("bpftool: Add struct_ops support"),
>> a new type of command struct_ops has been added.
>>
>> This command requires kernel CONFIG_DEBUG_INFO_BTF=y, and for retrieving
>> btf info, get_btf_vmlinux() is used.
>>
>> When running this command on kernel without BTF debug info, this will
>> lead to 'btf_vmlinux' variable contains invalid(error) pointer. And by
>> this, btf_free() causes a segfault when executing 'bpftool struct_ops'.
>>
>> This commit adds pointer validation with IS_ERR not to free invalid
>> pointer, and this will fix the segfault issue.
>>
>> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> Fixes: 65c93628599d ("bpftool: Add struct_ops support")
> Acked-by: Martin KaFai Lau

Applied & fixed up email, thanks!

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

end of thread, other threads:[~2020-04-14 19:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-10  2:06 [PATCH] tools: bpftool: fix struct_ops command invalid pointer free Daniel T. Lee
2020-04-10  5:03 ` Martin KaFai Lau
2020-04-10  5:34   ` Daniel T. Lee
2020-04-14 19:44   ` 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).