All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tools bpftool: Fix json dump crash on powerpc
@ 2019-07-04  8:58 Jiri Olsa
  2019-07-04 17:37 ` Quentin Monnet
  2019-07-04 20:42 ` Jakub Kicinski
  0 siblings, 2 replies; 8+ messages in thread
From: Jiri Olsa @ 2019-07-04  8:58 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Michael Petlan, netdev, bpf, Martin KaFai Lau

Michael reported crash with by bpf program in json mode on powerpc:

  # bpftool prog -p dump jited id 14
  [{
        "name": "0xd00000000a9aa760",
        "insns": [{
                "pc": "0x0",
                "operation": "nop",
                "operands": [null
                ]
            },{
                "pc": "0x4",
                "operation": "nop",
                "operands": [null
                ]
            },{
                "pc": "0x8",
                "operation": "mflr",
  Segmentation fault (core dumped)

The code is assuming char pointers in format, which is not always
true at least for powerpc. Fixing this by dumping the whole string
into buffer based on its format.

Please note that libopcodes code does not check return values from
fprintf callback, so there's no point to return error in case of
allocation failure.

Reported-by: Michael Petlan <mpetlan@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/bpf/bpftool/jit_disasm.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index 3ef3093560ba..05fa6dc970f8 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -11,6 +11,8 @@
  * Licensed under the GNU General Public License, version 2.0 (GPLv2)
  */
 
+#define _GNU_SOURCE
+#include <stdio.h>
 #include <stdarg.h>
 #include <stdint.h>
 #include <stdio.h>
@@ -44,11 +46,13 @@ static int fprintf_json(void *out, const char *fmt, ...)
 	char *s;
 
 	va_start(ap, fmt);
+	if (vasprintf(&s, fmt, ap) < 0)
+		return 0;
+	va_end(ap);
+
 	if (!oper_count) {
 		int i;
 
-		s = va_arg(ap, char *);
-
 		/* Strip trailing spaces */
 		i = strlen(s) - 1;
 		while (s[i] == ' ')
@@ -61,11 +65,10 @@ static int fprintf_json(void *out, const char *fmt, ...)
 	} else if (!strcmp(fmt, ",")) {
 		   /* Skip */
 	} else {
-		s = va_arg(ap, char *);
 		jsonw_string(json_wtr, s);
 		oper_count++;
 	}
-	va_end(ap);
+	free(s);
 	return 0;
 }
 
-- 
2.21.0


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

* Re: [PATCH] tools bpftool: Fix json dump crash on powerpc
  2019-07-04  8:58 [PATCH] tools bpftool: Fix json dump crash on powerpc Jiri Olsa
@ 2019-07-04 17:37 ` Quentin Monnet
  2019-07-04 20:42 ` Jakub Kicinski
  1 sibling, 0 replies; 8+ messages in thread
From: Quentin Monnet @ 2019-07-04 17:37 UTC (permalink / raw)
  To: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann
  Cc: Michael Petlan, netdev, bpf, Martin KaFai Lau

2019-07-04 10:58 UTC+0200 ~ Jiri Olsa <jolsa@kernel.org>
> Michael reported crash with by bpf program in json mode on powerpc:
> 
>   # bpftool prog -p dump jited id 14
>   [{
>         "name": "0xd00000000a9aa760",
>         "insns": [{
>                 "pc": "0x0",
>                 "operation": "nop",
>                 "operands": [null
>                 ]
>             },{
>                 "pc": "0x4",
>                 "operation": "nop",
>                 "operands": [null
>                 ]
>             },{
>                 "pc": "0x8",
>                 "operation": "mflr",
>   Segmentation fault (core dumped)
> 
> The code is assuming char pointers in format, which is not always
> true at least for powerpc. Fixing this by dumping the whole string
> into buffer based on its format.
> 
> Please note that libopcodes code does not check return values from
> fprintf callback, so there's no point to return error in case of
> allocation failure.
> 
> Reported-by: Michael Petlan <mpetlan@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

Looks good to me, thank you for the fix!

Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>

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

* Re: [PATCH] tools bpftool: Fix json dump crash on powerpc
  2019-07-04  8:58 [PATCH] tools bpftool: Fix json dump crash on powerpc Jiri Olsa
  2019-07-04 17:37 ` Quentin Monnet
@ 2019-07-04 20:42 ` Jakub Kicinski
  2019-07-05 12:10   ` [PATCHv2] " Jiri Olsa
  1 sibling, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2019-07-04 20:42 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Michael Petlan, netdev, bpf,
	Martin KaFai Lau

On Thu,  4 Jul 2019 10:58:56 +0200, Jiri Olsa wrote:
> Michael reported crash with by bpf program in json mode on powerpc:
> 
>   # bpftool prog -p dump jited id 14
>   [{
>         "name": "0xd00000000a9aa760",
>         "insns": [{
>                 "pc": "0x0",
>                 "operation": "nop",
>                 "operands": [null
>                 ]
>             },{
>                 "pc": "0x4",
>                 "operation": "nop",
>                 "operands": [null
>                 ]
>             },{
>                 "pc": "0x8",
>                 "operation": "mflr",
>   Segmentation fault (core dumped)
> 
> The code is assuming char pointers in format, which is not always
> true at least for powerpc. Fixing this by dumping the whole string
> into buffer based on its format.
> 
> Please note that libopcodes code does not check return values from
> fprintf callback, so there's no point to return error in case of
> allocation failure.

Well, it doesn't check it today, it may perhaps do it in the future?
Let's flip the question - since it doesn't check it today, why not
propagate the error? :)  We should stay close to how fprintf would
behave, IMHO.

Fixes: 107f041212c1 ("tools: bpftool: add JSON output for `bpftool prog dump jited *` command")

> Reported-by: Michael Petlan <mpetlan@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

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

* [PATCHv2] tools bpftool: Fix json dump crash on powerpc
  2019-07-04 20:42 ` Jakub Kicinski
@ 2019-07-05 12:10   ` Jiri Olsa
  2019-07-05 17:24     ` Jakub Kicinski
  0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2019-07-05 12:10 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Michael Petlan,
	netdev, bpf, Martin KaFai Lau

On Thu, Jul 04, 2019 at 01:42:10PM -0700, Jakub Kicinski wrote:
> On Thu,  4 Jul 2019 10:58:56 +0200, Jiri Olsa wrote:
> > Michael reported crash with by bpf program in json mode on powerpc:
> > 
> >   # bpftool prog -p dump jited id 14
> >   [{
> >         "name": "0xd00000000a9aa760",
> >         "insns": [{
> >                 "pc": "0x0",
> >                 "operation": "nop",
> >                 "operands": [null
> >                 ]
> >             },{
> >                 "pc": "0x4",
> >                 "operation": "nop",
> >                 "operands": [null
> >                 ]
> >             },{
> >                 "pc": "0x8",
> >                 "operation": "mflr",
> >   Segmentation fault (core dumped)
> > 
> > The code is assuming char pointers in format, which is not always
> > true at least for powerpc. Fixing this by dumping the whole string
> > into buffer based on its format.
> > 
> > Please note that libopcodes code does not check return values from
> > fprintf callback, so there's no point to return error in case of
> > allocation failure.
> 
> Well, it doesn't check it today, it may perhaps do it in the future?
> Let's flip the question - since it doesn't check it today, why not
> propagate the error? :)  We should stay close to how fprintf would
> behave, IMHO.
> 
> Fixes: 107f041212c1 ("tools: bpftool: add JSON output for `bpftool prog dump jited *` command")

ok fair enough, v2 attached

thanks,
jirka


---
Michael reported crash with by bpf program in json mode on powerpc:

  # bpftool prog -p dump jited id 14
  [{
        "name": "0xd00000000a9aa760",
        "insns": [{
                "pc": "0x0",
                "operation": "nop",
                "operands": [null
                ]
            },{
                "pc": "0x4",
                "operation": "nop",
                "operands": [null
                ]
            },{
                "pc": "0x8",
                "operation": "mflr",
  Segmentation fault (core dumped)

The code is assuming char pointers in format, which is not always
true at least for powerpc. Fixing this by dumping the whole string
into buffer based on its format.

Please note that libopcodes code does not check return values from
fprintf callback, but as per Jakub suggestion returning -1 on allocation
failure so we do the best effort to propagate the error. 

Reported-by: Michael Petlan <mpetlan@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/bpf/bpftool/jit_disasm.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index 3ef3093560ba..bfed711258ce 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -11,6 +11,8 @@
  * Licensed under the GNU General Public License, version 2.0 (GPLv2)
  */
 
+#define _GNU_SOURCE
+#include <stdio.h>
 #include <stdarg.h>
 #include <stdint.h>
 #include <stdio.h>
@@ -44,11 +46,13 @@ static int fprintf_json(void *out, const char *fmt, ...)
 	char *s;
 
 	va_start(ap, fmt);
+	if (vasprintf(&s, fmt, ap) < 0)
+		return -1;
+	va_end(ap);
+
 	if (!oper_count) {
 		int i;
 
-		s = va_arg(ap, char *);
-
 		/* Strip trailing spaces */
 		i = strlen(s) - 1;
 		while (s[i] == ' ')
@@ -61,11 +65,10 @@ static int fprintf_json(void *out, const char *fmt, ...)
 	} else if (!strcmp(fmt, ",")) {
 		   /* Skip */
 	} else {
-		s = va_arg(ap, char *);
 		jsonw_string(json_wtr, s);
 		oper_count++;
 	}
-	va_end(ap);
+	free(s);
 	return 0;
 }
 
-- 
2.21.0


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

* Re: [PATCHv2] tools bpftool: Fix json dump crash on powerpc
  2019-07-05 12:10   ` [PATCHv2] " Jiri Olsa
@ 2019-07-05 17:24     ` Jakub Kicinski
  2019-07-05 17:26       ` Quentin Monnet
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2019-07-05 17:24 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Michael Petlan,
	netdev, bpf, Martin KaFai Lau, Quentin Monnet

On Fri, 5 Jul 2019 14:10:31 +0200, Jiri Olsa wrote:
> Michael reported crash with by bpf program in json mode on powerpc:
> 
>   # bpftool prog -p dump jited id 14
>   [{
>         "name": "0xd00000000a9aa760",
>         "insns": [{
>                 "pc": "0x0",
>                 "operation": "nop",
>                 "operands": [null
>                 ]
>             },{
>                 "pc": "0x4",
>                 "operation": "nop",
>                 "operands": [null
>                 ]
>             },{
>                 "pc": "0x8",
>                 "operation": "mflr",
>   Segmentation fault (core dumped)
> 
> The code is assuming char pointers in format, which is not always
> true at least for powerpc. Fixing this by dumping the whole string
> into buffer based on its format.
> 
> Please note that libopcodes code does not check return values from
> fprintf callback, but as per Jakub suggestion returning -1 on allocation
> failure so we do the best effort to propagate the error. 
> 
> Reported-by: Michael Petlan <mpetlan@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

Thanks, let me repost all the tags (Quentin, please shout if you're
not ok with this :)):

Fixes: 107f041212c1 ("tools: bpftool: add JSON output for `bpftool prog dump jited *` command")
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>

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

* Re: [PATCHv2] tools bpftool: Fix json dump crash on powerpc
  2019-07-05 17:24     ` Jakub Kicinski
@ 2019-07-05 17:26       ` Quentin Monnet
  2019-07-05 22:00         ` Daniel Borkmann
  0 siblings, 1 reply; 8+ messages in thread
From: Quentin Monnet @ 2019-07-05 17:26 UTC (permalink / raw)
  To: Jakub Kicinski, Jiri Olsa
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Michael Petlan,
	netdev, bpf, Martin KaFai Lau

2019-07-05 10:24 UTC-0700 ~ Jakub Kicinski <jakub.kicinski@netronome.com>
> On Fri, 5 Jul 2019 14:10:31 +0200, Jiri Olsa wrote:
>> Michael reported crash with by bpf program in json mode on powerpc:
>>
>>   # bpftool prog -p dump jited id 14
>>   [{
>>         "name": "0xd00000000a9aa760",
>>         "insns": [{
>>                 "pc": "0x0",
>>                 "operation": "nop",
>>                 "operands": [null
>>                 ]
>>             },{
>>                 "pc": "0x4",
>>                 "operation": "nop",
>>                 "operands": [null
>>                 ]
>>             },{
>>                 "pc": "0x8",
>>                 "operation": "mflr",
>>   Segmentation fault (core dumped)
>>
>> The code is assuming char pointers in format, which is not always
>> true at least for powerpc. Fixing this by dumping the whole string
>> into buffer based on its format.
>>
>> Please note that libopcodes code does not check return values from
>> fprintf callback, but as per Jakub suggestion returning -1 on allocation
>> failure so we do the best effort to propagate the error. 
>>
>> Reported-by: Michael Petlan <mpetlan@redhat.com>
>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> 
> Thanks, let me repost all the tags (Quentin, please shout if you're
> not ok with this :)):

I confirm it's all good for me, thanks :)

> 
> Fixes: 107f041212c1 ("tools: bpftool: add JSON output for `bpftool prog dump jited *` command")
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> 


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

* Re: [PATCHv2] tools bpftool: Fix json dump crash on powerpc
  2019-07-05 17:26       ` Quentin Monnet
@ 2019-07-05 22:00         ` Daniel Borkmann
  2019-07-09 17:53           ` Jiri Olsa
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Borkmann @ 2019-07-05 22:00 UTC (permalink / raw)
  To: Quentin Monnet, Jakub Kicinski, Jiri Olsa
  Cc: Jiri Olsa, Alexei Starovoitov, Michael Petlan, netdev, bpf,
	Martin KaFai Lau

On 07/05/2019 07:26 PM, Quentin Monnet wrote:
> 2019-07-05 10:24 UTC-0700 ~ Jakub Kicinski <jakub.kicinski@netronome.com>
>> On Fri, 5 Jul 2019 14:10:31 +0200, Jiri Olsa wrote:
>>> Michael reported crash with by bpf program in json mode on powerpc:
>>>
>>>   # bpftool prog -p dump jited id 14
>>>   [{
>>>         "name": "0xd00000000a9aa760",
>>>         "insns": [{
>>>                 "pc": "0x0",
>>>                 "operation": "nop",
>>>                 "operands": [null
>>>                 ]
>>>             },{
>>>                 "pc": "0x4",
>>>                 "operation": "nop",
>>>                 "operands": [null
>>>                 ]
>>>             },{
>>>                 "pc": "0x8",
>>>                 "operation": "mflr",
>>>   Segmentation fault (core dumped)
>>>
>>> The code is assuming char pointers in format, which is not always
>>> true at least for powerpc. Fixing this by dumping the whole string
>>> into buffer based on its format.
>>>
>>> Please note that libopcodes code does not check return values from
>>> fprintf callback, but as per Jakub suggestion returning -1 on allocation
>>> failure so we do the best effort to propagate the error. 
>>>
>>> Reported-by: Michael Petlan <mpetlan@redhat.com>
>>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>>
>> Thanks, let me repost all the tags (Quentin, please shout if you're
>> not ok with this :)):
> 
> I confirm it's all good for me, thanks :)
> 
>> Fixes: 107f041212c1 ("tools: bpftool: add JSON output for `bpftool prog dump jited *` command")
>> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
>> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Given merge window coming up, I've applied this to bpf-next, thanks everyone!

P.s.: Jiri, please repost full/proper patch next time instead of inline reply.

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

* Re: [PATCHv2] tools bpftool: Fix json dump crash on powerpc
  2019-07-05 22:00         ` Daniel Borkmann
@ 2019-07-09 17:53           ` Jiri Olsa
  0 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2019-07-09 17:53 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Quentin Monnet, Jakub Kicinski, Jiri Olsa, Alexei Starovoitov,
	Michael Petlan, netdev, bpf, Martin KaFai Lau

On Sat, Jul 06, 2019 at 12:00:44AM +0200, Daniel Borkmann wrote:
> On 07/05/2019 07:26 PM, Quentin Monnet wrote:
> > 2019-07-05 10:24 UTC-0700 ~ Jakub Kicinski <jakub.kicinski@netronome.com>
> >> On Fri, 5 Jul 2019 14:10:31 +0200, Jiri Olsa wrote:
> >>> Michael reported crash with by bpf program in json mode on powerpc:
> >>>
> >>>   # bpftool prog -p dump jited id 14
> >>>   [{
> >>>         "name": "0xd00000000a9aa760",
> >>>         "insns": [{
> >>>                 "pc": "0x0",
> >>>                 "operation": "nop",
> >>>                 "operands": [null
> >>>                 ]
> >>>             },{
> >>>                 "pc": "0x4",
> >>>                 "operation": "nop",
> >>>                 "operands": [null
> >>>                 ]
> >>>             },{
> >>>                 "pc": "0x8",
> >>>                 "operation": "mflr",
> >>>   Segmentation fault (core dumped)
> >>>
> >>> The code is assuming char pointers in format, which is not always
> >>> true at least for powerpc. Fixing this by dumping the whole string
> >>> into buffer based on its format.
> >>>
> >>> Please note that libopcodes code does not check return values from
> >>> fprintf callback, but as per Jakub suggestion returning -1 on allocation
> >>> failure so we do the best effort to propagate the error. 
> >>>
> >>> Reported-by: Michael Petlan <mpetlan@redhat.com>
> >>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> >>
> >> Thanks, let me repost all the tags (Quentin, please shout if you're
> >> not ok with this :)):
> > 
> > I confirm it's all good for me, thanks :)
> > 
> >> Fixes: 107f041212c1 ("tools: bpftool: add JSON output for `bpftool prog dump jited *` command")
> >> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> >> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> 
> Given merge window coming up, I've applied this to bpf-next, thanks everyone!
> 
> P.s.: Jiri, please repost full/proper patch next time instead of inline reply.

will do, thanks
jirka

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

end of thread, other threads:[~2019-07-09 17:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-04  8:58 [PATCH] tools bpftool: Fix json dump crash on powerpc Jiri Olsa
2019-07-04 17:37 ` Quentin Monnet
2019-07-04 20:42 ` Jakub Kicinski
2019-07-05 12:10   ` [PATCHv2] " Jiri Olsa
2019-07-05 17:24     ` Jakub Kicinski
2019-07-05 17:26       ` Quentin Monnet
2019-07-05 22:00         ` Daniel Borkmann
2019-07-09 17:53           ` Jiri Olsa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.