From patchwork Wed Feb 27 17:40:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Davide Caratti X-Patchwork-Id: 1049057 Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 448jdk3pyCz9s1b for ; Thu, 28 Feb 2019 04:40:58 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729986AbfB0Rk4 (ORCPT ); Wed, 27 Feb 2019 12:40:56 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39284 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726223AbfB0Rk4 (ORCPT ); Wed, 27 Feb 2019 12:40:56 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AE1A230BC12F; Wed, 27 Feb 2019 17:40:55 +0000 (UTC) Received: from localhost.localdomain.com (unknown [10.32.181.77]) by smtp.corp.redhat.com (Postfix) with ESMTP id CB09E6063A; Wed, 27 Feb 2019 17:40:53 +0000 (UTC) From: Davide Caratti To: "David S. Miller" , Jamal Hadi Salim , Cong Wang , Jiri Pirko , Vlad Buslov 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 Message-Id: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.42]); Wed, 27 Feb 2019 17:40:56 +0000 (UTC) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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