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=-1.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,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 CEF13C43381 for ; Mon, 25 Feb 2019 15:12:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 84CC320842 for ; Mon, 25 Feb 2019 15:12:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=Mellanox.com header.i=@Mellanox.com header.b="dD4Y4HTj" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727552AbfBYPMw (ORCPT ); Mon, 25 Feb 2019 10:12:52 -0500 Received: from mail-eopbgr60056.outbound.protection.outlook.com ([40.107.6.56]:15184 "EHLO EUR04-DB3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727480AbfBYPMw (ORCPT ); Mon, 25 Feb 2019 10:12:52 -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=bNug80dURB3R2v1I79OHLcUFFBLUPwS6J7yLEdvk8aE=; b=dD4Y4HTjNeH9KA9aeh5Iuq2oYIvEbb7qjoKi0q0O+GGqdV4+iEUU4xmaRbeW6qHS40COsDTSiJB5fNwjcl0axyN8SYIeMsJnh3NCV00WQU+NpU/w9Km3u1H4aINa32om0T0ECYtJulQVbDMdrofSRpUF1gX4qi+OtGS9vNrKOv0= Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com (10.167.127.11) by HE1PR0502MB3899.eurprd05.prod.outlook.com (10.167.143.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1643.18; Mon, 25 Feb 2019 15:12:33 +0000 Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com ([fe80::b03d:8cd4:d259:f749]) by HE1PR0502MB3641.eurprd05.prod.outlook.com ([fe80::b03d:8cd4:d259:f749%5]) with mapi id 15.20.1643.019; Mon, 25 Feb 2019 15:12:33 +0000 From: Vlad Buslov To: Davide Caratti CC: "netdev@vger.kernel.org" , "jhs@mojatatu.com" , "xiyou.wangcong@gmail.com" , "jiri@resnulli.us" , "davem@davemloft.net" , "wenxu@ucloud.cn" , Roi Dayan Subject: Re: [PATCH net-next] net: sched: act_tunnel_key: fix metadata handling Thread-Topic: [PATCH net-next] net: sched: act_tunnel_key: fix metadata handling Thread-Index: AQHUzQSzh4GRGD0bik2go/1EPUhG0qXwjACAgAAS/YA= Date: Mon, 25 Feb 2019 15:12:33 +0000 Message-ID: References: <20190225122122.8128-1-vladbu@mellanox.com> <4bde1d403d4ba9b51cf18bbaac1d46147011b959.camel@redhat.com> In-Reply-To: <4bde1d403d4ba9b51cf18bbaac1d46147011b959.camel@redhat.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: LO2P265CA0102.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:c::18) 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: dce17b62-5ef9-4ee5-9c77-08d69b33aaf2 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:HE1PR0502MB3899; x-ms-traffictypediagnostic: HE1PR0502MB3899: x-microsoft-exchange-diagnostics: =?iso-8859-1?Q?1;HE1PR0502MB3899;23:wYtK/4BM5q/tzCX/Kp/sZ/aXha7QdIqZqHLuq?= =?iso-8859-1?Q?BAy7MyCbtEwwKxMrQ4evVdZwXC3SAJW+5Or+fST+koJxJDCx7gCXie5g3I?= =?iso-8859-1?Q?lUogb6TgEhfg1WXtWxU3/kGL4k/OXADDx/pF+DyYafh5AS4mm9/r2lFlBj?= =?iso-8859-1?Q?RJX8HVNOGV6zAieegCeIaizJExAwCb2T1ggkBS2qQZy9yWlGOJu1ZMBfW2?= =?iso-8859-1?Q?Y0nBy+DFb6JSz1VurftjDD8//yfbFaOpw0yZHzPFZFtNjfQi/mxcrqckpO?= =?iso-8859-1?Q?iTYW16c508Z+DrDoKo8eTUseYVyxON8NyHcMzSSrpU4LWmqUOCDhzRLZy5?= =?iso-8859-1?Q?JKpOsbjliDLJJCrQsIef0N1ii4fZtplG820mBh4ICjDy6J2onqbImbGEI8?= =?iso-8859-1?Q?IxIPkC5mo3a/X7w/h6err9W26zPXEPNYqNpHIzGzqOwwc6N9WAu4rQnfQS?= =?iso-8859-1?Q?Jlsxy9n1Us6SWz8R5wEINjtHxCAk2sMoBHwNOKiwpa9Wdvr9UaeofgkdwC?= =?iso-8859-1?Q?YP99MG6JYEsBHa+d/eHbk2JOSewSHeM16pTIMEgYWOdV32zTA4CAL3I4m9?= =?iso-8859-1?Q?SSlYIRGXiOHRLzPeshVOH1d02VWcjg3wf9XSLJowVWzRIhV6G/opXXepX7?= =?iso-8859-1?Q?1d2io0KGOVqlXEm9DjVdidjZ5maFVRH+m2lbAyw+U3ochRUmXU1X1/HOIy?= =?iso-8859-1?Q?PxCWslNiK7VbusH+fFNjxy0ItCZyjOaW38XjFPRLltYlyQDFKIZVJAOylN?= =?iso-8859-1?Q?J89XltuVfESRzlI/fLc9u6qGZESrgjcvHEYRNJKRTnZxnkQ++4McpN8JaK?= =?iso-8859-1?Q?3bZKnHkhhsyMz6EBpdl+UCP4vJh/mnVYbkWp37/FlmEINqtEd5kclWSugj?= =?iso-8859-1?Q?ULPYRv05jE09x2LfHHnthUHgz9cdXapvrOswXrN20lF92Dgb/WFNf/+X1w?= =?iso-8859-1?Q?Jk7fXMmx9GLWsH/RkEZt7ldvDL7EL2TBNu/ZDl+J2Tl9P8afxo1mDRIqRy?= =?iso-8859-1?Q?k0OGVB9xA/k/FFBYpd/bQ3M7QiytsqtyhzVQOvZ1wDRw5+6b36CRMaoXbP?= =?iso-8859-1?Q?II6cA939YGE3GVI6JBezGFwtGF5DGypz+Wt2Me/pgE7FaUBgWbCbJOOmkC?= =?iso-8859-1?Q?s99kjt0U7g2H5bB6Asj0krdRC4hzM5SvlVikFAB+zDmTrLEx1Bnd7xYAgQ?= =?iso-8859-1?Q?80LT0TjDifcZWQlty907r0oHsI/wS5bvWJCWWU4COFKE3GkwJsGj5dvmSJ?= =?iso-8859-1?Q?LbXMvJ3uHG2aIQze1Ifn07UlCvk2Tr746rLAMODoW2QbF/KW19rKeMdGg3?= =?iso-8859-1?Q?q2DI=3D?= x-microsoft-antispam-prvs: x-forefront-prvs: 095972DF2F x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(346002)(366004)(39860400002)(396003)(136003)(376002)(199004)(189003)(66066001)(4326008)(76176011)(68736007)(99286004)(6916009)(478600001)(256004)(386003)(6486002)(25786009)(8936002)(26005)(6506007)(14454004)(52116002)(36756003)(97736004)(186003)(229853002)(2906002)(102836004)(8676002)(81156014)(53936002)(11346002)(5660300002)(106356001)(81166006)(316002)(107886003)(6246003)(86362001)(6512007)(7736002)(305945005)(71190400001)(6116002)(14444005)(54906003)(6436002)(3846002)(486006)(105586002)(446003)(476003)(2616005)(71200400001);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0502MB3899;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: GM8bWUu9/8L+2GPmB3CYyLmf5X1neN1NCSkbhWH6kxCGb0EcffkxgjRzscdcK4QvgNKd9Yee1Df/vwwOptk+x+rF+h/G+9IL9F1nn56TgjIF6Xyg8/sYx9srz8TL60PaNfTNNmd/FMRNg7I7DYQhmRpewS2JxWZcgwGKQyyhpdL4iBhvXaVmhrgeLZRl3NK7iDu9lLWjI4uODE+cVHzG1f1+Mto34hvA4eeO6V92Fgzwb+uNz0VNzzoDLw/6fSoFo4DnOAjL6/XymrCv35Vkf4KNqSQLXJbGQVhsAz9dWm+WM8VGRJ+XY7P90EbPsecZxwuhMQpwwfPZ0mpN8KkbJPacw988eVH1zobY4Bog+sf6WzJmGnU9AIKWzzWI7knsAEManungMcrnPg4zJtGtgZTefjKdDcMRk0aJK3/I/7A= 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: dce17b62-5ef9-4ee5-9c77-08d69b33aaf2 X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Feb 2019 15:12:32.1219 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0502MB3899 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon 25 Feb 2019 at 14:04, Davide Caratti wrote: > On Mon, 2019-02-25 at 14:21 +0200, Vlad Buslov wrote: >> Tunnel key action params->tcft_enc_metadata is only set when action is >> TCA_TUNNEL_KEY_ACT_SET. However, metadata pointer is incorrectly >> dereferenced during tunnel key init and release without verifying that >> action is if correct type, which causes NULL pointer dereference. Metada= ta >> tunnel dst_cache is also leaked on action overwrite. >>=20 >> Fix metadata handling: >> - Verify that metadata pointer is not NULL before dereferencing it in >> tunnel_key_init error handling code. > > hello Vlad, > > thanks a lot for fixing this! > > <...> > =20 >> @@ -384,10 +390,12 @@ static int tunnel_key_init(struct net *net, struct= nlattr *nla, >> =20 >> release_dst_cache: >> #ifdef CONFIG_DST_CACHE >> - dst_cache_destroy(&metadata->u.tun_info.dst_cache); >> + if (metadata) >> + dst_cache_destroy(&metadata->u.tun_info.dst_cache); >> #endif >> release_tun_meta: >> - dst_release(&metadata->dst); >> + if (metadata) >> + dst_release(&metadata->dst); > > on Linux 'net' tree we don't have commit 41411e2fd6b8 ("net/sched: > act_tunnel_key: Add dst_cache support"), but still the above two lines ca= n > avoid a NULL dereference in tunnel_key_init() error path, in the followin= g > case: > > * create an action with tunnel "set", with success > * replace the previous rule rule with tunnel "unset", and have a failure= =20 > here (e.g. allocation of 'params_new'). > > At the cost of creating some conflicts during the merge, it would probabl= y > be safer to split this commit into two parts, one targeting 'net' and one > targeting 'net-next', so that the first one can be proposed for stable > backports (and also I can rebase/retest my 'goto chain' series on top of > it :) ) > > WDYT? Makes sense. I'll send split patches.