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 6C1F141D3D; Thu, 2 Mar 2023 10:58:45 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0B7B440EE3; Thu, 2 Mar 2023 10:58:45 +0100 (CET) Received: from NAM10-BN7-obe.outbound.protection.outlook.com (mail-bn7nam10on2051.outbound.protection.outlook.com [40.107.92.51]) by mails.dpdk.org (Postfix) with ESMTP id AFC2D40E09 for ; Thu, 2 Mar 2023 10:58:43 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=jJq+xVRXSUaCN6MmLFV/CS1ZSPpySEfmJbUp9DZrqQ02xq7ccKmt8eATLl7xT4mWp/MXpFKBcVk3C4tS4XQVVfy2Kfrv/4Q8tQsvL6NS9NKh3L37ByZH84xewPsfwAuZGCH1LobLuziffbI3c/jkVMN+z1lBTmYnpVKJxjbPTY+q35iexqsMXtbsNz0yWBYo4SpPLQdi02OFN3hcsG1GT5SmT7UO49PkX41A80fwmPciBUsC7YmKhexZALRn+aeXDVHI+bP+jlEnN91Lv9RqbRoX6F1vVWgm62hu/2Wv6UMPncVkp4AiF512DGKVoJ15iRTf10qTo5XaIOF6eLDddw== 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=R7GnV2qYcChB7OcvNryphrOCBH+aUv8P3+6DQ+O4hQE=; b=k5Uz4Cyp33AhyfwDeaZjkKfD5H/X6GJb3MzeDTP58JEfCzyX4bachiZapL9JwR+ESXUXDdOr2bpiEIlWJQ4tK30swzXz13KLzJQf2L9RDc2LPijoY4fmXceyfGgSqwiVNMuYbhZVvmSdUnVeBBEdgd69uZFtmAXfSO8mHl7L/ydcUsMegtL4ngXpmWX8jgqoW4/+JkuImkdxqnzJVwg1TijXjfX3tjD2akwMu9D54fiMDojinmMX7rV2O3Np+gOKYH4cOguKk5U+AGk3rCYfeabgnT3+1PucWBVCKMmJR8PlK/Pl9xKn0P1E0LBakrFi7jv/z2nUrONus0mU9S8+0g== 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=R7GnV2qYcChB7OcvNryphrOCBH+aUv8P3+6DQ+O4hQE=; b=OuXbR+3wP12MeA8xc7rSH8QeItIHOhve2s3F9Qx5xGIU+BtWq5Zvqxd414O9kSgAuqpj9uQ7OYeXq5ShhYB54OZTc1A5dtVDwX7fT3HIHDJWc0EjgrFOTcbN+qO1Ji/Qom+gVVQiZFUj8pO9QGgr4z/ZbP0eKVxJYYxtDQ+pExE= 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 MN0PR12MB5738.namprd12.prod.outlook.com (2603:10b6:208:371::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6156.17; Thu, 2 Mar 2023 09:58:40 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::dd5a:8a5c:f493:9640]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::dd5a:8a5c:f493:9640%4]) with mapi id 15.20.6156.017; Thu, 2 Mar 2023 09:58:40 +0000 Message-ID: <3696b520-4c8e-83f4-cdbd-d244b2a2cf69@amd.com> Date: Thu, 2 Mar 2023 09:58:34 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [EXT] Re: [PATCH v1 1/2] ethdev: fix null pointer dereference Content-Language: en-US From: Ferruh Yigit To: Ankur Dwivedi , "dev@dpdk.org" Cc: Jerin Jacob Kollanukkaran References: <20230223123029.2117781-1-adwivedi@marvell.com> <20230223123029.2117781-2-adwivedi@marvell.com> <5b714a8e-1851-d204-18bc-6fe9cd5d5afd@amd.com> <008678c6-580c-af82-3696-23726b225861@amd.com> In-Reply-To: <008678c6-580c-af82-3696-23726b225861@amd.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO2P123CA0013.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:a6::25) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|MN0PR12MB5738:EE_ X-MS-Office365-Filtering-Correlation-Id: 40edfc84-70f3-4b26-0979-08db1b04b2ff X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: h/X+BGWHqKQ8A4L7g+B2zzDmZNu7u+m4ou+R7Dce7RWcm4mYo9fmKgcdPwAU/YBylj0AChoct1VbW7LRFDx0aBj51x5FynIFSL7DpE+fYmCFEwitm88g/X7+HGMMrRkzcJRiMVdnGb7TS5hxwDO91oMXDE5vcQQPeWdOuDW/ZA/gQ446DK95qojA2b0GnPrRMAvfl5e4vZ1W+cv76EOnwuzov9nvI4MxDhmNjNjTTV0ZDJRXtz/KTlXR6WzDJw1iHRw7KfQOUhfdCXTEvXDeOxBZOzlkZrCg17Hxpqitq+WR1VaVo44xbn9QPvvcZ1KGsQizmMvCIkiyNNyFclVUWyt5ktMgmW7kxgfriePpXWeSTcnpNB7w7p0FFdzwsQ2iW6QU54zHnscv+QUhTlXWbwOu/LmGyWW/c0tT8gOtZ0qRYdD6qWBX7T4iAGkmfJ+2uRhMS1YrGqciJ4g41w9NPuaBaFPOeeqR+6te11gvYhhtmODxhXEq9xEB0SaGd4BcgUcAgSWMmesqZedm6UPuLkAlrpJWX8QVrOoQiPOcIQnjJutZdHchR3+REkaeuB6PgcI1B50cQGuel9BbkH+/FFBEwfOt7W7HUaPj8fZldRDplDaVK748S3CU9ZBlMteW1erawieBvbRBbhEBnWXwjpOuHcHTM//+z8gdnyOK//xVGT6RI3PpWzaxE74Ak2CzC8x7/Dfx/w4zxjsND2lE8nF5oYjc/3l1oLHpWSddklI= 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)(376002)(136003)(39860400002)(346002)(366004)(396003)(451199018)(44832011)(6666004)(6486002)(36756003)(478600001)(31696002)(66476007)(8676002)(66946007)(4326008)(66556008)(86362001)(5660300002)(41300700001)(110136005)(316002)(38100700002)(2906002)(2616005)(83380400001)(8936002)(31686004)(6512007)(6506007)(26005)(53546011)(186003)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?R2tPODd5ckltS1FkU1ZLODZFRlgxUGo3RmlONVIwdWdTVkppZ1ZRMDY0SXhv?= =?utf-8?B?aDRjOVM4cG45eEpXSWMwOFIxWWxXWitVSWVjcE9PcmgybGVEdFdtSzZ4WlZX?= =?utf-8?B?WFN1UUFxbkZPb1V4LzNxNWRkVCtselk2ZktHUEJRdWJjMk43UndrYWJSK1VJ?= =?utf-8?B?QXhqMHl2QlNJRW54cXBRR3ZSN3Y3N3Vsakh0RkR1S09XMGRpa3hCZW01RGRj?= =?utf-8?B?Q251T1h0SE9vOTU1dVZRTzRSVy92MmdYK1YvdlJDS0pEUzhyaWFBOTdxZzll?= =?utf-8?B?SU55T0hTU2ppWXlCN3JYazQzTHY4LzJNbXVqbERwRnlKblA0Zm1hcHdIbW9u?= =?utf-8?B?MlRIS1BZeU9obCtob2tEai9HcmJmRVc2VVNQcW9rWGgrS0VZd1djUSt5ZnAx?= =?utf-8?B?V2Jya1JUVXdXakcrcWJKOUR6WU8vKzZsa001dkhBdHhoS1VoMVY5dHdRRW5z?= =?utf-8?B?TG9ObTlSVXlhM2t5OFBRb1pTaFJQSWZjQVpqZ1VDMU5zdlE4NVppVEdRTlRD?= =?utf-8?B?RHErNm1raGJ1aXZWR0dwN2ROUUliWTRKR3N0QWNIWjdPV2wzY2FUMXNVZGw0?= =?utf-8?B?L3A5K0U0Qnh6d2JIeThMOXhXcEM1bllSLyswS2hpRUhYNWx0OTBjNTBjNmQy?= =?utf-8?B?ZGtzRjBEbXRJVmtqOHYvVlVHQVRTcUNtc1VLUFk2VS81bDh3MzBTbWNzM0Mv?= =?utf-8?B?bUR0WldJYk5uZHRHdjgxWG53T3BPMW1Ic2RQNnlFRnN2YWJ2S3ZLL0Nwc096?= =?utf-8?B?L1VxeXBZWnhqeTdJT1oxenk5Y2cxSkRKR2lpZE05ZTJJTllhSlVIM0Irbk4z?= =?utf-8?B?aGhOMjRYNG1Wd3BZQ1dwdHY2YVNYUGZ2c3pQOUFHYU5zdno3WmRoL3FKZDNP?= =?utf-8?B?cXlPM3ZhdW41OXYvM210Z2xtelk5N3ljeUhlVEk3K0VWSzJMT3ZleU1ENUhR?= =?utf-8?B?OGdqaU9MaU9zZlBHQXlsdG1DYVRyTkRFeXhMekdIaVNIV3ptZVdnS0F1ejc5?= =?utf-8?B?emozSzNBVExUV2tPaWJTN29QczRtOUVaR2FPU3NpVndJTHB0WTdZQlNOZWNQ?= =?utf-8?B?eWxpbDBET05jMjQ3RW9tRG1Ma0FyOFRCQnJKbzF5MUNxUkNJK0N6SGR6M3Iy?= =?utf-8?B?S094dVl0dGg2TjZaTVpSUmZiVnBoeEpYY0FzaXdBUFpZSnJUakxiV0lKeElD?= =?utf-8?B?NHFqaGs0V3pjSE80RTBNYVRVc3hUMkx0QVVBMXdyYWFXNGthemN0MGhvM2xT?= =?utf-8?B?MG5KMHZsZ3ROVW1Eam1QbDdaY1NoUWhyN1ZGT3d1OEZzd0p6VjlLQk5CZUg3?= =?utf-8?B?WUx4cnBpbVQ1QmkwZEVRL05MdE5PQlNlNTQ4dm1hU1RDdWw4T0Z5WUZmOGZr?= =?utf-8?B?TUtiMWdDZEk0YlEyUUY1eDdnOEJoSUgrWk1uLzhXUmVpMnhEZkJEL05uL1pW?= =?utf-8?B?VFRuY1l5WkczL3hhNGNUelRpaGdZcFJ4bnRwL21YdUxHS3VHUEpmcitmbUNH?= =?utf-8?B?RFViaVRqWnRhNmV1NFBCa2NZaXdVV0pLeVl6b2V4Qlh6YU5wdWlaWUxkUDEr?= =?utf-8?B?ZW9zOTlYc0FKdVkvT3FkZzNBeHlCZXdZSkRZOVR1ekgwK29tajNGMm5GcFp0?= =?utf-8?B?QlZ2cGM5U0V3T1JkZkp2UU0zQ253dG0xaGcydEtHUFlvd0hGUUdRamdNbHZ2?= =?utf-8?B?WUtZRFRXd01YbmhxcXE5MG9kVzZMNEZVdHZkVVlyZ2JLWmhmZWxueDVscWwy?= =?utf-8?B?R2k1TEZ0OUROa3pSZENFSHRkWVVqakYrcWVLWVpTY28rSDFYaVFjeG9HZXpX?= =?utf-8?B?eUYyQXlTSStIR2d1cHdMOWxiS1RhbnZjOFhqQWNJaGlWajZOeFhxWmczQWhI?= =?utf-8?B?V2FuL2F2NENQMmRDWHZqR1RRekx4aVFzbmg4ZlJOSzhKcVVtK3hubnlDeWcw?= =?utf-8?B?eUZ3NTJBSEtUblVaT09kMkxMRXFjbTUvM2syWUhENzFCaXo2NnBKdk8zd1Vt?= =?utf-8?B?UmN0Yy9PbkJWbHhsdVAxNHBYSzVnSVh2NUdTYW55OFlON3ZsQUUwZFJWaWNE?= =?utf-8?B?YnY4bzQzWmk3RkUrbkk3UXdqZEFOQmtuTkdSSUtFWWxpblpuS2NqU0ZkMXZX?= =?utf-8?Q?82m8UF2wRG40bXuSSz9ZwSvJG?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 40edfc84-70f3-4b26-0979-08db1b04b2ff X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Mar 2023 09:58:40.2278 (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: eBRpHFeaFYbtRx6XyzJ+RxBETXRRDZTMi3/YoMTUZ8LKLWbuUFItaZb6d/S3Up4J X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB5738 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 2/28/2023 4:27 PM, Ferruh Yigit wrote: > On 2/28/2023 3:40 PM, Ankur Dwivedi wrote: >>> ---------------------------------------------------------------------- >>> On 2/23/2023 12:30 PM, Ankur Dwivedi wrote: >>>> The speed_fec_capa pointer can be null. So dereferencing the pointer >>>> is removed and only the pointer is captured in trace function. >>>> Fixed few more trace functions in which null pointer can be dereferenced. >>>> >>>> Coverity issue: 383238 >>>> Bugzilla ID: 1162 >>>> Fixes: 6679cf21d608 ("ethdev: add trace points") >>>> Fixes: ed04fd4072e9 ("ethdev: add trace points for flow") >>>> >>> >>> In below changes, pointers can be NULL at runtime, so agree on to the change, >>> with minor comments please see below. >>> >>> >>> But I don't think this is a fix for Bug 1162, which is an ASan reported error, can >>> you please drop that tag unless it is verified. >> The asan error reported in 1162 was because of capturing rte_trace_point_emit_string(parent->bus_info); in >> rte_eth_trace_find_next_of. I could not find the exact reason for the asan error with parent->bus_info. >> >> But I think parent pointer can be NULL in rte_eth_find_next_of, so I removed the pointer reference. This resolved the asan error in 1162 as a side effect. >> >> - rte_trace_point_emit_string(parent->name); >> - rte_trace_point_emit_string(parent->bus_info); >> - rte_trace_point_emit_int(parent->numa_node); >> + rte_trace_point_emit_ptr(parent); >> >> I will check if I can add an if condition to check if a pointer is NULL inside the trace function. If that works I will update this patch. >>> >>> <...> >>> >>>> @@ -2308,8 +2300,7 @@ RTE_TRACE_POINT_FP( >>>> int ret), >>>> rte_trace_point_emit_u16(port_id); >>>> rte_trace_point_emit_ptr(flow); >>>> - rte_trace_point_emit_int(action->type); >>>> - rte_trace_point_emit_ptr(action->conf); >>>> + rte_trace_point_emit_ptr(action); >>>> rte_trace_point_emit_ptr(data); >>>> rte_trace_point_emit_int(ret); >>> >>> I think 'rte_flow_trace_create()' is missed, rest looks OK. >>> >>> Can you please double check 'rte_flow_trace_create()' too? >> Yes. Will add rte_flow_trace_create. > > > Can you please check 'rte_eth_trace_read_clock()' too, > it has: > RTE_TRACE_POINT_ARGS(..., const uint64_t *clk, ...) > uint64_t clk_v = *clk; > Hi Ankur, It seems bug 1162 is verified with this patch, can you please send a v2 so we can close the defect. Thanks, ferruh >>> >>>> ) >>>> @@ -2349,14 +2340,8 @@ RTE_TRACE_POINT_FP( >>>> const struct rte_flow_indir_action_conf *conf, >>>> const struct rte_flow_action *action, >>>> const struct rte_flow_action_handle *handle), >>>> - uint8_t ingress = conf->ingress; >>>> - uint8_t egress = conf->egress; >>>> - uint8_t transfer = conf->transfer; >>>> - >>>> rte_trace_point_emit_u16(port_id); >>>> - rte_trace_point_emit_u8(ingress); >>>> - rte_trace_point_emit_u8(egress); >>>> - rte_trace_point_emit_u8(transfer); >>>> + rte_trace_point_emit_ptr(conf); >>>> rte_trace_point_emit_ptr(action); >>>> rte_trace_point_emit_ptr(handle); >>>> ) >>> >>> This change is different than others, this is removing bitfield related local >>> variable assignment. >>> >>> According to bug 1167 that is causing a crash. So we need a separate patch to >>> either remove or fix bitfield related issues, for now I am OK to remove (as >>> done above). >>> >>> But can you please make another patch for bitfield issue and move above >>> change to that patch? >> >> Yes, will move this change to fix bitfield patch. >>> >>> Thanks, >>> ferruh >> >