From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id EB06742804; Wed, 22 Mar 2023 09:53:32 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B46DF41148; Wed, 22 Mar 2023 09:53:32 +0100 (CET) Received: from NAM10-BN7-obe.outbound.protection.outlook.com (mail-bn7nam10on2077.outbound.protection.outlook.com [40.107.92.77]) by mails.dpdk.org (Postfix) with ESMTP id F013540A84 for ; Wed, 22 Mar 2023 09:53:30 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SWXm/Rw+9BInpoO1ADlCNdU6a0OT5AoYNoc178ZVi92f9f4/y0fss4WFfYls4JxUIkICoRbLYcqvXsRkQcBl8k9TNThT2Xb/xpo+t/uolo5XMQKx8f6DvVvxN+zsi6FcNnQy8yXmGaSjjDdQXthLGoUYH3JJwqvZNWJuY7K4KI0Vg+hE6EOhczHdX45B2xARVVqFJO6PW7QiPKul/k+mg/r2CqEaXKAqJ/VZwEUrhADfymF5U9SLlmEvR7vgruXYRe+vI4a2//x1Ex/fdXnQRqzw/Ye9mhU+MWlG/N/64ELzTSp1o4B7TK6OsGkSLicOmnqFuvJ7zy6vq4fRffMaPA== 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=N+hWOj8yIP0Ea/p8W9mjYskKj41ATnP6612viC3HIv4=; b=WfD41DlDy9x18aDuqQMp8CfRPgOTTovlBh+APgGwtg6xF64NYyxeTR1wv6tkMjiHSDqstOKrG1nBpLDuwVFZEbnlVSylc7RQilZuHHqsmMyeURMA9ro/e4l1vta0UGiddhO1EImejKLaFa8zIZouvAeS0JmSJD2X6sKxv5v8kINzN3Dc8f5SQWr8eEm03tr94RXekPZpJX/Q4DSFcoM9QaE4HZdhIFIkzoAy2YD7obevSUdppUM1VvgH14gJuGTV/DLhMYBWIKrXsRLPv5SU+BBva9Fb0A7VXlN9y2qn45gj/Wb50XZGBC1Xi0Gm37xNI1bieJ/gtRmK4eLfOYa1ZQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=N+hWOj8yIP0Ea/p8W9mjYskKj41ATnP6612viC3HIv4=; b=yqzI7g2L0ZWKiDaGdv8aYR2DVCCnojg5J1vzTe9Qky801FiD0TUnG5CZ5HP7QVmA9n9ao8qyugcMd43z3lxwoQvWg8tLLI4MAq4hlWBTKX1vPxwNRZNMYlNfnaljWP/7u7WbV1Dvz51n1BpA86v+PQuVriPrOI0WLM+gHIfqU+s= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by PH7PR12MB8178.namprd12.prod.outlook.com (2603:10b6:510:2b3::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6178.37; Wed, 22 Mar 2023 08:53:28 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::dd5a:8a5c:f493:9640]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::dd5a:8a5c:f493:9640%5]) with mapi id 15.20.6178.037; Wed, 22 Mar 2023 08:53:28 +0000 Message-ID: <6e6b0618-3f36-1832-19a2-9dcadd62de9b@amd.com> Date: Wed, 22 Mar 2023 08:53:21 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Content-Language: en-US To: fengchengwen , thomas@monjalon.net, Olivier Matz Cc: dev@dpdk.org, David Marchand References: <20230314124813.39521-1-fengchengwen@huawei.com> <53326af4-26ab-eba7-047f-42917e8f3b00@amd.com> <75f7ed07-3e9d-0ac6-2c14-64c598b434ec@amd.com> From: Ferruh Yigit Subject: Re: [PATCH 0/5] fix segment fault when parse args In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P123CA0208.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:1a5::15) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|PH7PR12MB8178:EE_ X-MS-Office365-Filtering-Correlation-Id: 7fd8096e-edb2-4f3f-495e-08db2ab2e774 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: xP3rommaQ6kDcAZK9i2NS5KojlLlV0SHwcjXLmfTM/4ILwJLBcB57ewbcYqJg0GpiGZtpfk444vtXSXp9i/QjCpOJsl9XL6Hc1BYESerXeyrGS3l4wV08YiZupd1By059PzOT+oOZ+Vc5wktvZuqHAZVrXcYlWvAzAQCFa3U7hJE4SrJhAlnK9CEIu2WJUoB+y5nUTP0icCYX2XKbSPbWfPIoyHI779vie8j5skUifjWJWXh/b2L8ZLX5NzA9117W9TRc7uO8B0CXrBJjdh2wRRn5YIlPE5iP/PSDqvGtVrRFH6V02gqFXGIVrtL53aBtA8JLVp1Z5kH4LonwjY4FeLowA/SwvHdV1jhU4yUuI37GqFtExU5OlhiVXdrA9NHt61gSaBIzwQp5E+7d/M6hcZqPeHGrZEr7zd8hmx8aY+/T0xp1xMsbMWNcmpYRv0pN8jLaFEKGDpNJwFgkglQ7xmCigGqAKYtXZ7azvH17eSn3JWcpsoLUegB+1N6pRIe1Gfqj1QPDjF9D5m+5f4K8425H9IYZuHKn9uSid33vm59jFzJg2u0PjZf4+ANbrzL97bD2wN5UjdORYj8Gps5Xk78A5Tm0La1tpe11gfiBmSANxxv6WPpapuG14X+wzlgJtWE5q5UuD+o/JKnfT3FDtdJ0aKhUacZPXUcgmytvgXSMBkeNYeKxCTwSnidvt7LS2pfnQ5NelVZNsUwmO+Pc13Fg9vQjs1bCGLq0qpB2lg= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(4636009)(346002)(39860400002)(396003)(376002)(366004)(136003)(451199018)(41300700001)(36756003)(66556008)(44832011)(4326008)(8676002)(66476007)(38100700002)(2906002)(66946007)(8936002)(86362001)(6512007)(5660300002)(26005)(31696002)(6506007)(53546011)(6666004)(31686004)(2616005)(6486002)(186003)(110136005)(478600001)(316002)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?clNnRzVpVmdFZXZXUUp6SmFPc3BBRzNOUVpTMkdjRFY4cUNZMEVBMzVNTGwz?= =?utf-8?B?SmwwdGp6WERkbk9CR0ZXS2o1NHdjWko4dUVRbzZySFZlSWxDMkpYRnBUWXNE?= =?utf-8?B?Q3ZLSmVkOWpKbWJLb3FBVlAyQ3NseklBanJ2VG9YOEM2cFVnTjQ4cjdjSERy?= =?utf-8?B?amFNOGRqQ3JVWFAySHV1OWhIZFIrQjUyUjI1eTVhZXl2L2thM2Nkc1RNZTJm?= =?utf-8?B?YUdVRVppZmowMGFMOVNTZ1Nid25PUGhEYkF4TExWYmMrcWR6azVISU16STMw?= =?utf-8?B?cU14WmtpdTNQdzB3VkVzZDRXUmVmS0l2NmtyM0NxZU9tT2ZVUS9TYUVRMERJ?= =?utf-8?B?Mk0zLzJtcTNLK2JXWEpUZHBsVU83R2R6ZG1DQjZDTGJtcVM5OERTMEI2NXFl?= =?utf-8?B?QnYrTEoyeURzb3NEakVlTXlZYjhMYklXV25MMFlKeEtOVlAwdHBma1hQVGg0?= =?utf-8?B?Z1FMUmU4Mk1lR0tGeUlYQXlUQ3VGV0JNT3Rxb0VkQU1aa3JuZDVSNnRkZVQ2?= =?utf-8?B?MVVvZmYwV0xJUFRDeHdCazNXQUNsVDlaMkZqdjdEbFFKc0NyM0k1MDNrTG5X?= =?utf-8?B?YmdSYzJrWnBVZDk2blZyYmxPaW13cmZ2YWFKd0U3Smh4S3IxU2c4UVFIL1FV?= =?utf-8?B?bFh0WWZmajN3WllyaU1yMmhqc3FXVkVDOUdoNk00VmlKcUxMNkN1R2tHeWdL?= =?utf-8?B?REE2QW92Y1M3amw2U1NFVUNkQmM1cnphWUIxVEdCTWYwZ2VyWFhobjdvUUJZ?= =?utf-8?B?RURqR0JRWiszTEJaWkdoOXM4KzRZbjJ2dE5sMStscWlvUXJYMnlwRW1UMWpm?= =?utf-8?B?clEyRXFrQU43aTBYcFVYNXVLalZ1aEJLVTF2VWtnQ0s0d2tGaDBGOTdYbG5o?= =?utf-8?B?QkUzUGlaa3E1SWx1THlYSTBVUVNXdFM3aXRhNWdDdkZtZnFCc1dZMzF0YUdI?= =?utf-8?B?aGI5K2dvWFRjekJERzJaM0xBOHp4aUdIY0VJQVdSK1B5Z1daVjBEZnZRaGpT?= =?utf-8?B?Ym9BSkxqUWJsbG9OYU42bGZ3dlQzQ0NLSFdZUHhJWlFPNjRZSFBaT3djbUhu?= =?utf-8?B?aXdDRmNqYit1TDhpLzNieHVmU3pKWUllaFlrMDVYM0czRzQ5L2kyNmZrY2VY?= =?utf-8?B?MUhXM2ZhREhsdmNJZm51TEdtcXM3MDZSNW1hT1htdDl5QzF1SHBSYkp4bDdB?= =?utf-8?B?VEU5RFBsU3dUZ2ZBbk5GK0RnTjVBNGdNT1BCd1poMmlZbyt0Mzc0SllmYVRi?= =?utf-8?B?OUJTWlhkbTZXV3lTQUZtV1pZbEp1Y05SMEN0MGdzaUExUk9waFB1NnE3Q3ZS?= =?utf-8?B?RUJGQ1dPRy9hdTFYOXNwdDBQdklKMk4vN1VVVEZmTktDQ3p2bGVkYm9wMnAw?= =?utf-8?B?VDBLUXRjY2VBYVZEZ2IzMW9VbGU5RXBHZXh0RjRQc0o1cWQrbGU1QWdYM3g3?= =?utf-8?B?Zjh4NkdreGlLdlBPUVA0d3Y0QVpON1N4WnBTUjVOS2Ezelo5YU9aNDZxOTRx?= =?utf-8?B?MjU3L2F2MmhiZ2JjUVR5Z0FGeG1XRU9vcUx3cEhTc3IrUTEwVnl3Tkl1STFV?= =?utf-8?B?SEhIZlpmZWRWSXBHRzM5WGkwRUxXYUpFL2ZmMnh5aytLSmFIZDUzd3NidWh6?= =?utf-8?B?WjcyWHpiSTZsQ3paOW13NjROU2RZY1VOVFlNcHBBUmdZa3U4ajJDWnlKbmhv?= =?utf-8?B?MkhBUzJYbWJrTXU0aUl2cDlVcFdBTHNIU0tvaGswTEVaUFhKeTdub05CTDZQ?= =?utf-8?B?Tk5OYWw4V2ZraThLbXlWRGFrZXQ4N09xOGF3aG8wanBISkZQaTlEaXV6eGov?= =?utf-8?B?ZERYT2Qrc3NyLzMzQWFKS2VPVzdXelAxQ0pzUjFGV0JxckhrUGdsSVdueHlj?= =?utf-8?B?elpQb29xQ0JnMGpjMlVteHNiL1owN3h0ZTRJejhpeVFpeFNEZUNaeUswQisy?= =?utf-8?B?SWt4MUZmUXJLekdOTjVZQ2x4SEtGWmlDcVRDY2wwcmFFOXd2UGhBakxBOTRl?= =?utf-8?B?Y2pXU3RobnRaaXNsRnNRN3I2MEx2TVpjT3BJQTBkUGozUGVxQXV0MDJhU3BO?= =?utf-8?B?VzhhRlpWaFd1SU1jNUpQMzVuUkViTVJWL0VKcnlaOTl4Q1V4Q1h0K1FyWmxo?= =?utf-8?Q?wPXC9FUtyO6LdpyMmWhXjQLxE?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7fd8096e-edb2-4f3f-495e-08db2ab2e774 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Mar 2023 08:53:28.1115 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: DRKpuivuocEpb99y/mIChWZaqHeHNpa/4oSHl8H44lhiVY5gZNf1MhYXoHKm0rBG X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB8178 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 3/22/2023 1:15 AM, fengchengwen wrote: > On 2023/3/21 21:50, Ferruh Yigit wrote: >> On 3/17/2023 2:43 AM, fengchengwen wrote: >>> On 2023/3/17 2:18, Ferruh Yigit wrote: >>>> On 3/14/2023 12:48 PM, Chengwen Feng wrote: >>>>> The rte_kvargs_process() was used to parse KV pairs, it also supports >>>>> to parse 'only keys' (e.g. socket_id) type. And the callback function >>>>> parameter 'value' is NULL when parsed 'only keys'. >>>>> >>>>> It may leads to segment fault when parse args with 'only key', this >>>>> patchset fixes rest of them. >>>>> >>>>> Chengwen Feng (5): >>>>> app/pdump: fix segment fault when parse args >>>>> net/memif: fix segment fault when parse devargs >>>>> net/pcap: fix segment fault when parse devargs >>>>> net/ring: fix segment fault when parse devargs >>>>> net/sfc: fix segment fault when parse devargs >>>> >>>> Hi Chengwen, >>>> >>>> Did you scan all `rte_kvargs_process()` instances? >>> >>> No, I was just looking at the modules I was concerned about. >>> I looked at it briefly, and some modules had the same problem. >>> >>>> >>>> >>>> And if there would be a way to tell kvargs that a value is expected (or >>>> not) this checks could be done in kvargs layer, I think this also can be >>>> to look at. >>> >>> Yes, the way to tell kvargs may lead to a lot of modifys and also break ABI. >>> I also think about just set value = "" when only exist key, It could perfectly solve the above segment scene. >>> But it also break the API's behavior. >>> >> >> What about having a new API, like `rte_kvargs_process_extended()`, >> >> That gets an additional flag as parameter, which may have values like >> following to indicate if key expects a value or not: >> ARG_MAY_HAVE_VALUE --> "key=value" OR 'key' >> ARG_WITH_VALUE --> "key=value" >> ARG_NO_VALUE --> 'key' >> >> Default flag can be 'ARG_MAY_HAVE_VALUE' and it becomes same as >> `rte_kvargs_process()`. >> >> This way instead of adding checks, relevant usage can be replaced by >> `rte_kvargs_process_extended()`, this requires similar amount of change >> but code will be more clean I think. >> >> Do you think does this work? > > Yes, it can work. > > But I think the introduction of new API adds some complexity. > And a good API definition could more simpler. > Other option is changing existing API, but that may be widely used and changing it impacts applications, I don't think it worth. Of course we can live with as it is and add checks to the callback functions, although I still believe a new 'process()' API is better idea. >> >> >>> >>> Or continue fix the exist code (about 10+ place more), >>> for new invoking, because the 'arg_handler_t' already well documented (52ab17efdecf935792ee1d0cb749c0dbd536c083), >>> they'll take the initiative to prevent this. >>> >>> >>> Hope for more advise for the next. >>> >>>> . >>>> >> >> . >>