From: Davide Caratti <dcaratti@redhat.com>
To: "David S. Miller" <davem@davemloft.net>,
Jamal Hadi Salim <jhs@mojatatu.com>,
Cong Wang <xiyou.wangcong@gmail.com>,
Jiri Pirko <jiri@resnulli.us>, Vlad Buslov <vladbu@mellanox.com>
Cc: pabeni@redhat.com, netdev@vger.kernel.org
Subject: [PATCH net 00/16] validate the control action with all the other parameters
Date: Wed, 27 Feb 2019 18:40:28 +0100 [thread overview]
Message-ID: <cover.1551288982.git.dcaratti@redhat.com> (raw)
currently, the kernel checks for bad values of the control action in
tcf_action_init_1(), after a successful call to the action's init()
function. This causes three bad behaviors:
1. the "half configuration"
if the action is overwritten, the new configuration data are
applied successfully - but the value of the control action remains
the previous one.
2. the "refcount leak that makes kmemleak complain"
when a valid 'goto chain' action is overwritten with another 'goto chain'
action, the kernel leaks two refcounts.
3. the "kernel crash in the traffic plane"
when an action is overwritten with an invalid 'goto chain' action,
packets hitting the new rule will trigger a NULL pointer dereference.
all the above three problems can be fixed if we validate the control
action in each action's init() handler, the same way as we are already
doing for all the other parameters.
Changes since RFC:
- include a fix for all TC actions
- add a selftest for each TC action
- squash fix for refcount leaks into a single patch, the first in the
series, thanks to Cong Wang
- ensure that chain refcount is released without tcfa_lock held, thanks
to Vlad Buslov
Notes:
- act_ipt didn't need any fix, as the control action is constantly equal
to TC_ACT_OK.
- the selftest for act_simple fails because userspace tc backend for
'simple' does not parse the control action correctly (and hardcodes it
to TC_ACT_PIPE).
Davide Caratti (16):
net/sched: prepare TC actions to properly validate the control action
net/sched: act_bpf: validate the control action inside init()
net/sched: act_csum: validate the control action inside init()
net/sched: act_gact: validate the control action inside init()
net/sched: act_ife: validate the control action inside init()
net/sched: act_mirred: validate the control action inside init()
net/sched: act_connmark: validate the control action inside init()
net/sched: act_nat: validate the control action inside init()
net/sched: act_pedit: validate the control action inside init()
net/sched: act_police: validate the control action inside init()
net/sched: act_sample: validate the control action inside init()
net/sched: act_simple: validate the control action inside init()
net/sched: act_skbedit: validate the control action inside init()
net/sched: act_skbmod: validate the control action inside init()
net/sched: act_tunnel_key: validate the control action inside init()
net/sched: act_vlan: validate the control action inside init()
include/net/act_api.h | 7 +-
net/sched/act_api.c | 84 +++++++++++--------
net/sched/act_bpf.c | 15 +++-
net/sched/act_connmark.c | 25 +++++-
net/sched/act_csum.c | 23 ++++-
net/sched/act_gact.c | 15 +++-
net/sched/act_ife.c | 34 +++++---
net/sched/act_ipt.c | 11 +--
net/sched/act_mirred.c | 24 +++++-
net/sched/act_nat.c | 15 +++-
net/sched/act_pedit.c | 19 ++++-
net/sched/act_police.c | 13 +++
net/sched/act_sample.c | 22 ++++-
net/sched/act_simple.c | 52 +++++++++---
net/sched/act_skbedit.c | 23 ++++-
net/sched/act_skbmod.c | 27 ++++--
net/sched/act_tunnel_key.c | 19 ++++-
net/sched/act_vlan.c | 23 ++++-
.../tc-testing/tc-tests/actions/bpf.json | 25 ++++++
.../tc-testing/tc-tests/actions/connmark.json | 25 ++++++
.../tc-testing/tc-tests/actions/csum.json | 25 ++++++
.../tc-testing/tc-tests/actions/gact.json | 25 ++++++
.../tc-testing/tc-tests/actions/ife.json | 25 ++++++
.../tc-testing/tc-tests/actions/mirred.json | 25 ++++++
.../tc-testing/tc-tests/actions/nat.json | 25 ++++++
.../tc-testing/tc-tests/actions/pedit.json | 51 +++++++++++
.../tc-testing/tc-tests/actions/police.json | 25 ++++++
.../tc-testing/tc-tests/actions/sample.json | 25 ++++++
.../tc-testing/tc-tests/actions/simple.json | 25 ++++++
.../tc-testing/tc-tests/actions/skbedit.json | 25 ++++++
.../tc-testing/tc-tests/actions/skbmod.json | 25 ++++++
.../tc-tests/actions/tunnel_key.json | 25 ++++++
.../tc-testing/tc-tests/actions/vlan.json | 25 ++++++
33 files changed, 757 insertions(+), 95 deletions(-)
create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/pedit.json
--
2.20.1
next reply other threads:[~2019-02-27 17:40 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-27 17:40 Davide Caratti [this message]
2019-02-27 17:40 ` [PATCH net 01/16] net/sched: prepare TC actions to properly validate the control action Davide Caratti
2019-02-28 1:28 ` Cong Wang
2019-03-01 17:59 ` Davide Caratti
2019-02-27 17:40 ` [PATCH net 02/16] net/sched: act_bpf: validate the control action inside init() Davide Caratti
2019-02-27 17:40 ` [PATCH net 03/16] net/sched: act_csum: " Davide Caratti
2019-02-28 1:50 ` Cong Wang
2019-03-01 18:02 ` Davide Caratti
2019-03-02 0:04 ` Cong Wang
2019-03-07 13:56 ` Davide Caratti
2019-03-07 14:51 ` Vlad Buslov
2019-03-07 16:56 ` Davide Caratti
2019-03-08 17:47 ` Davide Caratti
2019-02-27 17:40 ` [PATCH net 04/16] net/sched: act_gact: " Davide Caratti
2019-02-27 17:40 ` [PATCH net 05/16] net/sched: act_ife: " Davide Caratti
2019-03-01 16:33 ` Vlad Buslov
2019-02-27 17:40 ` [PATCH net 06/16] net/sched: act_mirred: " Davide Caratti
2019-02-27 17:40 ` [PATCH net 07/16] net/sched: act_connmark: " Davide Caratti
2019-03-01 16:35 ` Vlad Buslov
2019-02-27 17:40 ` [PATCH net 08/16] net/sched: act_nat: " Davide Caratti
2019-02-27 17:40 ` [PATCH net 09/16] net/sched: act_pedit: " Davide Caratti
2019-02-27 17:40 ` [PATCH net 10/16] net/sched: act_police: " Davide Caratti
2019-02-27 17:40 ` [PATCH net 11/16] net/sched: act_sample: " Davide Caratti
2019-02-27 17:40 ` [PATCH net 12/16] net/sched: act_simple: " Davide Caratti
2019-03-01 16:39 ` Vlad Buslov
2019-02-27 17:40 ` [PATCH net 13/16] net/sched: act_skbedit: " Davide Caratti
2019-03-01 16:43 ` Vlad Buslov
2019-02-27 17:40 ` [PATCH net 14/16] net/sched: act_skbmod: " Davide Caratti
2019-02-27 17:40 ` [PATCH net 15/16] net/sched: act_tunnel_key: " Davide Caratti
2019-02-27 17:40 ` [PATCH net 16/16] net/sched: act_vlan: " Davide Caratti
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1551288982.git.dcaratti@redhat.com \
--to=dcaratti@redhat.com \
--cc=davem@davemloft.net \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=vladbu@mellanox.com \
--cc=xiyou.wangcong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.