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 09BD6A00C2; Thu, 23 Apr 2020 10:06:19 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id CD5451D447; Thu, 23 Apr 2020 10:06:17 +0200 (CEST) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id B36221D443 for ; Thu, 23 Apr 2020 10:06:16 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20200423080616euoutp02d32151e35a3e72025713f3918b7d7e34~IZFLyIofu2787927879euoutp029 for ; Thu, 23 Apr 2020 08:06:16 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20200423080616euoutp02d32151e35a3e72025713f3918b7d7e34~IZFLyIofu2787927879euoutp029 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1587629176; bh=mv8kelg+rd9VWVC8U00atMUF4pa45WNhTAPI9aF2Z3c=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=FyU1aWHwVhvCypXDh3UkE89+9jBORNkwh14WPwzYtkHtyrTzKDCZYD7R9sF4+uhP5 6zxNzVhjZyEmJs7hxZMbcnkn8/h9ns8NIseu1NUFA/iAyesd6W+gZqay0PyTcznzBq ofhIPIFsV25yuBEHnngK/ZsmTJb/cY5ZXG82JOLE= Received: from eusmges1new.samsung.com (unknown [203.254.199.242]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20200423080615eucas1p25772bd183c26ea781fb75c8449a10bb2~IZFLrtqGQ2957429574eucas1p2m; Thu, 23 Apr 2020 08:06:15 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges1new.samsung.com (EUCPMTA) with SMTP id 89.8C.61286.77C41AE5; Thu, 23 Apr 2020 09:06:15 +0100 (BST) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20200423080615eucas1p216b7e8e1936d216b0db392dac69b9ad6~IZFLT_pQH2336923369eucas1p2O; Thu, 23 Apr 2020 08:06:15 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20200423080615eusmtrp1c109dafb0929ca4e848df52b173e7a01~IZFLQMZkj2941329413eusmtrp14; Thu, 23 Apr 2020 08:06:15 +0000 (GMT) X-AuditID: cbfec7f2-ef1ff7000001ef66-85-5ea14c775a6a Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id A0.7D.07950.77C41AE5; Thu, 23 Apr 2020 09:06:15 +0100 (BST) Received: from [106.210.88.70] (unknown [106.210.88.70]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20200423080614eusmtip1cd00812dc53df6aee6149d296334f48f~IZFKpUAf31746917469eusmtip1d; Thu, 23 Apr 2020 08:06:14 +0000 (GMT) To: "Ananyev, Konstantin" , Anoob Joseph , "dev@dpdk.org" Cc: "akhil.goyal@nxp.com" , "Doherty, Declan" , "techboard@dpdk.org" From: Lukasz Wojciechowski Message-ID: <76d67fd8-d022-f07c-1709-bf8d2e1f05b4@partner.samsung.com> Date: Thu, 23 Apr 2020 10:06:13 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Transfer-Encoding: 8bit Content-Language: pl X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrIKsWRmVeSWpSXmKPExsWy7djP87rlPgvjDFbPsbBYf2Yeo8WyLVuZ LN48aGKxePdpO5PF+z+LWCy6Ljxkd2Dz+LVgKavH4j0vmTwmL7zI7LHx3Q6mAJYoLpuU1JzM stQifbsEroz5/2YxFyxUrjh22qyB8bBsFyMnh4SAicTU6x/Yuxi5OIQEVjBKHOnZxwbhfGGU OHT0ESOE85lR4t/NDhaYlqX3X7FAJJYzSnx99A3KecsocfD0ImaQKmGBMImz81sZQWwRgRqJ h/duM4MUMQt0MUq8XniGHSTBJmArcWTmV1YQm1fATWLayhVMIDaLgKrE7kmvgQ7h4BAViJWY fi0EokRQ4uTMJ2BXcAKF269fAmtlFpCXaN46mxnCFpG48agF7GwJgXXsEidmPGUGmSMh4CJx Y0UAxAfCEq+Ob2GHsGUkTk/uYYGo38YocfX3T6jm/YwS13tXQFVZSxz+9xvsIGYBTYn1u/Qh wo4SD9eDQg9kPp/EjbeCEDfwSUzaNh1qLa9ER5sQRLWexNOeqYwwa/+sfcIygVFpFpLPZiH5 ZhaSb2Yh7F3AyLKKUTy1tDg3PbXYMC+1XK84Mbe4NC9dLzk/dxMjMOGc/nf80w7Gr5eSDjEK cDAq8fBGKC6IE2JNLCuuzD3EKMHBrCTCu+HhvDgh3pTEyqrUovz4otKc1OJDjNIcLErivMaL XsYKCaQnlqRmp6YWpBbBZJk4OKUaGE333959KKSEYZOu6g82Wd87f1KXv1JqXzfpjGXvxAOi fDpVbD7lF0yOzv/15Nv32hXFTJ3+fC5zrEXu1bXYnp8Sxrw48v29RVbzrrN+3bA9+cSpf1l7 Fz3b0Lb4wINbV1meSBSYTZU84su0ae/2Vv+3RwTONG74o7A+5BPTB6dYnYvWvelR52WVWIoz Eg21mIuKEwEPgrO2NAMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrEIsWRmVeSWpSXmKPExsVy+t/xu7rlPgvjDC7PkrJYf2Yeo8WyLVuZ LN48aGKxePdpO5PF+z+LWCy6Ljxkd2Dz+LVgKavH4j0vmTwmL7zI7LHx3Q6mAJYoPZui/NKS VIWM/OISW6VoQwsjPUNLCz0jE0s9Q2PzWCsjUyV9O5uU1JzMstQifbsEvYz5/2YxFyxUrjh2 2qyB8bBsFyMnh4SAicTS+69Yuhi5OIQEljJKbL95h7WLkQMoISPx4ZIARI2wxJ9rXWwgtpDA a0aJr4fUQGxhgTCJs/NbGUFsEYEaiStrZ7KCzGEW6GGUOL3gOxvE0N1MEm1zXjGDVLEJ2Eoc mfmVFcTmFXCTmLZyBROIzSKgKrF70ms2kMWiArESLRc1IUoEJU7OfMICYnMChduvXwJrZRYw k5i3+SEzhC0v0bx1NpQtInHjUQvjBEahWUjaZyFpmYWkZRaSlgWMLKsYRVJLi3PTc4uN9IoT c4tL89L1kvNzNzEC42vbsZ9bdjB2vQs+xCjAwajEwxuhuCBOiDWxrLgy9xCjBAezkgjvhofz 4oR4UxIrq1KL8uOLSnNSiw8xmgL9NpFZSjQ5Hxj7eSXxhqaG5haWhubG5sZmFkrivB0CB2OE BNITS1KzU1MLUotg+pg4OKUaGEXdT5VXKhxnnbxu5/kPM75u/GP2NuVEmP+NA2+i7SVkbzyZ 6s1jZGd5MuxA3fq7Ba2Z37cu/3p+beOaYouaL2fOW3c1GDzq1/cuvxF680a2f1doy/0tk/bp 86YI8m4OerXtzOeEzudXxN+cNXlf4ch1Xqnwm+OTXovDP5LeTfm6/hV/vYtQEK8SS3FGoqEW c1FxIgDqF7XHxQIAAA== X-CMS-MailID: 20200423080615eucas1p216b7e8e1936d216b0db392dac69b9ad6 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20200423075453eucas1p1953419e5b7d84a024e8e5e7d48938d4a X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20200423075453eucas1p1953419e5b7d84a024e8e5e7d48938d4a References: <20200422235158.24497-1-konstantin.ananyev@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" W dniu 23.04.2020 o 09:54, Ananyev, Konstantin pisze: >> Hi Konstantin, >> >> These are data path ops and so it will be better if we can avoid such checks in the datapath. The same is done in ethdev also. > AFAIK, get_userdata is an *optional* dev-ops function that can be used by data-path. > So far there was no strict requirement for the rte_security PMDs to *always* implement it. > So what you guys did is a silent change of public API behaviour. > As result ixgbe, (and probably some others rte_security PMDs) stopped working properly. > I don't see any point in these changes, but if you'd like to do that, at > least our usual procedure has to be followed: > 1. Send and RFC to get an agreement with rte_security PMDs maintainers (one release ahead) > 2. send a deprecation note (one release ahead) > 3. change the behaviour of the public API > 4. update release notes > > AFAIK 1), 2), 4) wasn't done. > So I think right now we need to revert original behaviour. The current changes were made in patch: b6ee9854784 security: fix verification of parameters @@ -91,7 +119,9 @@ rte_security_get_userdata(struct rte_security_ctx *instance, uint64_t md)  {         void *userdata = NULL; - RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->get_userdata, NULL); +#ifdef RTE_DEBUG +       RTE_PTR_CHAIN3_OR_ERR_RET(instance, ops, get_userdata, NULL, NULL); +#endif         if (instance->ops->get_userdata(instance->device, md, &userdata))                 return NULL;   So as you can see, the checks were already there. They've just been wrapped up with RTE_DEBUG macro for disabling them in non-debug compilation mode and the validation of paramter was change to avoid possible segmentation fault if instance lub ops would be NULL >> https://protect2.fireeye.com/url?k=e0478418-bdd92a82-e0460f57-0cc47a336fae-55cc35a7b94c97c0&q=1&u=http%3A%2F%2Fcode.dpdk.org%2Fdpdk%2Fv20.02%2Fsource%2Flib%2Flibrte_ethdev%2Frte_ethdev.h%23L4372 >> >> Datapath functions in cryptodev (enqueue/dequeue) doesn't even have such checks. >> https://protect2.fireeye.com/url?k=79d7974a-244939d0-79d61c05-0cc47a336fae-19f540008a9467cf&q=1&u=http%3A%2F%2Fcode.dpdk.org%2Fdpdk%2Fv20.02%2Fsource%2Flib%2Flibrte_cryptodev%2Frte_cryptodev.h%23L962 > That's a different story: > rx_burst/tx_burst, enqueue/dequeue are mandatory dev-ops functions that > have to be implemented by each ethdev/cryptodev API. > >> >> Thanks, >> Anoob >> >>> -----Original Message----- >>> From: dev On Behalf Of Konstantin Ananyev >>> Sent: Thursday, April 23, 2020 5:22 AM >>> To: dev@dpdk.org >>> Cc: akhil.goyal@nxp.com; declan.doherty@intel.com; Konstantin Ananyev >>> >>> Subject: [dpdk-dev] [PATCH] security: fix crash at accessing non-implemented >>> ops >>> >>> Valid checks for optional function pointers inside dev-ops were disabled by >>> undefined macro. >>> >>> Fixes: b6ee98547847 ("security: fix verification of parameters") >>> >>> Signed-off-by: Konstantin Ananyev >>> --- >>> lib/librte_security/rte_security.c | 4 ---- >>> 1 file changed, 4 deletions(-) >>> >>> diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c >>> index d475b0977..b65430ce2 100644 >>> --- a/lib/librte_security/rte_security.c >>> +++ b/lib/librte_security/rte_security.c >>> @@ -107,11 +107,9 @@ rte_security_set_pkt_metadata(struct rte_security_ctx >>> *instance, >>> struct rte_security_session *sess, >>> struct rte_mbuf *m, void *params) { -#ifdef >>> RTE_DEBUG >>> RTE_PTR_CHAIN3_OR_ERR_RET(instance, ops, set_pkt_metadata, - >>> EINVAL, >>> -ENOTSUP); >>> RTE_PTR_OR_ERR_RET(sess, -EINVAL); >>> -#endif >>> return instance->ops->set_pkt_metadata(instance->device, >>> sess, m, params); >>> } >>> @@ -121,9 +119,7 @@ rte_security_get_userdata(struct rte_security_ctx >>> *instance, uint64_t md) { >>> void *userdata = NULL; >>> >>> -#ifdef RTE_DEBUG >>> RTE_PTR_CHAIN3_OR_ERR_RET(instance, ops, get_userdata, NULL, >>> NULL); -#endif >>> if (instance->ops->get_userdata(instance->device, md, &userdata)) >>> return NULL; >>> >>> -- >>> 2.17.1 > -- Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciechow@partner.samsung.com