From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5466CC64EC7 for ; Tue, 28 Feb 2023 16:08:30 +0000 (UTC) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A886D40EE6; Tue, 28 Feb 2023 17:08:29 +0100 (CET) Received: from NAM04-BN8-obe.outbound.protection.outlook.com (mail-bn8nam04on2081.outbound.protection.outlook.com [40.107.100.81]) by mails.dpdk.org (Postfix) with ESMTP id E2B394021F for ; Tue, 28 Feb 2023 17:08:27 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MS0TDsjVJwtpCMgyC+PhU1sqqvjXaubW+IM2o+tpmbdTD/r3sKwcd7Ox8wsylLO9nWeosZg7th2S0QQK7FHl4/j12E3GCHFIuesF/Osr4s5kmuvRhX56jZxNORMvVlXPPnxiFp+eX27LnukujsibJKfaXdN1/COhfnTSN0n5jFrJIHWRrA42LyiSs8wTGd29wcKENat0SVdgAICQpcIvVIETJQSZBLRR/fHJOUoy9jnx1QPbWT2mnM7PLwFuvpyCyNo7c69QF4BsQSxbMaFcLJNsrJyf2ICSaM+hAqaBnvcie5j6Aqa0hmLb3e701BuRVLFXvB3FANKK34u8z5HbhA== 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=nmOByxt3mhJ9iGfWsG1XrGJYZeJxIk8wnlts/6FhduA=; b=bCDJi64mY4HbXZrQjoaakzlJVCcq7A8PevaY8qCbHx00jS7AuO/6JRXKSUO87ZvcLK/Mf1vZx2KFmoIhR1G337MaHgFd/dROyPNEe7Mxb8RFwKMlKOMG8yVDr26Dgx2qKLm5Du3uLP6pX5lkA51Sf/ZMBaJcFnBw/E40A7evUhNUTupyMSxER2IGeYuj5m2IEaRlv7/QZoz4QKGM8MjACf/coYL33ck5szftUDEauUTs/spQ1JiwvZK5AzGekH88bT0XJceNHCGlhjYDjLju/Cia2nUuqgycs67FiyPj1JO3gi6XImfasq5zhtssyNnNcO+4YYoJ8VV7v79ltn1ciw== 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=nmOByxt3mhJ9iGfWsG1XrGJYZeJxIk8wnlts/6FhduA=; b=p0Fb8WKHQUq3Qg22aqUiwDjb14Gg+Kmge52hAxB9bRXmQe/6xsFPOXNdMERfjETQQeLN3fZvtOxRQ9uh6YOBC96bb9q1uSlwxi5NhUX+Z8aLkT9HmDo9PtWeBQE38WSsOVMnViAZqZdK2NNqS/Zk8eYTrSqzMo6hXDmZHw0zxmw= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from MN2PR12MB4301.namprd12.prod.outlook.com (2603:10b6:208:1d4::22) by SN7PR12MB6837.namprd12.prod.outlook.com (2603:10b6:806:267::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6134.29; Tue, 28 Feb 2023 16:08:26 +0000 Received: from MN2PR12MB4301.namprd12.prod.outlook.com ([fe80::80ae:e5ed:4fa7:2ad7]) by MN2PR12MB4301.namprd12.prod.outlook.com ([fe80::80ae:e5ed:4fa7:2ad7%9]) with mapi id 15.20.6134.030; Tue, 28 Feb 2023 16:08:25 +0000 Message-ID: <616170bc-5090-0f88-4506-58e56d5216ff@amd.com> Date: Tue, 28 Feb 2023 16:08:20 +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 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> From: Ferruh Yigit In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P265CA0143.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:2c4::19) To MN2PR12MB4301.namprd12.prod.outlook.com (2603:10b6:208:1d4::22) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MN2PR12MB4301:EE_|SN7PR12MB6837:EE_ X-MS-Office365-Filtering-Correlation-Id: 01d15760-a8fa-4ba1-a2f7-08db19a605d7 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: hS0Vp09lIgqCkBKHOARxI9YlL2e3Cq3WNh3RC59hpZZJxfCYXJw+IdtGEmeoI2IKhsmnq1L8IjYYyGJFAyVsa2dyQJEowrLvH3l/m3NNUWI0OvIM9yWYjQa2cdkYmxrZtNTlikyScV7PSJbVYGDhAo098Ino8wVJvKZhOB21K+E+TvH8vbNP6krW5CFYxJfH9BCjNsWbQ/9H5/1+LgQ5E9nzkKOCw70btcBDZEJCwyusTs7Zq3SjXImFxf6klXHYDZUT0O+gj3Z657PNaClMyZmYfAqzpz+JU5Gol9WwkpPH76GMA/H8nTF2GZy7vZlvtj1NGoJBTQ/vcH/FtPDOLiR/xMgMWqproOUCxsfpwSdcL1YR3uFvt6x4aT5Bk3zn6QgFjPhJ2AF54vApP8fszIwlIsQ/oenEDV6K/pZYGXVml72fHdkk+lIN6qogucZKtgxz61HXCcP7jUJjoZBV/kV678+vveYtcy/HWmN/dInOnZYB031Tjz59Le7solGwhftZRAOc/ci7+8h7ze+Zc7a/EWN083mepzvGm12kkJihc6N2dIHFayuzxYDjxskt/ZQBB9PvZI2xPML4n7DDxblkjHYsuSTK5ksdcC+CahzXBkPzqvGbI1AgjkxNX8Zaf4KLTRF1Yw9JViIkw15NWLIRHClZPnyBCrrYUxvzPiR7KExyVUHeIXmsx3pD0YLT7SGBulz74t5wEEJ7xWThCVUYgSD7rPpN8HLe7GoLh5o= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MN2PR12MB4301.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(4636009)(136003)(366004)(396003)(346002)(39860400002)(376002)(451199018)(66946007)(316002)(2906002)(31686004)(31696002)(44832011)(5660300002)(36756003)(8936002)(86362001)(53546011)(38100700002)(6512007)(6506007)(26005)(66476007)(110136005)(8676002)(66556008)(4326008)(41300700001)(478600001)(83380400001)(186003)(6486002)(2616005)(6666004)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?em1GbFZYTnhvOWpYWHNqNTg1cldqbDN6L0dRTVhNUXE3aDRJT3dLeGZMYXZk?= =?utf-8?B?d3d2MnBwYkZ1Z2hwenRIQm5LVkFIMDlpS2orcDJTSnl0UExycUs5cFducExU?= =?utf-8?B?enhKdjNrUEt5NGdjTjFwL29JQ2Jqc3F3NUJidTkxdFRHMWY2ajlpSitxd1pT?= =?utf-8?B?U1FsRmhBeHRCYnBEbnBIZEJXT1Q2U1VrdDFPRUVhNGQwZ2FXSHhCSEpSRWNP?= =?utf-8?B?eTd1V3JMV1V2c0hXamE4NTFieUp4YS95UmJvbDhjMVBkQlUvYjlVU3Riblpm?= =?utf-8?B?YlgxNTRkWkhSYytpdTM0aUtBakFlUGE5TjVWZHlYY3ZqRTA0ckEwLzZNYkZC?= =?utf-8?B?VmtXOGt6WG5VdTVHWUdocE83WUhkemdES1Bid0RpcXRjL3ZrY1h3dFBRV2xG?= =?utf-8?B?bG5FdTcvd0VGYzVoVmQ5ajBsL2k5M2JHRWI0QzVHSUR2Y2RMcnl1ZGVsRitn?= =?utf-8?B?N1JCOW4yZ3pMRXBnb1NDSnRBRWNkYXNndW5Ob1o4NzBxaFRQSmVpMDlFUjM0?= =?utf-8?B?YkxyQ2lWU09PbXQ2SG5yV2pZcHg0VmZSb0JFV1k1MlhScmJSZ0d1ZkdBWWVh?= =?utf-8?B?UmFPU0xQdldMVFFaWjBlNjBSblNWT1BFbHVVa1c0VDFvcmVRb20rV0c4T2hI?= =?utf-8?B?b29Hak0wWlV5OWh6bzd5QXE0YXFsNDBtQjQzMkRnN2phZGJoYWdrc3pXUFhC?= =?utf-8?B?a3Qzb2hsMzkvc3hpdG5GRFdxMjBpSmVjSHlQSUJ5TGpJMUZRQUNkVEdsNjc4?= =?utf-8?B?UU9DUms5ZXhHNmcyQ0ZMcTIzc21QM2kxQzFWc1Y0TW9MUTU2L09VTU9vTkZp?= =?utf-8?B?YmxiaCt0cVUrLzYxM3I4cWVLcmdYcFdkRHErY0Iva0N3S0hmR0tJOVZhN0o2?= =?utf-8?B?U1J4ZHJraVBSa3QzcUNQMDdHdzZLMHI5czAzQU9mcDZUTFFwVm1PWkxUQlNL?= =?utf-8?B?Qk1wN0lwWjVnM3o5OWs2a3JkVTdjT25RU1dCYldYSWd1MFZ0OFc4aWVtbnpB?= =?utf-8?B?M1BKNFRVWC9UemNocW1oSjVPMlhXSjRmOWJycnVYV2k3STdKM3NLOXYvQk1l?= =?utf-8?B?OGthMnZsa3RtdVY2aVg5dTl3YWFOWElXYk45b0hlc3JEeXcvQzlEeG80eVR4?= =?utf-8?B?QTVFK0Q4djJrK0NBMmEwcGhmSVhMN2MxRHl3SklyYUFCMWQ1S0Y3eXRKUFht?= =?utf-8?B?ekR2bGhjN2tlR2pWQWZZMHlIdmw1YmIxWHlEd2ZSeUxtQUVlS0FnSGlwTkFI?= =?utf-8?B?SzRiMFQvRkR1QkwvWTZXUmxLNlFuSTdudDcrNUg1dU9GNnBJR3B2Qmo0SWho?= =?utf-8?B?NDNjak9YOWI3Z2tHN0dnUHZEdCtpaGNRWmNSQm9pMlZiTnJLZUE1UTk5TytP?= =?utf-8?B?UXB6UG0yb2Rteks3NmdueG9WckEydCtnZlpGWkd6bkdVUDhpNklKL2pOM2Mr?= =?utf-8?B?QmxPb051WmdkTmJlUmdlUHFwclV0TllzZHlSbDRpZEJ0eDRUYnA2UzdBLzAz?= =?utf-8?B?RG5RTmpwSUJJVUwvVlBVODBEMjhZOXAyOGszQ2daS3dzaktzRmZBZ05UNTJI?= =?utf-8?B?ZTBrMmNZZDBrVlJXbUk4SGRvWndQcDZ4Y1ZJWWI3U2FLWFFPb3NHb0EwQm15?= =?utf-8?B?Mjc4MTBMYnJXL3IvaEpEeWl6UE1CTUMvbktuWW1xTVBkQXdJYU5qaVhMQ045?= =?utf-8?B?Y0lVNFVuWWQveGNOOWlxM2srY2tvTDdRN0pmM0Mwa0VJL3ZQbml1d29Ta0Fz?= =?utf-8?B?WTVTZmxXeFZJT3Q2VXZMaWhVa09NQzA0UHNvR3VmRVRuTVBWYjhZNXNFVnZN?= =?utf-8?B?NGNaUUM3NDlqeXVDeWo0K2I5WUNGWG9jRElsWnZZRlRUU2FVc1FzY3J4TWJR?= =?utf-8?B?a3RubGtxUVVjUTBHcVJkalVtc3IrK3FveFZXelVRdE0xOXdtRTc0dzRBK3dn?= =?utf-8?B?Uy9TRjF0T2pOMElaRWEwQ3BaajI3aDZPUFhUUWliZGIvSXdyRmxleWc1L2t6?= =?utf-8?B?ZGx0Tm5GQm9NSjJxeVY5WkxzYnhrVldaTUtqVkxCTk1rbDI0amRUSmZHdTBL?= =?utf-8?B?L1ZBRzhWKzNoWXFZVE5Tb0k2UnBiU1lxNDA3WWpPMnZwY2ZUQUZ5dlRhKytS?= =?utf-8?Q?3QjBTHx3vsIjkgI4S7S74lQje?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 01d15760-a8fa-4ba1-a2f7-08db19a605d7 X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB4301.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Feb 2023 16:08:25.8527 (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: xyToC1ScwYkZ+BndeqfcJs6fe0tLDGzPMgWXdRkv/65rot1z2/9to/fnveVYjtDS X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR12MB6837 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 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. > Yeah, I also looked at it but not able to identify why it is complaining. > 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. > Is it confirmed that patch fixing asan issue, I have not seen it in the bugzilla comments. If it is confirmed, OK to keep Bugzilla ID tag. And aside from the asan issue, OK to have this patch, because of reason you described. > - 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. >> >>> ) >>> @@ -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 >