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 20D73C43381 for ; Fri, 1 Mar 2019 16:33:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CFFC420857 for ; Fri, 1 Mar 2019 16:33:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=Mellanox.com header.i=@Mellanox.com header.b="tpC6cvyy" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388994AbfCAQdg (ORCPT ); Fri, 1 Mar 2019 11:33:36 -0500 Received: from mail-eopbgr10052.outbound.protection.outlook.com ([40.107.1.52]:11397 "EHLO EUR02-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728449AbfCAQdg (ORCPT ); Fri, 1 Mar 2019 11:33:36 -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=GFbvqBWGXrMy/o8wsNYII7nPwFi7QR2msCNiu7bHc/8=; b=tpC6cvyyA4MlRk7YG6r3KXNyU2dDijH2ZIBMb8wumPJ8QglkVeJCK8dLOqrkHrnvAreA9/TX2Je7CjaSFsHEwcw++8wz0QZWZl4l2DTpCZl4g/+R7sckk0sr1JAfp5mhW9surFLANlUPea+6wfQA0dBtPLh46x+RcScSd+E6JAo= Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com (10.167.127.11) by HE1PR0502MB3882.eurprd05.prod.outlook.com (10.167.143.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1665.16; Fri, 1 Mar 2019 16:33:30 +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:33:30 +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 05/16] net/sched: act_ife: validate the control action inside init() Thread-Topic: [PATCH net 05/16] net/sched: act_ife: validate the control action inside init() Thread-Index: AQHU0EyAPDEMljNT6UOmmUx9B5BX8A== Date: Fri, 1 Mar 2019 16:33:29 +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: LO2P265CA0387.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:f::15) 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: 738336cd-3efb-4558-8968-08d69e63a32f 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:HE1PR0502MB3882; x-ms-traffictypediagnostic: HE1PR0502MB3882: x-microsoft-exchange-diagnostics: =?iso-8859-1?Q?1;HE1PR0502MB3882;23:OUl/LVNuqj9Ni1zsLcvTGcIGWYGLfS73TaPmo?= =?iso-8859-1?Q?SNxI+fVmMKyQGwG/bcKy5nXFp0IZJnAbtaLYv6J96wfaKU6Dv6fR9gOJpE?= =?iso-8859-1?Q?lHmSpNj9uyL0rTcgdr6x5f4eBkg7F8Ort3LQjV5CIknfKy+xykHY96/nwA?= =?iso-8859-1?Q?vOkSbKYR/MPhbspY34KqUl8WOgX22MPRde7fX7Lb4VezaK8mepohPm43wE?= =?iso-8859-1?Q?CCcPd+yXlSLT1Ptqfhbxf2u7HxcwZNFFl8HstlLhjQ9hxvHBMwrpkK4cQh?= =?iso-8859-1?Q?fPYTXj/gJEcXxr0QP1qS/86wrsSAjTtYzAshHgUhHTDOm6GGKogceydhkA?= =?iso-8859-1?Q?YC9+y3a7QDj2Wqp2JdFKlHEqN5xGruLSLO3q6mJkVLV17xqWmmSqqvMq7e?= =?iso-8859-1?Q?mM7fO0TA6Yc31yadAm1R+DhHZlt9ipSUIl9fpW3f4L51ZWQepMIzlqgn42?= =?iso-8859-1?Q?WHYpOHGPSnXqY4tN5Qw3cBf9Lq2ukh+vCtpB1MykHNO8g9oPk8UQn9Ftm1?= =?iso-8859-1?Q?4JzLyqOIvgCUwoqAFqUjH0EFEKAf2jAmw+WaeI4+MZ/I2c3hXhHcyAuoQH?= =?iso-8859-1?Q?Mib4QmfbnmHfq+SYslMLZrdU0gEzQ5608r1X0EJj9QGO8OtyA/j+C6bXO7?= =?iso-8859-1?Q?1hn9tdeGvi/BzCwETIEWOuFlH4DEGCcjRbEGn2OkenBZVhyj7qIhj0n3DP?= =?iso-8859-1?Q?1SxY2j+zzcewGJu40CKecvm3t0GSHPtVCYvhWgUUWhhrT7HNfDMePRhkiG?= =?iso-8859-1?Q?atgUu7oxA3n3UqC4MZ2ZzduDI+TwFkXsEGKxs93M3c6AdD7VAl4rvmiMOo?= =?iso-8859-1?Q?/a8leeP4b5n4/6AlfcIyTPjSjrpqFbxZWJy5Kj98qctX1Nc4uezvtJthtn?= =?iso-8859-1?Q?wil/bdk/rwZOmYNjjLcjIuns/iycf+FAW3MvOpizONzKc5fnEBXhBn3irO?= =?iso-8859-1?Q?BhJyq148fCCWks6kCbrcCVVTmhevbMUgfmmMlPaiFuotUgEuSM4evB8KeC?= =?iso-8859-1?Q?zRV5xDviFB3z2PVK3YCR2mfdDunY/y/bSlYbgNA/UFplSKzsJv4C8fpAn2?= =?iso-8859-1?Q?pvmx5YDH8SzQVZVzcSETu/g/iLqpc81MjUF727NQUEAnJF+1LyPCK4zA9l?= =?iso-8859-1?Q?aEobAXW6iSdQbAXEfmvn3yvl+fuyPYIZQUiEzdjALusAUpEgWZcIfE+K5E?= =?iso-8859-1?Q?RWJmL8RO4xv9uqYMWpGAoJIMO9XPlhz4mY17ZQaNGGb2FE1AhcFPqUsFwI?= =?iso-8859-1?Q?+UT6YcRDESD5EmOfVUgcxLeO2QwLd1M5ClPI7iaFZ7YdZbeIO7YEA8YJf7?= =?iso-8859-1?Q?N14edtv7W7KVrWA4m2TV0enhUrZ3tVYqkWU232KHy/aR7VF/qrys3mU/UD?= =?iso-8859-1?Q?jI4KO2hCho=3D?= x-microsoft-antispam-prvs: x-forefront-prvs: 09634B1196 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(366004)(346002)(376002)(396003)(136003)(39860400002)(199004)(189003)(51234002)(6916009)(2906002)(316002)(54906003)(14454004)(97736004)(26005)(6246003)(6512007)(15650500001)(186003)(106356001)(478600001)(6486002)(68736007)(66066001)(6436002)(229853002)(36756003)(45080400002)(2616005)(11346002)(476003)(446003)(53936002)(105586002)(71200400001)(486006)(76176011)(25786009)(6506007)(386003)(4326008)(99286004)(52116002)(86362001)(8676002)(81156014)(71190400001)(81166006)(3846002)(14444005)(305945005)(7736002)(5660300002)(102836004)(8936002)(256004)(6116002);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0502MB3882;H:HE1PR0502MB3641.eurprd05.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX: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: xDamdTqWArNAXVfFhCP2qYSutpT9wNqqzbc5s5lQuOvjY4WOLQaNwE8W+eWyRG8+yzp481EpKc7Yr5McINKZH/Esvy81cUKFVohZAzIxqMTFm9fUH0CWDplW9WjfzchLNzgkOoxLlCWw38oPbMsTnY9hoh7+q6e+eeWie4vmwR5djYSI+AO48Yvgc6F1GEKK5Ny+0v0Yej+17B5mIOkeIucvwlovmDu/ERhNAlgqkIPh5qx75pzah/al5cw699yMebaGxYQ28E9ei8QPFyYbYB84AANbvXoYoWTOQguznXPsklnPR/x2jZQWtzG1hFWFK76JIFAXxMfSP3lKqMuTF4oLwUcaZF8yj7LFZbs1d7gji67h3zwTffIa3QTY3BOcqL2uvdXvLQszS9n4TUZS4Zm9OFl2lThnnkCFd5Y96ag= 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: 738336cd-3efb-4558-8968-08d69e63a32f X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Mar 2019 16:33:29.9969 (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: HE1PR0502MB3882 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 ife encode allow mark pass index 90 > # tc actions replace action ife \ > > encode allow mark goto chain 42 index 90 cookie c1a0c1a0 > # tc action show action ife > > had the following output: > > IFE type 0xED3E > IFE type 0xED3E > Error: Failed to init TC action chain. > We have an error talking to the kernel > total acts 1 > > action order 0: ife encode action goto chain 42 type 0XED3E > allow mark > 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 800000007b4e7067 P4D 800000007b4e7067 PUD 7b4e6067 PMD 0 > Oops: 0000 [#1] SMP PTI > CPU: 2 PID: 164 Comm: kworker/2:1 Not tainted 5.0.0-rc4.gotochain_crash+= #533 > 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:ffffa6a7c0553ad0 EFLAGS: 00010246 > RAX: 000000002000002a RBX: ffff9796ee1bbd00 RCX: 0000000000000001 > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 > RBP: ffffa6a7c0553b70 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: ffff9797385bb038 R12: ffff9796ead9d700 > R13: ffff9796ead9d708 R14: 0000000000000001 R15: ffff9796ead9d800 > FS: 0000000000000000(0000) GS:ffff97973db00000(0000) knlGS:000000000000= 0000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000000 CR3: 000000007c41e006 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_gact act_meta_mark act_ife dummy veth ip6table_fi= lter ip6_tables iptable_filter binfmt_misc snd_hda_codec_generic ext4 snd_h= da_intel snd_hda_codec crct10dif_pclmul mbcache crc32_pclmul jbd2 snd_hwdep= snd_hda_core ghash_clmulni_intel snd_seq snd_seq_device snd_pcm snd_timer = aesni_intel crypto_simd snd cryptd glue_helper virtio_balloon joydev pcspkr= soundcore i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables = xfs ata_generic pata_acpi qxl virtio_net drm_kms_helper virtio_blk net_fail= over syscopyarea failover sysfillrect virtio_console sysimgblt fb_sys_fops = ttm drm crc32c_intel serio_raw ata_piix virtio_pci virtio_ring libata virti= o floppy dm_mirror dm_region_hash dm_log dm_mod [last unloaded: act_ife] > CR2: 0000000000000000 > > Validating the control action within tcf_ife_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_ife.c | 32 ++++++++++++------- > .../tc-testing/tc-tests/actions/ife.json | 25 +++++++++++++++ > 2 files changed, 45 insertions(+), 12 deletions(-) > > diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c > index 9b2eb941e093..4480edb386d7 100644 > --- a/net/sched/act_ife.c > +++ b/net/sched/act_ife.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -472,6 +473,7 @@ static int tcf_ife_init(struct net *net, struct nlatt= r *nla, > struct tcf_proto *tp, struct netlink_ext_ack *extack) > { > struct tc_action_net *tn =3D net_generic(net, ife_net_id); > + struct tcf_chain *newchain =3D NULL, *oldchain; > struct nlattr *tb[TCA_IFE_MAX + 1]; > struct nlattr *tb2[IFE_META_MAX + 1]; > struct tcf_ife_params *p; > @@ -531,6 +533,10 @@ static int tcf_ife_init(struct net *net, struct nlat= tr *nla, > } > =20 > ife =3D to_ife(*a); > + err =3D tcf_action_check_ctrlact(parm->action, tp, &newchain, extack); > + if (err) > + goto release_idr; > + > p->flags =3D parm->flags; > =20 > if (parm->flags & IFE_ENCODE) { > @@ -563,13 +569,8 @@ static int tcf_ife_init(struct net *net, struct nlat= tr *nla, > if (tb[TCA_IFE_METALST]) { > err =3D nla_parse_nested(tb2, IFE_META_MAX, tb[TCA_IFE_METALST], > NULL, NULL); > - if (err) { > -metadata_parse_err: > - tcf_idr_release(*a, bind); > - kfree(p); > - return err; > - } > - > + if (err) > + goto metadata_parse_err; > err =3D populate_metalist(ife, tb2, exists, rtnl_held); > if (err) > goto metadata_parse_err; > @@ -581,15 +582,14 @@ static int tcf_ife_init(struct net *net, struct nla= ttr *nla, > * going to bail out > */ > err =3D use_all_metadata(ife, exists); > - if (err) { > - tcf_idr_release(*a, bind); > - kfree(p); > - return err; > - } > + if (err) > + goto metadata_parse_err; > } > =20 > if (exists) > spin_lock_bh(&ife->tcf_lock); > + oldchain =3D ife->tcf_goto_chain; > + ife->tcf_goto_chain =3D newchain; > ife->tcf_action =3D parm->action; > /* protected by tcf_lock when modifying existing action */ > rcu_swap_protected(ife->params, p, 1); > @@ -603,6 +603,14 @@ static int tcf_ife_init(struct net *net, struct nlat= tr *nla, > tcf_idr_insert(tn, *a); > oldchain is not released before return. > return ret; > + > +metadata_parse_err: > + if (newchain) > + tcf_chain_put_by_act(newchain); > +release_idr: > + kfree(p); > + tcf_idr_release(*a, bind); > + return err; > } > =20 > static int tcf_ife_dump(struct sk_buff *skb, struct tc_action *a, int bi= nd, > diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json= b/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json > index 0da3545cabdb..c13a68b98fc7 100644 > --- a/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json > +++ b/tools/testing/selftests/tc-testing/tc-tests/actions/ife.json > @@ -1060,5 +1060,30 @@ > "matchPattern": "action order [0-9]*: ife encode action pipe.*al= low prio.*index 4", > "matchCount": "0", > "teardown": [] > + }, > + { > + "id": "a0e2", > + "name": "Replace ife encode action with invalid goto chain contr= ol", > + "category": [ > + "actions", > + "ife" > + ], > + "setup": [ > + [ > + "$TC actions flush action ife", > + 0, > + 1, > + 255 > + ], > + "$TC actions add action ife encode allow mark pass index 90" > + ], > + "cmdUnderTest": "$TC actions replace action ife encode allow mar= k goto chain 42 index 90 cookie c1a0c1a0", > + "expExitCode": "255", > + "verifyCmd": "$TC actions get action ife index 90", > + "matchPattern": "action order [0-9]*: ife encode action pass.*ty= pe 0[xX]ED3E .*allow mark.*index 90 ref", > + "matchCount": "1", > + "teardown": [ > + "$TC actions flush action ife" > + ] > } > ]