diff mbox series

[iproute2] tc: fix segmentation fault on gact action

Message ID b498237ee5b99e4687ba5068de09da91ef315235.1569924170.git.aclaudi@redhat.com
State Accepted
Delegated to: stephen hemminger
Headers show
Series [iproute2] tc: fix segmentation fault on gact action | expand

Commit Message

Andrea Claudi Oct. 1, 2019, 10:32 a.m. UTC
tc segfaults if gact action is used without action or index:

$ ip link add type dummy
$ tc actions add action pipe index 1
$ tc filter add dev dummy0 parent ffff: protocol ip \
  pref 10 u32 match ip src 127.0.0.2 flowid 1:10 action gact
Segmentation fault

We expect tc to fail gracefully with an error message.

This happens if gact is the last argument of the incomplete
command. In this case the "gact" action is parsed, the macro
NEXT_ARG_FWD() is executed and the next matches() crashes
because of null argv pointer.

To avoid this, simply use NEXT_ARG() instead.

With this change in place:

$ ip link add type dummy
$ tc actions add action pipe index 1
$ tc filter add dev dummy0 parent ffff: protocol ip \
  pref 10 u32 match ip src 127.0.0.2 flowid 1:10 action gact
Command line is not complete. Try option "help"

Fixes: fa4958897314 ("tc: Fix binding of gact action by index.")
Reported-by: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
 tc/m_gact.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stephen Hemminger Oct. 9, 2019, 3:20 a.m. UTC | #1
On Tue,  1 Oct 2019 12:32:17 +0200
Andrea Claudi <aclaudi@redhat.com> wrote:

> tc segfaults if gact action is used without action or index:
> 
> $ ip link add type dummy
> $ tc actions add action pipe index 1
> $ tc filter add dev dummy0 parent ffff: protocol ip \
>   pref 10 u32 match ip src 127.0.0.2 flowid 1:10 action gact
> Segmentation fault
> 
> We expect tc to fail gracefully with an error message.
> 
> This happens if gact is the last argument of the incomplete
> command. In this case the "gact" action is parsed, the macro
> NEXT_ARG_FWD() is executed and the next matches() crashes
> because of null argv pointer.
> 
> To avoid this, simply use NEXT_ARG() instead.
> 
> With this change in place:
> 
> $ ip link add type dummy
> $ tc actions add action pipe index 1
> $ tc filter add dev dummy0 parent ffff: protocol ip \
>   pref 10 u32 match ip src 127.0.0.2 flowid 1:10 action gact
> Command line is not complete. Try option "help"
> 
> Fixes: fa4958897314 ("tc: Fix binding of gact action by index.")
> Reported-by: Davide Caratti <dcaratti@redhat.com>
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>

Thanks, applied
diff mbox series

Patch

diff --git a/tc/m_gact.c b/tc/m_gact.c
index dca2a2f9692fd..b06e8ee95818f 100644
--- a/tc/m_gact.c
+++ b/tc/m_gact.c
@@ -87,7 +87,7 @@  parse_gact(struct action_util *a, int *argc_p, char ***argv_p,
 		return -1;
 
 	if (!matches(*argv, "gact"))
-		NEXT_ARG_FWD();
+		NEXT_ARG();
 	/* we're binding existing gact action to filter by index. */
 	if (!matches(*argv, "index"))
 		goto skip_args;