From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 16CD2A00C4; Thu, 23 Apr 2020 15:48:02 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E61711D177; Thu, 23 Apr 2020 15:48:01 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 92F481D16F for ; Thu, 23 Apr 2020 15:48:00 +0200 (CEST) IronPort-SDR: W+Vw281y0SD/jm2k30igkI92x3uj/hTNj/AAGZLr/5j5Tw09LmtnYwb45g63A/TfJQUYMFIbav DeM4/TwtNt7g== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Apr 2020 06:47:59 -0700 IronPort-SDR: SGY5xJgzJzCfFNA6aRM3nejSo8WHb7RoIArr2SoWBzMBICKr9ZErEenLBGThld9lVZlPKRysMh TLnrDeEBxFlg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,307,1583222400"; d="scan'208";a="292279135" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga008.jf.intel.com with ESMTP; 23 Apr 2020 06:47:59 -0700 Received: from fmsmsx117.amr.corp.intel.com (10.18.116.17) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 23 Apr 2020 06:47:31 -0700 Received: from FMSEDG001.ED.cps.intel.com (10.1.192.133) by fmsmsx117.amr.corp.intel.com (10.18.116.17) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 23 Apr 2020 06:47:31 -0700 Received: from NAM02-SN1-obe.outbound.protection.outlook.com (104.47.36.52) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 23 Apr 2020 06:47:31 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ELxwYGElmD1gxWkZi5MWF9fEj6NmpWtRldlRD3pZe09Rzflnbd/8y9C/wT5btW8gYiVTaWkZ9R90iaQ9MGipphULt/isW6KItYar/RIjkBizHjJlhx2GK9xHTQakNGAoM1VKa9K/BTnLY9i+3cHanfHYnnduWHfK3WL89ZJBdXyKVCgoIhkwOlHqo0sW101YGmQXZz/pWCgicd4wAjRlUOiOVWgo3D9v2rt+80th6xs+xSy5G8cA50LofUH7RJTsdMcUQMVquMyKpi7dpmeJd2GRxgyREAM3MearSlW9QryYILpPNXnkgN5HH4VbQvrR7RcrsRsmh6Q/9qHyCSGjFg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=i6rYQ9x37PdznC4VH497c/Xxq6ApI1mCXnjHB7zf+64=; b=G6YYb+KVbhoWBjFdtGxwdyFfZ3RPVqcY2D4XwYvsFXUzjwBT7WLUtb0Dts7RHssojHH4/A9+OHUdIKJYXgLJJux5Rfq6FtTzEVdpytdbQTrCp31pRsWGogJl7DXq3r/l2AXuoKHmdRoILKfm6FETysudiU2Bj1V5HWebJO+FOOnhVd9Dv0DZGmqLI450UOXsniS6ZV07xmxGGQIq+c5wEoQm3ZJZyKVjnLWuDqdtxSV/z6jEUmrf93G4EeNnIw0D8w1QEQOgEnJGSV7jz1WSJI9dO6axx9ILs8fRricKix6nFESJQ5HUscaz95nh5TkJ8upglGaIgRY/nSJCw053dA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=i6rYQ9x37PdznC4VH497c/Xxq6ApI1mCXnjHB7zf+64=; b=ok5c6/hDzY67SPfXbblKKlXOi0xEobikKC+C+e1lbUPKzVyeerZgCytsQOq0fsWDF+PMf8oHuhr/LjMBcTvzM48jyvdsqIycOFs6GdxX5+anOpBpLepMu3MrRAkIUXAC3R/rRATKPWM6ztb/L0zJg0+EF5oJgMvK6zhIsSbfmIY= Received: from BYAPR11MB3301.namprd11.prod.outlook.com (2603:10b6:a03:7f::26) by BYAPR11MB3158.namprd11.prod.outlook.com (2603:10b6:a03:1c::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2921.29; Thu, 23 Apr 2020 13:47:29 +0000 Received: from BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f8cb:58cd:e958:fff4]) by BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f8cb:58cd:e958:fff4%6]) with mapi id 15.20.2937.020; Thu, 23 Apr 2020 13:47:29 +0000 From: "Ananyev, Konstantin" To: Akhil Goyal , Anoob Joseph , "dev@dpdk.org" , Lukasz Wojciechowski CC: "Doherty, Declan" Thread-Topic: [dpdk-dev] [PATCH] security: fix crash at accessing non-implemented ops Thread-Index: AQHWGSTLO6APWAgcfEqbxThOaNZf1KiGUA6ggAAbjICAABhdIIAADRIAgAAZmYCAAAmjIA== Date: Thu, 23 Apr 2020 13:47:29 +0000 Message-ID: References: <20200422235158.24497-1-konstantin.ananyev@intel.com> In-Reply-To: Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.2.0.6 authentication-results: spf=none (sender IP is ) smtp.mailfrom=konstantin.ananyev@intel.com; x-originating-ip: [192.198.151.169] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: cdc8ed2c-b7e3-40a6-9a86-08d7e78cdda7 x-ms-traffictypediagnostic: BYAPR11MB3158: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 03827AF76E x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB3301.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10019020)(396003)(39860400002)(136003)(346002)(376002)(366004)(2906002)(55016002)(478600001)(15650500001)(9686003)(52536014)(66946007)(76116006)(33656002)(186003)(66556008)(66476007)(81156014)(66446008)(6506007)(316002)(7696005)(110136005)(5660300002)(86362001)(8936002)(4326008)(71200400001)(107886003)(64756008)(26005); DIR:OUT; SFP:1102; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: wOyUC72jyAecu4rvfGe+hzQ/L/1//7AJqLBqBI49MddNz968vDM8SzDVwBB1xmgopruqRvvGceriJ7d5v2ozRJ4xUKWhXEVfl6ngwgmk2W5l/hUzoimDjw3H+I8HwzbCgJshSHymZi3pLycwLu8aTk3N8w0fUVCp1sDKn5d6myJ2epHjuAzkBnE/caeTI75D4wKEnaVjyGB5QAbY0f3NyxFEwztOYIT8j7+MbNS9y4w3XYdh1pn972Nms97Ln1QOoHj5uzo/Ixw13TjDoatB2dh7Z3Bph/k8Mkkg8uBHu3YH8N9FVjzGRguk7ec7cRxmSupM4fiN4259j1krLLP2KItQ2VDADO6uOAGPhtG2yWze4I6hR6nLo4/qR/muniRP1qotWyrK0qVtFhiGVf9l50ocQQM09CYrlp3gFjP+n1585MXH2cA5S3oPZLzqWyWR x-ms-exchange-antispam-messagedata: aIOoEKjhhR65XfEdP/BT0p5E+ZHFWgd+oPDUIuuAHnrhbW4+/8TgqYIEguMpjAWuDA/ZZNr+qSE8w1XzMfaa6ZKfWlx0LQV5zAOvgLn/6ak3slsW4S4RkY68U80vK7GQSrF7CUSsZmSiYi281MvRBg== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: cdc8ed2c-b7e3-40a6-9a86-08d7e78cdda7 X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Apr 2020 13:47:29.5281 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: WUzjv16vzGLCB/FHySPriqBjdyKFZVmaPdez9Q0ZTfrphV++OYGNrDrfnUdAgz3GYo5dRB+Ovdlcgu+crKmMGaUn42AXCObqfKiPfTkAyjQ= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3158 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH] security: fix crash at accessing non-implemented ops X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Akhil, >=20 > Hi Anoob/Konstantin, > > > > > > Check that ops->get_userdata is a valid function pointer will be comp= iled out. > > > So PMDs that don't implement this function will crash in > > > rte_security_get_userdata(). > > > In our particular case - ixgbe. > > > Same story with rte_security_set_pkt_metadata() - see the patch. > > > > [Anoob] But ixgbe doesn't implement inline protocol which is the primar= y > > consumer of this API (rte_security_get_userdata()). So what is the trou= ble? > > > > Also, application is expected to call rte_security_set_pkt_metadata() o= nly on > > devices with offload flag RTE_SECURITY_TX_OLOAD_NEED_MDATA. If a PMD > > states it needs MDATA but fails to register a function pointer for doin= g the same, > > it is a control path problem. Checking for that in the datapath is an o= verkill. > > > Whatever your concern is, we can resolve it later, but for now we should = have the same > Unconditional checks that were there earlier. We need to make RC1 today/t= omorrow. > And this cannot go as an issue. >=20 > These are optional APIs and every PMD may not have supported that. >=20 > Konstantin, > Please send an update to your patch reverting the original patch for thes= e 2 functions. > Currently it is adding 2 extra checks. >=20 I am afraid we can't do just that. As in that case /app/test/test_security.c build wih -DRE_DEBUG will start c= rashing. I think we have 3 alternative how to fix it: 1. Keep all these 3 checks for debug and non-debug mode (that what my curre= nt patch does). 2. Have both: existed 1 check in non-debug mode, plus new checks in debug m= ode, i.e.: rte_security_get_userdata(struct rte_security_ctx *instance, uint64_t md) { void *userdata =3D NULL; =20 +#ifdef RTE_DEBUG + RTE_PTR_CHAIN3_OR_ERR_RET(instance, ops, get_userdata, NULL, NULL); +#else RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->get_userdata, NULL); +#endif .... 3. Keep only 1 existed check in non-debug mode and remove cases in app/test= /test_security.c that would crash with -DRTE_DEBUG.=09 My preference is 1), I don't think these 2 extra checks will affect perform= ance greatly. Also with 1) we can make these new test-case to be executed for non-debug m= ode too. 2) is probably also ok - but I think RTE_DEBUG concept should be a separate= patch series, and I don't want to mix things. What is your opinion here? Konstantin