* [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions
@ 2018-10-20 21:33 Davide Caratti
2018-10-20 21:33 ` [PATCH net 1/4] net/sched: act_gact: disallow 'goto chain' on fallback control action Davide Caratti
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Davide Caratti @ 2018-10-20 21:33 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang, Jamal Hadi Salim, David S. Miller, netdev
the following command:
# tc actions add action police rate 1mbit burst 1k conform-exceed \
> pass / goto chain 42
generates a NULL pointer dereference when packets exceed the configured
rate. Similarly, the following command:
# tc actions add action pass random determ goto chain 42 2
makes the kernel crash with NULL dereference when the first packet does
not match the 'pass' action.
gact and police allow users to specify a fallback control action, that is
stored in the action private data. 'goto chain x' never worked for these
cases, since a->goto_chain handle was never initialized. There is only one
goto_chain handle per TC action, and it is designed to be non-NULL only if
tcf_action contains a 'goto chain' command. So, let's forbid 'goto chain'
on fallback actions.
Patch 1/4 and 2/4 change the .init() functions of police and gact, to let
them return an error when users try to set 'goto chain x' in the fallback
action. Patch 3/4 and 4/4 add TDC selftest coverage to this new behavior.
Davide Caratti (4):
net/sched: act_gact: disallow 'goto chain' on fallback control action
net/sched: act_police: disallow 'goto chain' on fallback control
action
tc-tests: test denial of 'goto chain' on 'random' traffic in gact.json
tc-tests: test denial of 'goto chain' for exceed traffic in
police.json
net/sched/act_gact.c | 5 ++++
net/sched/act_police.c | 12 ++++++++--
.../tc-testing/tc-tests/actions/gact.json | 24 +++++++++++++++++++
.../tc-testing/tc-tests/actions/police.json | 24 +++++++++++++++++++
4 files changed, 63 insertions(+), 2 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net 1/4] net/sched: act_gact: disallow 'goto chain' on fallback control action
2018-10-20 21:33 [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions Davide Caratti
@ 2018-10-20 21:33 ` Davide Caratti
2018-10-22 16:22 ` Cong Wang
2018-10-22 20:55 ` Jiri Pirko
2018-10-20 21:33 ` [PATCH net 2/4] net/sched: act_police: " Davide Caratti
` (4 subsequent siblings)
5 siblings, 2 replies; 11+ messages in thread
From: Davide Caratti @ 2018-10-20 21:33 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang, Jamal Hadi Salim, David S. Miller, netdev
in the following command:
# tc action add action <c1> random <rand_type> <c2> <rand_param>
'goto chain x' is allowed only for c1: setting it for c2 makes the kernel
crash with NULL pointer dereference, since TC core doesn't initialize the
chain handle.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_gact.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index cd1d9bd32ef9..505138047e5c 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -88,6 +88,11 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
p_parm = nla_data(tb[TCA_GACT_PROB]);
if (p_parm->ptype >= MAX_RAND)
return -EINVAL;
+ if (TC_ACT_EXT_CMP(p_parm->paction, TC_ACT_GOTO_CHAIN)) {
+ NL_SET_ERR_MSG(extack,
+ "goto chain not allowed on fallback");
+ return -EINVAL;
+ }
}
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net 2/4] net/sched: act_police: disallow 'goto chain' on fallback control action
2018-10-20 21:33 [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions Davide Caratti
2018-10-20 21:33 ` [PATCH net 1/4] net/sched: act_gact: disallow 'goto chain' on fallback control action Davide Caratti
@ 2018-10-20 21:33 ` Davide Caratti
2018-10-22 16:23 ` Cong Wang
2018-10-22 20:57 ` Jiri Pirko
2018-10-20 21:33 ` [PATCH net 3/4] tc-tests: test denial of 'goto chain' on 'random' traffic in gact.json Davide Caratti
` (3 subsequent siblings)
5 siblings, 2 replies; 11+ messages in thread
From: Davide Caratti @ 2018-10-20 21:33 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang, Jamal Hadi Salim, David S. Miller, netdev
in the following command:
# tc action add action police rate <r> burst <b> conform-exceed <c1>/<c2>
'goto chain x' is allowed only for c1: setting it for c2 makes the kernel
crash with NULL pointer dereference, since TC core doesn't initialize the
chain handle.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_police.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 5d8bfa878477..3b793393efd1 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -150,6 +150,16 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
goto failure;
}
+ if (tb[TCA_POLICE_RESULT]) {
+ police->tcfp_result = nla_get_u32(tb[TCA_POLICE_RESULT]);
+ if (TC_ACT_EXT_CMP(police->tcfp_result, TC_ACT_GOTO_CHAIN)) {
+ NL_SET_ERR_MSG(extack,
+ "goto chain not allowed on fallback");
+ err = -EINVAL;
+ goto failure;
+ }
+ }
+
spin_lock_bh(&police->tcf_lock);
/* No failure allowed after this point */
police->tcfp_mtu = parm->mtu;
@@ -173,8 +183,6 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
police->peak_present = false;
}
- if (tb[TCA_POLICE_RESULT])
- police->tcfp_result = nla_get_u32(tb[TCA_POLICE_RESULT]);
police->tcfp_burst = PSCHED_TICKS2NS(parm->burst);
police->tcfp_toks = police->tcfp_burst;
if (police->peak_present) {
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net 3/4] tc-tests: test denial of 'goto chain' on 'random' traffic in gact.json
2018-10-20 21:33 [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions Davide Caratti
2018-10-20 21:33 ` [PATCH net 1/4] net/sched: act_gact: disallow 'goto chain' on fallback control action Davide Caratti
2018-10-20 21:33 ` [PATCH net 2/4] net/sched: act_police: " Davide Caratti
@ 2018-10-20 21:33 ` Davide Caratti
2018-10-20 21:33 ` [PATCH net 4/4] tc-tests: test denial of 'goto chain' for exceed traffic in police.json Davide Caratti
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Davide Caratti @ 2018-10-20 21:33 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang, Jamal Hadi Salim, David S. Miller, netdev
add test to verify if act_gact forbids 'goto chain' control actions on
'random' traffic in gact.json.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
.../tc-testing/tc-tests/actions/gact.json | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
index 68c91023cdb9..89189a03ce3d 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/gact.json
@@ -536,5 +536,29 @@
"matchPattern": "^[ \t]+index [0-9]+ ref",
"matchCount": "0",
"teardown": []
+ },
+ {
+ "id": "8e47",
+ "name": "Add gact action with random determ goto chain control action",
+ "category": [
+ "actions",
+ "gact"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action gact",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action pass random determ goto chain 1 2 index 90",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions list action gact",
+ "matchPattern": "action order [0-9]*: gact action pass random type determ goto chain 1 val 2.*index 90 ref",
+ "matchCount": "0",
+ "teardown": [
+ "$TC actions flush action gact"
+ ]
}
]
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net 4/4] tc-tests: test denial of 'goto chain' for exceed traffic in police.json
2018-10-20 21:33 [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions Davide Caratti
` (2 preceding siblings ...)
2018-10-20 21:33 ` [PATCH net 3/4] tc-tests: test denial of 'goto chain' on 'random' traffic in gact.json Davide Caratti
@ 2018-10-20 21:33 ` Davide Caratti
2018-10-22 18:39 ` [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions Jamal Hadi Salim
2018-10-23 2:43 ` David Miller
5 siblings, 0 replies; 11+ messages in thread
From: Davide Caratti @ 2018-10-20 21:33 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang, Jamal Hadi Salim, David S. Miller, netdev
add test to verify if act_police forbids 'goto chain' control actions for
'exceed' traffic.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
.../tc-testing/tc-tests/actions/police.json | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
index 30f9b54bd666..4086a50a670e 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
@@ -715,5 +715,29 @@
"teardown": [
"$TC actions flush action police"
]
+ },
+ {
+ "id": "b48b",
+ "name": "Add police action with exceed goto chain control action",
+ "category": [
+ "actions",
+ "police"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action police",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action police rate 1mbit burst 1k conform-exceed pass / goto chain 42",
+ "expExitCode": "255",
+ "verifyCmd": "$TC actions ls action police",
+ "matchPattern": "action order [0-9]*: police 0x1 rate 1Mbit burst 1Kb mtu 2Kb action pass/goto chain 42",
+ "matchCount": "0",
+ "teardown": [
+ "$TC actions flush action police"
+ ]
}
]
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net 1/4] net/sched: act_gact: disallow 'goto chain' on fallback control action
2018-10-20 21:33 ` [PATCH net 1/4] net/sched: act_gact: disallow 'goto chain' on fallback control action Davide Caratti
@ 2018-10-22 16:22 ` Cong Wang
2018-10-22 20:55 ` Jiri Pirko
1 sibling, 0 replies; 11+ messages in thread
From: Cong Wang @ 2018-10-22 16:22 UTC (permalink / raw)
To: Davide Caratti
Cc: Jiri Pirko, Jamal Hadi Salim, David Miller,
Linux Kernel Network Developers
On Sat, Oct 20, 2018 at 2:33 PM Davide Caratti <dcaratti@redhat.com> wrote:
>
> in the following command:
>
> # tc action add action <c1> random <rand_type> <c2> <rand_param>
>
> 'goto chain x' is allowed only for c1: setting it for c2 makes the kernel
> crash with NULL pointer dereference, since TC core doesn't initialize the
> chain handle.
>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net 2/4] net/sched: act_police: disallow 'goto chain' on fallback control action
2018-10-20 21:33 ` [PATCH net 2/4] net/sched: act_police: " Davide Caratti
@ 2018-10-22 16:23 ` Cong Wang
2018-10-22 20:57 ` Jiri Pirko
1 sibling, 0 replies; 11+ messages in thread
From: Cong Wang @ 2018-10-22 16:23 UTC (permalink / raw)
To: Davide Caratti
Cc: Jiri Pirko, Jamal Hadi Salim, David Miller,
Linux Kernel Network Developers
On Sat, Oct 20, 2018 at 2:33 PM Davide Caratti <dcaratti@redhat.com> wrote:
>
> in the following command:
>
> # tc action add action police rate <r> burst <b> conform-exceed <c1>/<c2>
>
> 'goto chain x' is allowed only for c1: setting it for c2 makes the kernel
> crash with NULL pointer dereference, since TC core doesn't initialize the
> chain handle.
>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions
2018-10-20 21:33 [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions Davide Caratti
` (3 preceding siblings ...)
2018-10-20 21:33 ` [PATCH net 4/4] tc-tests: test denial of 'goto chain' for exceed traffic in police.json Davide Caratti
@ 2018-10-22 18:39 ` Jamal Hadi Salim
2018-10-23 2:43 ` David Miller
5 siblings, 0 replies; 11+ messages in thread
From: Jamal Hadi Salim @ 2018-10-22 18:39 UTC (permalink / raw)
To: Davide Caratti, Jiri Pirko, Cong Wang, David S. Miller, netdev
On 2018-10-20 5:33 p.m., Davide Caratti wrote:
> the following command:
>
> # tc actions add action police rate 1mbit burst 1k conform-exceed \
> > pass / goto chain 42
>
> generates a NULL pointer dereference when packets exceed the configured
> rate. Similarly, the following command:
>
> # tc actions add action pass random determ goto chain 42 2
>
> makes the kernel crash with NULL dereference when the first packet does
> not match the 'pass' action.
>
> gact and police allow users to specify a fallback control action, that is
> stored in the action private data. 'goto chain x' never worked for these
> cases, since a->goto_chain handle was never initialized. There is only one
> goto_chain handle per TC action, and it is designed to be non-NULL only if
> tcf_action contains a 'goto chain' command. So, let's forbid 'goto chain'
> on fallback actions.
>
> Patch 1/4 and 2/4 change the .init() functions of police and gact, to let
> them return an error when users try to set 'goto chain x' in the fallback
> action. Patch 3/4 and 4/4 add TDC selftest coverage to this new behavior.
>
For the series,
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
cheers,
jamal
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net 1/4] net/sched: act_gact: disallow 'goto chain' on fallback control action
2018-10-20 21:33 ` [PATCH net 1/4] net/sched: act_gact: disallow 'goto chain' on fallback control action Davide Caratti
2018-10-22 16:22 ` Cong Wang
@ 2018-10-22 20:55 ` Jiri Pirko
1 sibling, 0 replies; 11+ messages in thread
From: Jiri Pirko @ 2018-10-22 20:55 UTC (permalink / raw)
To: Davide Caratti; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller, netdev
Sat, Oct 20, 2018 at 11:33:07PM CEST, dcaratti@redhat.com wrote:
>in the following command:
>
> # tc action add action <c1> random <rand_type> <c2> <rand_param>
>
>'goto chain x' is allowed only for c1: setting it for c2 makes the kernel
>crash with NULL pointer dereference, since TC core doesn't initialize the
>chain handle.
>
>Signed-off-by: Davide Caratti <dcaratti@redhat.com>
>---
> net/sched/act_gact.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
>index cd1d9bd32ef9..505138047e5c 100644
>--- a/net/sched/act_gact.c
>+++ b/net/sched/act_gact.c
>@@ -88,6 +88,11 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
> p_parm = nla_data(tb[TCA_GACT_PROB]);
> if (p_parm->ptype >= MAX_RAND)
> return -EINVAL;
>+ if (TC_ACT_EXT_CMP(p_parm->paction, TC_ACT_GOTO_CHAIN)) {
>+ NL_SET_ERR_MSG(extack,
>+ "goto chain not allowed on fallback");
No need for a line-wrap. Otherwise
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net 2/4] net/sched: act_police: disallow 'goto chain' on fallback control action
2018-10-20 21:33 ` [PATCH net 2/4] net/sched: act_police: " Davide Caratti
2018-10-22 16:23 ` Cong Wang
@ 2018-10-22 20:57 ` Jiri Pirko
1 sibling, 0 replies; 11+ messages in thread
From: Jiri Pirko @ 2018-10-22 20:57 UTC (permalink / raw)
To: Davide Caratti; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller, netdev
Sat, Oct 20, 2018 at 11:33:08PM CEST, dcaratti@redhat.com wrote:
>in the following command:
>
> # tc action add action police rate <r> burst <b> conform-exceed <c1>/<c2>
>
>'goto chain x' is allowed only for c1: setting it for c2 makes the kernel
>crash with NULL pointer dereference, since TC core doesn't initialize the
>chain handle.
>
>Signed-off-by: Davide Caratti <dcaratti@redhat.com>
>---
> net/sched/act_police.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
>diff --git a/net/sched/act_police.c b/net/sched/act_police.c
>index 5d8bfa878477..3b793393efd1 100644
>--- a/net/sched/act_police.c
>+++ b/net/sched/act_police.c
>@@ -150,6 +150,16 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
> goto failure;
> }
>
>+ if (tb[TCA_POLICE_RESULT]) {
>+ police->tcfp_result = nla_get_u32(tb[TCA_POLICE_RESULT]);
>+ if (TC_ACT_EXT_CMP(police->tcfp_result, TC_ACT_GOTO_CHAIN)) {
>+ NL_SET_ERR_MSG(extack,
>+ "goto chain not allowed on fallback");
Also, no need for line-wrap
Acked-by: Jiri Pirko <jiri@mellanox.com>
>+ err = -EINVAL;
>+ goto failure;
>+ }
>+ }
>+
> spin_lock_bh(&police->tcf_lock);
> /* No failure allowed after this point */
> police->tcfp_mtu = parm->mtu;
>@@ -173,8 +183,6 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
> police->peak_present = false;
> }
>
>- if (tb[TCA_POLICE_RESULT])
>- police->tcfp_result = nla_get_u32(tb[TCA_POLICE_RESULT]);
> police->tcfp_burst = PSCHED_TICKS2NS(parm->burst);
> police->tcfp_toks = police->tcfp_burst;
> if (police->peak_present) {
>--
>2.17.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions
2018-10-20 21:33 [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions Davide Caratti
` (4 preceding siblings ...)
2018-10-22 18:39 ` [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions Jamal Hadi Salim
@ 2018-10-23 2:43 ` David Miller
5 siblings, 0 replies; 11+ messages in thread
From: David Miller @ 2018-10-23 2:43 UTC (permalink / raw)
To: dcaratti; +Cc: jiri, xiyou.wangcong, jhs, netdev
From: Davide Caratti <dcaratti@redhat.com>
Date: Sat, 20 Oct 2018 23:33:06 +0200
> the following command:
>
> # tc actions add action police rate 1mbit burst 1k conform-exceed \
> > pass / goto chain 42
>
> generates a NULL pointer dereference when packets exceed the configured
> rate. Similarly, the following command:
>
> # tc actions add action pass random determ goto chain 42 2
>
> makes the kernel crash with NULL dereference when the first packet does
> not match the 'pass' action.
>
> gact and police allow users to specify a fallback control action, that is
> stored in the action private data. 'goto chain x' never worked for these
> cases, since a->goto_chain handle was never initialized. There is only one
> goto_chain handle per TC action, and it is designed to be non-NULL only if
> tcf_action contains a 'goto chain' command. So, let's forbid 'goto chain'
> on fallback actions.
>
> Patch 1/4 and 2/4 change the .init() functions of police and gact, to let
> them return an error when users try to set 'goto chain x' in the fallback
> action. Patch 3/4 and 4/4 add TDC selftest coverage to this new behavior.
Series applied.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-10-23 11:04 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-20 21:33 [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions Davide Caratti
2018-10-20 21:33 ` [PATCH net 1/4] net/sched: act_gact: disallow 'goto chain' on fallback control action Davide Caratti
2018-10-22 16:22 ` Cong Wang
2018-10-22 20:55 ` Jiri Pirko
2018-10-20 21:33 ` [PATCH net 2/4] net/sched: act_police: " Davide Caratti
2018-10-22 16:23 ` Cong Wang
2018-10-22 20:57 ` Jiri Pirko
2018-10-20 21:33 ` [PATCH net 3/4] tc-tests: test denial of 'goto chain' on 'random' traffic in gact.json Davide Caratti
2018-10-20 21:33 ` [PATCH net 4/4] tc-tests: test denial of 'goto chain' for exceed traffic in police.json Davide Caratti
2018-10-22 18:39 ` [PATCH net 0/4] net/sched: forbid 'goto_chain' on fallback actions Jamal Hadi Salim
2018-10-23 2:43 ` David Miller
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).