From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3183BC10F03 for ; Fri, 1 Mar 2019 16:43:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DF6B720857 for ; Fri, 1 Mar 2019 16:43:20 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=Mellanox.com header.i=@Mellanox.com header.b="RKw5bFAa" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2389183AbfCAQnT (ORCPT ); Fri, 1 Mar 2019 11:43:19 -0500 Received: from mail-eopbgr70074.outbound.protection.outlook.com ([40.107.7.74]:33112 "EHLO EUR04-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S2388922AbfCAQnR (ORCPT ); Fri, 1 Mar 2019 11:43:17 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=t1dydB5Dnd++KvDDhEuAdzrfN6qIsQrsMg8dW6j1ZJc=; b=RKw5bFAa46AL6mIOz51wKtm2Q47trSO4Y5LgSpef82Awh8SiV+lI1a4gxAAPrNaNCOK1y0bRjNrGsx1ZdQn7mPD/D5tDL0kr0KMBUcwUfnj0Fnujex0ac689nnHcWhm6Ego7J8UULUrRHNDevdXyUY7TmsRHZUQt6dg81wNUrZY= Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com (10.167.127.11) by HE1PR0502MB3833.eurprd05.prod.outlook.com (10.167.143.12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1665.17; Fri, 1 Mar 2019 16:43:06 +0000 Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com ([fe80::b03d:8cd4:d259:f749]) by HE1PR0502MB3641.eurprd05.prod.outlook.com ([fe80::b03d:8cd4:d259:f749%6]) with mapi id 15.20.1665.017; Fri, 1 Mar 2019 16:43:06 +0000 From: Vlad Buslov To: Davide Caratti CC: "David S. Miller" , Jamal Hadi Salim , Cong Wang , Jiri Pirko , "pabeni@redhat.com" , "netdev@vger.kernel.org" Subject: Re: [PATCH net 13/16] net/sched: act_skbedit: validate the control action inside init() Thread-Topic: [PATCH net 13/16] net/sched: act_skbedit: validate the control action inside init() Thread-Index: AQHU0E3YlN1Gizvr60K+avucuVENEg== Date: Fri, 1 Mar 2019 16:43:06 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: LO2P123CA0022.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:a6::34) To HE1PR0502MB3641.eurprd05.prod.outlook.com (2603:10a6:7:85::11) authentication-results: spf=none (sender IP is ) smtp.mailfrom=vladbu@mellanox.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [37.142.13.130] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 8c2a2f8e-71ec-42fd-aaa2-08d69e64fa9b x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(4618075)(2017052603328)(7153060)(7193020);SRVR:HE1PR0502MB3833; x-ms-traffictypediagnostic: HE1PR0502MB3833: x-microsoft-exchange-diagnostics: =?iso-8859-1?Q?1;HE1PR0502MB3833;23:pC5vu9jUwXM48hodIopp6EH1I2wJot/h4AhpR?= =?iso-8859-1?Q?XGTR074nXePXb5JuJ758mIKtwHYhmEx8SsC1wf2CXmhL/tpEGNqNIsQDAy?= =?iso-8859-1?Q?qgRy99FjtNOtwBNNIe/twn53fncjKR4mXBuzAJKfCyYWK7VKUDLRJSAQSw?= =?iso-8859-1?Q?9t12c9o0n3P6w9fhnxgqTXUTqzzDN8MEChNz24Hn0SQKKcUWTnumqCRUTZ?= =?iso-8859-1?Q?Xc5lsHKTPQIqrvHFkkteuqIy5Jh1tvzwDFTJfRxkI1X6uckd5SSVifPfFa?= =?iso-8859-1?Q?uazdF+ByZ99cbsDSSZDopFkeM3Ezq49ywGJYudfYrVb9WDZQST1jV6BRI4?= =?iso-8859-1?Q?sJsycbRrWjy0uqv5x6rQo3k7YYKkqvB1zyhhmSvWPj5ZS+bGkgSaMdjtQv?= =?iso-8859-1?Q?Ybuu6GIE5aNrb6t0byEmDIAqyNRC1mvdiTzcU0CQ+eWsmXH4SyXB1xuRRC?= =?iso-8859-1?Q?KOKq9oejxF9fXoCmIV8qkReC2J6k2RL7du8QhJO2y0umvRMHnt3gLGXGXB?= =?iso-8859-1?Q?Z43KIRA7p0UlcnUHJdPANvMUzR2sxnNa6Bik+TkG9pBu3BbD0HxfCuFXGb?= =?iso-8859-1?Q?QIuc0xVzXLS/LynXitsZDlSmIqfZL1NEEr8pGGremfChCQaCJru+QBwbL3?= =?iso-8859-1?Q?ounFBftzLGNrpcdqMRY/cmED/b3FkXcBXoccI8zm8/1qaKvcTOlgkAvXb9?= =?iso-8859-1?Q?lBX16j6RoYh3gCmAmeozvpUtVGVAE1+4T3RdgJJ5yJf/oDpt+b/qkDnCwE?= =?iso-8859-1?Q?DN2+JfO0S+gftMjfOVh0ecM7Nx3nhQI49+me9Pcs8DtTyR+Vuu+wBq19Rg?= =?iso-8859-1?Q?ndoqRhBlqnDssS0OH2WzlYvBUMVafjWZpNnZR9pcMAz1ubpEduYjQ+YsYu?= =?iso-8859-1?Q?OAcWAT3wlacAxtiJz7iqVADM+xP5tXnSsiW7RVyMEma75h6dSBwWVqU4mv?= =?iso-8859-1?Q?15HHu+bDVY01LxZvuTX9ZDhu9nGlEu6f9dWHgwLWUkvwAVg3AOGzZRya54?= =?iso-8859-1?Q?MppcZ4ojJGoEeqOGUsNWCHQt/uwdT7K3Exf+alOt1p8TT92wBthwkyuFVf?= =?iso-8859-1?Q?g2SkxeIx4fxcnOmO0Ah89jyLXELkDRtPoZthNnfWc+Bzs63VybMd4puv9G?= =?iso-8859-1?Q?Eukl0q/q9y1hvgRD6C/W0VGFm8pXB4iPRoD27sbLcxhTGJGejvzTaJMFnF?= =?iso-8859-1?Q?VWEK7/VwxlVJtjr/PNXaxRz8StgHgyNN/eG/KwLQBvi5Q4gx/Rbq3Ie2Vt?= =?iso-8859-1?Q?W1YBQ9p7/6MrdxUvwIGl0yyu2MlbbRhhOBXwKdLRcpKAMycOLwu9RzOGxT?= =?iso-8859-1?Q?R5yXvthhtCPLQ+kpqFRAkVk5MyG3nMLt/U5BgYWwDHN2hveWB+lztuPkaN?= =?iso-8859-1?Q?kwouOnWnLs=3D?= x-microsoft-antispam-prvs: x-forefront-prvs: 09634B1196 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(376002)(39860400002)(396003)(346002)(366004)(136003)(189003)(199004)(51234002)(53936002)(6512007)(2616005)(105586002)(305945005)(54906003)(478600001)(81156014)(6916009)(386003)(106356001)(52116002)(102836004)(86362001)(99286004)(476003)(8936002)(76176011)(6506007)(7736002)(81166006)(66066001)(316002)(25786009)(486006)(45080400002)(71200400001)(4326008)(71190400001)(186003)(446003)(97736004)(6116002)(11346002)(3846002)(8676002)(14454004)(5660300002)(26005)(15650500001)(68736007)(6486002)(14444005)(6246003)(2906002)(36756003)(229853002)(256004)(6436002);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0502MB3833;H:HE1PR0502MB3641.eurprd05.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: qhrp5LKlw0F+u2djpkckWkrY3/R3zTzzPscEIfQS00vGEcY8BmYQpZp4NI5qmAs1CY04MRCUJsp0vi3kwdlGYEBZ48MY+VYnolB3D+yAgC2gdfcNF4h+QSWre4XYCTSBJxUGPdMSw/7Mq44AXKrFse7025+lQnmKCT0+c8KrmsWtim3C3d6aCHbIoFyiWiWnxZKn2EoIYxdCfsYdpJTzDzed+VrJEbzZHZNPsmjiOYhY52zDckkPg+mXa2A8mpFqzY85AHapIbNNG3TQqjperQCpYh2ylSXN3jfwPR7WH0nmFEjWqc8l5FSXWUoHW2+smXHucMxwI06csSvOC2W32MoDA4oC1qY/dSjDc0nqZN2PhRBnFhgdYFgiJ0K9+l4mSEmoybZVgKjT/Yr92ToYSAF9mP07fAbwowayNB9cCl8= Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8c2a2f8e-71ec-42fd-aaa2-08d69e64fa9b X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Mar 2019 16:43:06.0779 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0502MB3833 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed 27 Feb 2019 at 17:40, Davide Caratti wrote: > the following script: > > # tc qdisc add dev crash0 clsact > # tc filter add dev crash0 egress matchall \ > > action skbedit ptype host pass index 90 > # tc actions replace action skbedit \ > > ptype host goto chain 42 index 90 cookie c1a0c1a0 > # tc actions show action skbedit > > had the following output: > > Error: Failed to init TC action chain. > We have an error talking to the kernel > total acts 1 > > action order 0: skbedit ptype host goto chain 42 > index 90 ref 2 bind 1 > cookie c1a0c1a0 > > Then, the first packet transmitted by crash0 made the kernel crash: > > BUG: unable to handle kernel NULL pointer dereference at 000000000000000= 0 > #PF error: [normal kernel read fault] > PGD 0 P4D 0 > Oops: 0000 [#1] SMP PTI > CPU: 3 PID: 3467 Comm: kworker/3:3 Not tainted 5.0.0-rc4.gotochain_crash= + #536 > Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011 > Workqueue: ipv6_addrconf addrconf_dad_work > RIP: 0010:tcf_action_exec+0xb8/0x100 > Code: 00 00 00 20 74 1d 83 f8 03 75 09 49 83 c4 08 4d 39 ec 75 bc 48 83 = c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 49 8b 97 a8 00 00 00 <48> 8b 12 48 8= 9 55 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 > RSP: 0018:ffffb50a81e1fad0 EFLAGS: 00010246 > RAX: 000000002000002a RBX: ffff9aa47ba4ea00 RCX: 0000000000000001 > RDX: 0000000000000000 RSI: ffff9aa469eeb3c0 RDI: ffff9aa47ba4ea00 > RBP: ffffb50a81e1fb70 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: ffff9aa47bce0638 R12: ffff9aa4793b0c00 > R13: ffff9aa4793b0c08 R14: 0000000000000001 R15: ffff9aa469eeb3c0 > FS: 0000000000000000(0000) GS:ffff9aa474780000(0000) knlGS:000000000000= 0000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000000 CR3: 000000007360e005 CR4: 00000000001606e0 > Call Trace: > tcf_classify+0x58/0x120 > __dev_queue_xmit+0x40a/0x890 > ? ndisc_next_option+0x50/0x50 > ? ___neigh_create+0x4d5/0x680 > ? ip6_finish_output2+0x1b5/0x590 > ip6_finish_output2+0x1b5/0x590 > ? ip6_output+0x68/0x110 > ip6_output+0x68/0x110 > ? nf_hook.constprop.28+0x79/0xc0 > ndisc_send_skb+0x248/0x2e0 > ndisc_send_ns+0xf8/0x200 > ? addrconf_dad_work+0x389/0x4b0 > addrconf_dad_work+0x389/0x4b0 > ? __switch_to_asm+0x34/0x70 > ? process_one_work+0x195/0x380 > ? addrconf_dad_completed+0x370/0x370 > process_one_work+0x195/0x380 > worker_thread+0x30/0x390 > ? process_one_work+0x380/0x380 > kthread+0x113/0x130 > ? kthread_park+0x90/0x90 > ret_from_fork+0x35/0x40 > Modules linked in: act_skbedit veth ip6table_filter ip6_tables iptable_f= ilter binfmt_misc crct10dif_pclmul crc32_pclmul ghash_clmulni_intel ext4 sn= d_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep mbcache snd_hda_c= ore jbd2 snd_seq snd_seq_device snd_pcm aesni_intel crypto_simd cryptd snd_= timer glue_helper snd joydev soundcore pcspkr virtio_balloon i2c_piix4 nfsd= auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs ata_generic pata_acpi= qxl drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm virti= o_net net_failover drm failover virtio_blk virtio_console ata_piix virtio_p= ci crc32c_intel serio_raw libata virtio_ring virtio floppy dm_mirror dm_reg= ion_hash dm_log dm_mod > CR2: 0000000000000000 > > Validating the control action within tcf_skbedit_init() proved to fix the > above issue. A TDC selftest is added to verify the correct behavior. > > Fixes: db50514f9a9c ("net: sched: add termination action to allow goto ch= ain") > Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values") > Signed-off-by: Davide Caratti > --- > net/sched/act_skbedit.c | 22 +++++++++++++--- > .../tc-testing/tc-tests/actions/skbedit.json | 25 +++++++++++++++++++ > 2 files changed, 43 insertions(+), 4 deletions(-) > > diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c > index 9a8a0f2d4418..5a0d35b2dac3 100644 > --- a/net/sched/act_skbedit.c > +++ b/net/sched/act_skbedit.c > @@ -26,6 +26,7 @@ > #include > #include > #include > +#include > =20 > #include > #include > @@ -100,6 +101,7 @@ static int tcf_skbedit_init(struct net *net, struct n= lattr *nla, > struct netlink_ext_ack *extack) > { > struct tc_action_net *tn =3D net_generic(net, skbedit_net_id); > + struct tcf_chain *newchain =3D NULL, *oldchain; > struct tcf_skbedit_params *params_new; > struct nlattr *tb[TCA_SKBEDIT_MAX + 1]; > struct tc_skbedit *parm; > @@ -187,12 +189,13 @@ static int tcf_skbedit_init(struct net *net, struct= nlattr *nla, > return -EEXIST; > } > } > + err =3D tcf_action_check_ctrlact(parm->action, tp, &newchain, extack); > + if (err) > + goto release_idr; > =20 > params_new =3D kzalloc(sizeof(*params_new), GFP_KERNEL); > - if (unlikely(!params_new)) { > - tcf_idr_release(*a, bind); > - return -ENOMEM; > - } > + if (unlikely(!params_new)) > + goto put_chain; Set err=3D-ENOMEM before goto. > =20 > params_new->flags =3D flags; > if (flags & SKBEDIT_F_PRIORITY) > @@ -209,6 +212,8 @@ static int tcf_skbedit_init(struct net *net, struct n= lattr *nla, > params_new->mask =3D *mask; > =20 > spin_lock_bh(&d->tcf_lock); > + oldchain =3D d->tcf_goto_chain; > + d->tcf_goto_chain =3D newchain; > d->tcf_action =3D parm->action; > rcu_swap_protected(d->params, params_new, > lockdep_is_held(&d->tcf_lock)); > @@ -218,7 +223,16 @@ static int tcf_skbedit_init(struct net *net, struct = nlattr *nla, > =20 > if (ret =3D=3D ACT_P_CREATED) > tcf_idr_insert(tn, *a); > + if (oldchain) > + tcf_chain_put_by_act(oldchain); > return ret; > + > +put_chain: > + if (newchain) > + tcf_chain_put_by_act(newchain); > +release_idr: > + tcf_idr_release(*a, bind); > + return err; > } > =20 > static int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a, > diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.= json b/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json > index 5aaf593b914a..ecd96eda7f6a 100644 > --- a/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json > +++ b/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json > @@ -484,5 +484,30 @@ > "teardown": [ > "$TC actions flush action skbedit" > ] > + }, > + { > + "id": "1b2b", > + "name": "Replace skbedit action with invalid goto_chain control"= , > + "category": [ > + "actions", > + "skbedit" > + ], > + "setup": [ > + [ > + "$TC actions flush action skbedit", > + 0, > + 1, > + 255 > + ], > + "$TC actions add action skbedit ptype host pass index 90" > + ], > + "cmdUnderTest": "$TC actions replace action skbedit ptype host g= oto chain 42 index 90 cookie c1a0c1a0", > + "expExitCode": "255", > + "verifyCmd": "$TC actions list action skbedit", > + "matchPattern": "action order [0-9]*: skbedit ptype host pass.*= index 90 ref", > + "matchCount": "1", > + "teardown": [ > + "$TC actions flush action skbedit" > + ] > } > ]