From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by dpdk.org (Postfix) with ESMTP id B13532B9C for ; Fri, 22 Feb 2019 05:47:05 +0100 (CET) Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x1M4jOsq014581; Thu, 21 Feb 2019 20:47:04 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : references : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pfpt0818; bh=7wWUpqJefAGzXv696xig3wd2VBj969xEe9SY4QFgSyg=; b=GL7BwbyvLV51vI6J2RPux1J9kxAoRFX66AcbJgG4EQ3lx+Bukc2ocyY87PII7nw4+FLO tXtTPGqgYy1oiLMvtOnwzjpn91SUOFwGkU31UDnuxf+rEndPnC/BQaHmm7fgzNnU4+YT r0qrBl619NM+RoYZDicv6gi02m1MYNajwWQfwSfaiOkHFNcfZCby4OrZrYvBBKEccPIm 0sN56vSHw7Jz2PM7gtsjdS27qJMQOt4GFjiYJc4f0uAj0/gHUDev0l8WAiNzQSps9MBU u5l0JgpSHUoZWhKcmXhJe0GcwzFKoJmXY6CjWVO7vTzXUqZ4idwALXPHNhjToFOH1IGh DQ== Received: from sc-exch01.marvell.com ([199.233.58.181]) by mx0a-0016f401.pphosted.com with ESMTP id 2qsxn3ssvw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Thu, 21 Feb 2019 20:47:04 -0800 Received: from SC-EXCH03.marvell.com (10.93.176.83) by SC-EXCH01.marvell.com (10.93.176.81) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Thu, 21 Feb 2019 20:47:03 -0800 Received: from NAM04-BN3-obe.outbound.protection.outlook.com (104.47.46.52) by SC-EXCH03.marvell.com (10.93.176.83) with Microsoft SMTP Server (TLS) id 15.0.1367.3 via Frontend Transport; Thu, 21 Feb 2019 20:47:03 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.onmicrosoft.com; s=selector1-marvell-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=7wWUpqJefAGzXv696xig3wd2VBj969xEe9SY4QFgSyg=; b=gQFXBpgRZQtt0173uuORhh2DugtuuBKLXbJ1eFbfXjS1Db1obxbGzUCRee0ysAuQLWcu16roh+aIKLWnC1s8NUKdmUpIn4MXDEr20mdQ9amgclmpZjMO0J1FYUlHoCDOjb9jCXVypFWCeejvmx+fXIXAnLsnCurQZvNgwwOZwG8= Received: from MN2PR18MB2877.namprd18.prod.outlook.com (20.179.20.218) by MN2PR18MB2638.namprd18.prod.outlook.com (20.179.84.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1643.15; Fri, 22 Feb 2019 04:47:01 +0000 Received: from MN2PR18MB2877.namprd18.prod.outlook.com ([fe80::1905:8cf6:f4a1:5d9a]) by MN2PR18MB2877.namprd18.prod.outlook.com ([fe80::1905:8cf6:f4a1:5d9a%4]) with mapi id 15.20.1643.016; Fri, 22 Feb 2019 04:47:01 +0000 From: Anoob Joseph To: "Trahe, Fiona" , Akhil Goyal , "Doherty, Declan" , "De Lara Guarch, Pablo" CC: Jerin Jacob Kollanukkaran , "Narayana Prasad Raju Athreya" , "dev@dpdk.org" , Ankur Dwivedi Thread-Topic: [PATCH] lib/cryptodev: fix driver name comparison Thread-Index: AQHUvHxsntNewzBh2Ey4taeN+7Qt9aXo7DZAgAGkSYCAAMGkYA== Date: Fri, 22 Feb 2019 04:47:00 +0000 Message-ID: References: <1549279528-10397-1-git-send-email-anoobj@marvell.com> <348A99DA5F5B7549AA880327E580B435896F431A@IRSMSX101.ger.corp.intel.com> In-Reply-To: <348A99DA5F5B7549AA880327E580B435896F431A@IRSMSX101.ger.corp.intel.com> Accept-Language: en-IN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [115.113.156.2] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: a751e69d-c7bb-479e-27b5-08d69880c8be x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(5600110)(711020)(4605104)(4534185)(7168020)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7153060)(7193020); SRVR:MN2PR18MB2638; x-ms-traffictypediagnostic: MN2PR18MB2638: x-microsoft-exchange-diagnostics: 1; MN2PR18MB2638; 20:AZEJQNCpU5nL8hyWSGnd88QZs/I7np49AgQK9Fvu8IFU0pxt30n/OIGQV7E6lgUcgsm54eaT/GqvvsgUsg7Y6kuSjADfKGbdJulnJALmaXUmvqFgpMI1n4E/J/RtEDewzKs6hchF0UyRO+KH01rvLKaQvjO3XL3JHTyeDbUuxAw= x-microsoft-antispam-prvs: x-forefront-prvs: 09565527D6 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39860400002)(396003)(376002)(366004)(346002)(136003)(199004)(13464003)(189003)(11346002)(446003)(486006)(53546011)(6506007)(9686003)(86362001)(97736004)(71200400001)(5024004)(71190400001)(3846002)(229853002)(14444005)(478600001)(186003)(476003)(74316002)(2906002)(256004)(55016002)(6116002)(81166006)(6436002)(26005)(316002)(106356001)(7696005)(8936002)(305945005)(25786009)(54906003)(14454004)(110136005)(105586002)(81156014)(55236004)(4326008)(8676002)(53936002)(76176011)(68736007)(7736002)(107886003)(66066001)(6246003)(5660300002)(33656002)(102836004)(99286004); DIR:OUT; SFP:1101; SCL:1; SRVR:MN2PR18MB2638; H:MN2PR18MB2877.namprd18.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: marvell.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: Lcaz64gin1yhB4P867owlZVj5vcHFG9pQdVElhQH3KgwSqcyjCLRy0MJrXLs+aoclzPRQbraTm9fMkeu9wM8ufvLr0P/u/BeJSHDfYl2aeAe2filyeNpB+Ua0vPdaUYZCaB/Cmt/1aCAXO8X1NVafJ5PwPjKuGc2RP8Z/ZmRkMXrL2tIBEQ3j21xOK5ULHvUg9MyaU33EyaqFwZatW5+zyu0Sq04gnMA2vmJCVNZ7FUrfe9jW80EEdbWJ4ulZb3ld4gr9rY0tpvLesy3BQVGTWPYyYP8blNYzSKl6KiBSeLosjGXbNAOSvZVPPmIjD9Yzshny+k6Yc1kVxU6PCRHivNaMuWlP2VE3tM5Tu6PBL0qEjVW9+Gm/x3PYWuvbT9i+s7ylxynQ/mIEXRpszFU+snm6UiOxZwQ8XlxkNxbjCE= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: a751e69d-c7bb-479e-27b5-08d69880c8be X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Feb 2019 04:47:00.9584 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR18MB2638 X-OriginatorOrg: marvell.com X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2019-02-22_04:, , signatures=0 X-Proofpoint-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1810050000 definitions=main-1902220031 Subject: Re: [dpdk-dev] [PATCH] lib/cryptodev: fix driver name comparison 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: , X-List-Received-Date: Fri, 22 Feb 2019 04:47:06 -0000 Hi Fiona, Please see inline. Thanks, Anoob > -----Original Message----- > From: Trahe, Fiona > Sent: Thursday, February 21, 2019 10:33 PM > To: Anoob Joseph ; Akhil Goyal > ; Doherty, Declan ; De > Lara Guarch, Pablo > Cc: Jerin Jacob Kollanukkaran ; Narayana Prasad Raju > Athreya ; dev@dpdk.org; Ankur Dwivedi > ; Trahe, Fiona > Subject: RE: [PATCH] lib/cryptodev: fix driver name comparison >=20 >=20 >=20 > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anoob Joseph > > Sent: Wednesday, February 20, 2019 3:42 PM > > To: Akhil Goyal ; Doherty, Declan > > ; De Lara Guarch, Pablo > > > > Cc: Jerin Jacob Kollanukkaran ; Narayana Prasad > > Raju Athreya ; dev@dpdk.org; Ankur Dwivedi > > > > Subject: Re: [dpdk-dev] [PATCH] lib/cryptodev: fix driver name > > comparison > > > > Hi Akhil, Declan, Pablo, > > > > Can you review this patch and share your thoughts? > > > > Thanks, > > Anoob > > > > > -----Original Message----- > > > From: Anoob Joseph > > > Sent: Monday, February 4, 2019 4:56 PM > > > To: Akhil Goyal ; Declan Doherty > > > ; Pablo de Lara > > > > > > Cc: Anoob Joseph ; Jerin Jacob Kollanukkaran > > > ; Narayana Prasad Raju Athreya > > > ; dev@dpdk.org; Ankur Dwivedi > > > > > > Subject: [PATCH] lib/cryptodev: fix driver name comparison > > > > > > The string compare to the length of driver name might give false > > > positives when there are drivers with similar names (one being the > subset of another). > > > > > > Following is such a naming which could result in false positive. > > > 1. crypto_driver > > > 2. crypto_driver1 > [Fiona] This patch changes compare for both driver and device names. > Update description to mention device names too. [Anoob] Will update that. =20 >=20 > > > When strncmp with len =3D strlen("crypto_driver") is done, it could > > > give a false positive when compared against "crypto_driver1". > > > > > > Fixes: d11b0f30df88 ("cryptodev: introduce API and framework for > > > crypto > > > devices") > > > > > > Signed-off-by: Ankur Dwivedi > > > Signed-off-by: Anoob Joseph > > > --- > > > lib/librte_cryptodev/rte_cryptodev.c | 11 ++++++----- > > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > > > diff --git a/lib/librte_cryptodev/rte_cryptodev.c > > > b/lib/librte_cryptodev/rte_cryptodev.c > > > index 7009735..b743c60 100644 > > > --- a/lib/librte_cryptodev/rte_cryptodev.c > > > +++ b/lib/librte_cryptodev/rte_cryptodev.c > > > @@ -510,7 +510,8 @@ rte_cryptodev_pmd_get_named_dev(const char > *name) > > > dev =3D &cryptodev_globals.devs[i]; > > > > > > if ((dev->attached =3D=3D RTE_CRYPTODEV_ATTACHED) && > > > - (strcmp(dev->data->name, name) =3D=3D 0)) > > > + (strncmp(dev->data->name, name, > > > + RTE_CRYPTODEV_NAME_MAX_LEN) > > > =3D=3D 0)) > > > return dev; > > > } > > > > > > @@ -542,8 +543,8 @@ rte_cryptodev_get_dev_id(const char *name) > > > return -1; > > > > > > for (i =3D 0; i < cryptodev_globals.nb_devs; i++) > > > - if ((strcmp(cryptodev_globals.devs[i].data->name, name) > > > - =3D=3D 0) && > > > + if ((strncmp(cryptodev_globals.devs[i].data->name, name, > > > + RTE_CRYPTODEV_NAME_MAX_LEN) =3D=3D 0) > && > [Fiona] Is this safe? The const passed to this may not be the full length= of > RTE_CRYPTODEV_NAME_MAX_LEN. Does this prototype need to specify > that a full length const filled with trailing zeros must be passed in? An= d if so is > this an ABI breakage? >=20 [Anoob] strcmp itself is not safe when we have buffers which are not NULL t= erminated. Strncmp will make sure the check won't exceed RTE_CRYPTODEV_NAME= _MAX_LEN.=20 >>From man page, "The strncmp() function is similar, except it only compares = the first (at most) n bytes of s1 and s2." The main issue here is the usage of strncmp with strlen(driver_name), as in= the below cases. Strlen will return string length, which doesn't include \= 0. strcmp is good enough to fix the issue. But usage of strcmp would assume= that the const is filled with trailing zero. IMO, none of these options ar= e really safe. So please advise on what would be the best solution here. I'= ll revise the patch accordingly. >=20 > > > (cryptodev_globals.devs[i].attached =3D=3D > > > > RTE_CRYPTODEV_ATTACHED)) > > > return i; > > > @@ -586,7 +587,7 @@ rte_cryptodev_devices_get(const char > > > *driver_name, uint8_t *devices, > > > > > > cmp =3D strncmp(devs[i].device->driver->name, > > > driver_name, > > > - strlen(driver_name)); > > > + RTE_CRYPTODEV_NAME_MAX_LEN); > [Fiona] Is this safe? Same comment as for device name. > Also assumes RTE_CRYPTODEV_NAME_MAX_LEN is same as > RTE_DEV_NAME_MAX_LEN. They're both 64 at the moment so not currently > an issue. >=20 [Anoob] Will fix this with v2. > > > > > > if (cmp =3D=3D 0) > > > devices[count++] =3D devs[i].data->dev_id; > @@ - > > > 1691,7 +1692,7 @@ rte_cryptodev_driver_id_get(const char *name) > > > > > > TAILQ_FOREACH(driver, &cryptodev_driver_list, next) { > > > driver_name =3D driver->driver->name; > > > - if (strncmp(driver_name, name, strlen(driver_name)) =3D=3D 0) > > > + if (strncmp(driver_name, name, > > > RTE_CRYPTODEV_NAME_MAX_LEN) =3D=3D 0) > > > return driver->id; > > > } > > > return -1; > > > -- > > > 2.7.4