From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 4E1BA432F3;
Fri, 10 Nov 2023 18:41:50 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id CA4704064C;
Fri, 10 Nov 2023 18:41:49 +0100 (CET)
Received: from NAM12-DM6-obe.outbound.protection.outlook.com
(mail-dm6nam12on2061.outbound.protection.outlook.com [40.107.243.61])
by mails.dpdk.org (Postfix) with ESMTP id 13512402EA
for <dev@dpdk.org>; Fri, 10 Nov 2023 18:41:49 +0100 (CET)
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
b=cFYh6s9gF0Gda7f3TExschDeQNKHQySiWnz1WZYSRJ8SyhSO5bihmMg5ZwiWX+u6J4P9LgjEbZuRZpBQ4PBCleHnCNI3CKk3gf9BpMMpPjUZuvwrZIcYkID9PHs29XPlBgQ5If3LOGrcJsUCClz3/G2VsSXRZvLP0lqfpTWt2jf+b8Tspc7jWHv0ZlXx5n3jVLF1A7v9SZVkp/2SGvqY+VP+aRs7+jpgKnQvjwX6uCJtSZgd9Wh4tyjM1kcQf9n4veJGvKniOIWfHfrRxzqba8nQNswtshJXtocEVfb8ACjF91sV9hOHseOgbmUR+7pFIuDCj9+8IEiNWOQdyVperA==
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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;
bh=3l7dVTlnzz7GnLNk0nc/fo+RXnGVaaCib1qU2MFy6VQ=;
b=CvAi9dT0k7rHUlZhNHnKfAnHVC6kaA6kDj0RTO3EsFYIy6bXULOlxSLB/ApW1csUzj6jRXP5wgEUSj6qHVH2kG+i6pR5hAQ+S4xUAs0nTUsdhCYoG5KqfbuAciC2g2AhWm70KqTP09yAC5/4ppf3V/W8z5ByN2HC8M7wKvmCQgJOE1PJZLl7w4FLKMToLTzQ6uSEtJSQRLIRkySYq8I+LpVtJtFoeu8mAglH0Wwi4QzR61mCaOyLSR9EFKaUE4Nl6oONBf+tIkowUflKbOeTRHGfPb7f+vY/tP319io8XWRxLgjbwGpPGsB3FyFYqCADr5fWoea8RiOw84u6evkXjw==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com;
dkim=pass header.d=nvidia.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com;
s=selector2;
h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
bh=3l7dVTlnzz7GnLNk0nc/fo+RXnGVaaCib1qU2MFy6VQ=;
b=WSajuZ321eqPussrE8zLJiQEAtOvWeBQAd48QnO+jdSZkpn51enQGBapFtuZH1K69zLwf9LFMvfRkqDsRGreDFnmqDLdwk/6XoZQR5DDdYSAn/a3ZIa9jeREYja/279hi7bXY264RwMn49Geu8AAdnfWRcmthmo435lgPxOpHZ+JUfw6qTnFPyk4WqOs2aOx81YVUig9EM0sFp1kIc89/e+Plz0mxMK+O9rMWrYU9T5OwxVA0DGcwXt1xmA7EfXST+qehCbRn4alNb6iMwwBIGmI9cN5cCXq4jRPVAiq8jTW20XsDIOR+0oXSCepw8jvwp+4X24qqj87bc5O/+ZVcw==
Authentication-Results: dkim=none (message not signed)
header.d=none;dmarc=none action=none header.from=nvidia.com;
Received: from IA1PR12MB6332.namprd12.prod.outlook.com (2603:10b6:208:3e2::13)
by MN2PR12MB4424.namprd12.prod.outlook.com (2603:10b6:208:26a::7)
with Microsoft SMTP Server (version=TLS1_2,
cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6977.21; Fri, 10 Nov
2023 17:41:43 +0000
Received: from IA1PR12MB6332.namprd12.prod.outlook.com
([fe80::43ed:468d:3105:4071]) by IA1PR12MB6332.namprd12.prod.outlook.com
([fe80::43ed:468d:3105:4071%4]) with mapi id 15.20.6954.029; Fri, 10 Nov 2023
17:41:43 +0000
Date: Fri, 10 Nov 2023 19:41:39 +0200 (IST)
From: "Etelson, Gregory" <getelson@nvidia.com>
To: Ferruh Yigit <ferruh.yigit@amd.com>
cc: "Etelson, Gregory" <getelson@nvidia.com>, "dev@dpdk.org" <dev@dpdk.org>,
Maayan Kashani <mkashani@nvidia.com>,
Raslan Darawsheh <rasland@nvidia.com>, Ori Kam <orika@nvidia.com>,
Aman Singh <aman.deep.singh@intel.com>,
Yuying Zhang <yuying.zhang@intel.com>
Subject: Re: [PATCH] app/testpmd: fix indirect action list parameters parsing
In-Reply-To: <95a14c2f-f6ba-4133-b787-99c2feb8739c@amd.com>
Message-ID: <2ddf95cd-840d-81dc-1fff-99b3dbe59095@nvidia.com>
References: <20231108163445.422772-1-getelson@nvidia.com>
<bd8fad06-ebd7-491c-a0ef-9adc92ffaae9@amd.com>
<9c982259-df1f-bdce-11ff-554831abc42f@nvidia.com>
<01176d6f-7f4e-465e-b0e3-20dd6a7e6d18@amd.com>
<c1d05bd9-c97d-e229-8280-44f58baa037f@nvidia.com>
<95a14c2f-f6ba-4133-b787-99c2feb8739c@amd.com>
Content-Type: text/plain; charset=US-ASCII; format=flowed
X-ClientProxiedBy: LO4P302CA0032.GBRP302.PROD.OUTLOOK.COM
(2603:10a6:600:317::13) To IA1PR12MB6332.namprd12.prod.outlook.com
(2603:10b6:208:3e2::13)
MIME-Version: 1.0
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: IA1PR12MB6332:EE_|MN2PR12MB4424:EE_
X-MS-Office365-Filtering-Correlation-Id: 794a0f6e-6c96-4d38-dada-08dbe2144d72
X-MS-Exchange-SenderADCheck: 1
X-MS-Exchange-AntiSpam-Relay: 0
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: 6Z4dVnYqzAY/wuIKyth97fPlBm6U1qbF8lKXxZdMoDRTnJYFI8rmpUlKZ9UlRUDGAn/mft0QNOGH8fhhb20Aqww0Z8W+1ZZtn/ga360gUffeoG73BPW4X2doWUwVrmJQjEstQ0uyS2LIHp3gbMulvvf+VsJDj/vY4qiEkr4CXZnkNLaY6nZfikIr3QxMSPY1u71V1X6653LIW2B1Xc7WbzDTTnvSuWZ+Z9IRzSme5G1my2qbVc/vb/Euz9/9YK0Xe6J4ycRFilhI0VMmwMOQJQ6BOrJBWY+e/KJNkQhuCazZ/ZWrdwNRkPdNmPT96XxZKDBscS5fAT6PdhFUeMBFM7WC71UmnbIg0d9c9KyF6GOPfhaXLwi1OsgX0jJh2D3O9GftLyAyiNgPJ5ggVwGvLctzSYJQTJhxVJSNilgPm4giX5ScaS7DlEkgRpujHyB7ij/YhzwBV6a+iTyU3WZJun2KMCmyAp1UMCCS7N95SKqfqv+VSYR0c2uDXWRIfV31LVTdY3RPEcioyaH6Pd1NCNOS4K3DZa+7hHgB+yfj+KZREYdFo5TexUjgxPBAo9WtWc7nnSMzTKJ3oxFUKGAvgsBcdxvVR56AawKLrI5Nyz9B4d83GQ45BLUrcpuWPERY
X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;
IPV:NLI; SFV:NSPM; H:IA1PR12MB6332.namprd12.prod.outlook.com; PTR:; CAT:NONE;
SFS:(13230031)(376002)(39860400002)(396003)(346002)(366004)(136003)(230922051799003)(451199024)(1800799009)(64100799003)(186009)(2906002)(83380400001)(6666004)(6486002)(86362001)(31696002)(38100700002)(478600001)(316002)(6916009)(41300700001)(36756003)(54906003)(66476007)(66556008)(66946007)(4326008)(8936002)(8676002)(2616005)(6512007)(31686004)(6506007)(5660300002)(45980500001);
DIR:OUT; SFP:1101;
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?T4Ci6cVz8ILssUYdU5tspOpHupLpGQP67x9DcdYmRuTn+ctXQF/z2oMokyoO?=
=?us-ascii?Q?pft+ACRl/wzddI3lX8CUeNtilGeqO9lJ+hgtcCxUav86bB6e+dUKW8DZpZMS?=
=?us-ascii?Q?c5zXsLFMWDwD4rSKM6xqVFCXLQWODxkxGA5JEbN0bY5Ox2ZnnFHB/W2b701m?=
=?us-ascii?Q?xtCJk+hD+W2XzbVz3sr5q1A2tJ/unE9D3W59GnfjlENyeV/u7lPqBUIJFPRI?=
=?us-ascii?Q?5oUxRdCsOiyjCtmB30u2yCInXu53YY0Eynf7oJGPfkWX7G5gIPqeiGvnPm4j?=
=?us-ascii?Q?bZpmMU6YShlNiS7lWm6/qPwDKwSYU70h0s+k3JgmyHcHWYIRs1Kqtb/khgz2?=
=?us-ascii?Q?ugjCcCE9xtmawZSd3FpvlOjPgXTfWxHtqdQJRuucggbtqHaMnvXfyC1OTSps?=
=?us-ascii?Q?btihBiK/k/SQX1Ismn7t7ibTot2rG0dO6Gd8TwkYtInt3hZm3mwmmnddbJpN?=
=?us-ascii?Q?zzD4rpO3qbg50y4akAeGcFm5ekkEJNBdzmckKKgjehqk1XnCP5BvKgMn55K4?=
=?us-ascii?Q?/zFjb1A98+bASigp+agjygNsVUUxeVnn7NqjgW6ZTM/Ah6zL9X6HYcw+vHV1?=
=?us-ascii?Q?+eGKuN56l7RDudri2NywbuQMM9oKS2SL/aR5fRzFHwjNcBIr06dARJhb7PWz?=
=?us-ascii?Q?7WxNzrce93OEdPju5vwRXjztDFGRR/n+EqBiS+UE2RIIK48/WNAu1aL8aOlO?=
=?us-ascii?Q?HybsMNVSSYU6+/0sJ9ewy+LtowqanNTRI0qGONBf8nBkMJzMZk2jNY33kidp?=
=?us-ascii?Q?eQK0RwVPqjk5xJ3t2P/aNJk2lfYuxXyI51DaFrU/oRCDD2wiFMiIWbAR0bx2?=
=?us-ascii?Q?BzzpIopzPh+YaiR4X9/dvSPVmVhkxUpNKKB1lW3b8foWW3U9uMJgC67dR/Zm?=
=?us-ascii?Q?QfsCYTkW+9/uqookr8wbYxmGAdvacKj+/Dbx1qGdKX1XcEEG8/lUpkfLucXc?=
=?us-ascii?Q?COD/wQDU+Q1HoZuSG7mNvmlNjHXHC94oJ8BwrwDMa942Lmxh3DIdASUK4ZxF?=
=?us-ascii?Q?cMjs+AEZF362zBS/tu9NlxuJKTXmCFg397d18BnwM/NSjHTdf1dpFwaTo5O+?=
=?us-ascii?Q?a9qJFrrisD54lEujbA1Um9oWYLUc8hVcLdjLuxYD2XbxEI7ubHXgRWp1a3xb?=
=?us-ascii?Q?MZqgtVpaJbBzS6lASLQo5yNnOqH+/lpIvj5rgu/ldzSbQvg/+raUz4D9p1JV?=
=?us-ascii?Q?Q7s+yKl779ZqoY8zq+hQO3wfNh1BJzo11ahtvtYXRdd4I6aCS3P8WF0tZ6ar?=
=?us-ascii?Q?D6t9pWFGpAOXOQxIsFBawHreY4Y32YIorowQZW1SY0+qbtU8LdC7SIT+L98p?=
=?us-ascii?Q?3b4ppFHv2uKV5Q5qjEeJHmRvqJiVbAExEaXDK1SxQniGWXaYbcnIkgPbfU2K?=
=?us-ascii?Q?xecLxB6aCF8+Zj1/xGgI0/Q983+eITcTMwYcc4eCVwZkjrXkalID6oH+a+iK?=
=?us-ascii?Q?wALjkiwheLHT85ZrGWD3TITjotz3b1Ps3AFJBlepBXYuqJQqqnodA46WqNYr?=
=?us-ascii?Q?4EPczplhCkewjweIQmyZO6DqFcxobNLt7j5rY+wST3cq0mpG6WmxwP18xZbW?=
=?us-ascii?Q?+U/ITPgAPCdDibNzwzwD+eGyIjJo2KbNiHelB4uh300Ouo+0rJIlac/7IFY8?=
=?us-ascii?Q?mdFLTruOuDsUn8WgC6EfisqeqbeieK3yHaEuesm+ZTCz?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 794a0f6e-6c96-4d38-dada-08dbe2144d72
X-MS-Exchange-CrossTenant-AuthSource: IA1PR12MB6332.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Nov 2023 17:41:43.1890 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: JODWW8uyfBIIPJjuDkn2ebGxzB7Ys6ocJscM5rnNpTgsPBdSUJT842+BJX90IQcei/28jeOd9B0Vu5lmP7KO7A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4424
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Hello Ferruh,
[:snip:]
>> *** stack smashing detected ***: terminated
>>
>> The corruption occurred in `parse_int()` called from
>> `parse_indlst_id2ptr()`.
>>
>> Inside `parse_int()` the arg parameter referenced 8 bytes of memory
>> while the target buffer was 4 bytes allocated on caller optimized stack:
>>
>> (gdb) p *arg
>> $1 = { ... size = 8, ...}
>>
>
> Thanks Gregory, I can see the problem now.
>
[: Thumbs up :]
[:snip:]
> BUT back to the root cause of the problem,
> `parse_int()` tries to be generic and it support different size of
> variables [1], but it fails on this.
>
> `parse_int()` gets 'size' as argument, but it doesn't use parameter
> value, instead overwrites it with 'size = arg->size;' and uses this
> value, in this case when context provides larger variable size than what
> `parse_int()` gets as parameter, the problem you observed occurs.
>
> What do you think to use 'size' from parameter list, as it is intended,
> instead of using 'arg->size'?
> Or perhaps use 'buf' and 'size' from parameter if they are valid, else
> get the from context/arg [2]?
> I think this solves your problem, can you please verify it?
>
> btw, 'buf' usage is a little more complex, since `parse_int()` checks
> for "ctx->object != NULL" to continue, I can't really be sure about
> intention there, but please check usage in `parse_port()`, it looks like
> when 'buf' provided expectation is to get parsed value in the 'buf'.
>
>
>
>
> [1]
> switch (size) {
> case sizeof(uint8_t):
> ...
> case sizeof(uint16_t):
> ...
> ...
> case sizeof(uint64_t):
> ...
>
>
> [2]
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 0d521159e97d..cd123c69265e 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -10805,8 +10805,10 @@ parse_int(struct context *ctx, const struct
> token *token,
> goto error;
> return len;
> }
> - buf = (uint8_t *)ctx->object + arg->offset;
> - size = arg->size;
> + if (buf == NULL || size == 0) {
> + buf = (uint8_t *)ctx->object + arg->offset;
> + size = arg->size;
> + }
> if (u > RTE_LEN2MASK(size * CHAR_BIT, uint64_t))
> return -1;
> objmask:
>
About the fault root cause.
There were 2 uncoupled resources in that case: static token size and
variable size passed to parse_int().
parse_int() caller must provide a buffer large enough for token size.
Otherwise parse_int() will corrupt memory outside the input buffer.
In the generic solution parse_int() caller allocates target buffer using
existing knowledge about input token size.
Testpmd add_port() imitates the ARGS_ENTRY() macro that extrapolates
target buffer size from RTE structure member.
Current testpmd cannot use that approach directly because indirect action
references internal testpmd ID.
Testpmd indirect ID has no defined type or token that leads to indirect ID
parser.
As a solution, testpmd can provide centralized parser function for all
indirect IDs. The function will parse ID value and use the token as the
key to indirect database search:
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index ce71818705..7fb3b61d37 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
+typedef uint32_t indirect_id_t;
+
+static int
+parse_indirect_index(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size)
+{
+ indirect_id_t id;
+
+ ctx->object = &id;
+ parse_int(ctx, token, str, len, ctx->object, sizeof(id));
+
+ switch (ctx->curr) {
+ case INDIRECT_ACTION_ID2PTR:
+ /**/
+ break;
+ case INDIRECT_LIST_ACTION_ID2PTR_HANDLE:
+ /**/
+ break;
+ case INDIRECT_LIST_ACTION_ID2PTR_CONF:
+ /**/
+ break;
+ default:
+ /**/
+ }
+
+
+}
+
Regards,
Gregory