netdev - [PATCH net 00/16] validate the control action with all the other parameters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <cover.1551288982.git.dcaratti@redhat.com>
Date:   Wed, 27 Feb 2019 18:40:28 +0100
From:   Davide Caratti <dcaratti@...hat.com>
To:     "David S. Miller" <davem@...emloft.net>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Vlad Buslov <vladbu@...lanox.com>
Cc:     pabeni@...hat.com, netdev@...r.kernel.org
Subject: [PATCH net 00/16] validate the control action with all the other parameters

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