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 AC889C64EC7 for ; Tue, 28 Feb 2023 13:39:36 +0000 (UTC) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 020FB410D4; Tue, 28 Feb 2023 14:39:36 +0100 (CET) Received: from NAM11-CO1-obe.outbound.protection.outlook.com (mail-co1nam11on2055.outbound.protection.outlook.com [40.107.220.55]) by mails.dpdk.org (Postfix) with ESMTP id 8DCBA4021F for ; Tue, 28 Feb 2023 14:39:34 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RmswPpX3uaT1SmlneCmm33Xv4zSmEPKHu2O80yDbSjMLbQZP8lL5ZiKTL9e50asAHtmBQ9JQLdFpjIz/qK3YZN8bWVMyU5UqKizSAKkXXRpB5cqdtNEG2uUjrzhzuQ/vmQyYFFA5lPpw2HohSGZ9+XHmYYXS5YEjf+7MEAGLsXBcX8Bb8dWjR9T3zb3nlVe7cIlSECY+32j/KgtFRbDSC0k6nRv+6IA9xKgvLBUq4JQu7nWF3h/417V93TaPHUkh7jl/OyHaM2Rw33+T9aSnp1rphIpfr+t7V0GWfXjlkdscQ6eVHRwhJzXkj5INuy24i1caDeEJZUvfm7K5CgH7xg== 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=yPW+PRjuseFdsrfhLQllsxPGPSdmesh4sJ4onX93UKU=; b=UpHVRWmuAyXamGsYshSETpvpMLD2b/QVSfA/vWfmkclSd8n6HCp6nauK/PSkgKA3jTrqX+2UHD7rPJZAHhz/PLXH7Z6BiXAMZHRrPuIH8J/zZZVj2/SBYMG1+162qD2iiTZSckIpoms5CBXcFYY/nGfUCkPlDtWx3pX8s5pOckDbatm7nKcHw05P9GWR1UU8bsO+DQjD4MxNnXhq+/726ggpji4kRpG4QCKTnmNo9qH6/J925eqCBlQHWvs37NhPefx7NjkUtkbX35BOex3I3XF9madzrsPhTWbqJMGySjKJ+n8q9J/OB+f9C+kcCPpYGNfoTle0TQzeeRo2MV5QNg== 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=yPW+PRjuseFdsrfhLQllsxPGPSdmesh4sJ4onX93UKU=; b=mEBAVHQVQwPWXx/7+Segf4OPzm6+dNW/7o41zGTWpcNuhBNC+0OUMMSId9rtBL/0LgguKtarMtcBCl7dBQvHFrnGOMCq/3ek1AMGu9+XRV7AR1wMaM1FUyLwjW2Mp9cY/zxEVwoS3R0QAJIxt4T1mjiNGVVF/iCQFnQfWrimcgg= 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 CY8PR12MB7681.namprd12.prod.outlook.com (2603:10b6:930:84::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6134.30; Tue, 28 Feb 2023 13:39:32 +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 13:39:32 +0000 Message-ID: <2fddd022-f95e-aac3-8da5-3a3f6f064f55@amd.com> Date: Tue, 28 Feb 2023 13:39:25 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Content-Language: en-US To: Jerin Jacob Cc: David Marchand , Ankur Dwivedi , dev@dpdk.org, Thomas Monjalon , jerinj@marvell.com, Ali Alnubani , "Li, WeiyuanX" References: <20230223123029.2117781-1-adwivedi@marvell.com> <20230223123029.2117781-2-adwivedi@marvell.com> From: Ferruh Yigit Subject: Re: [PATCH v1 1/2] ethdev: fix null pointer dereference In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO2P123CA0095.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:139::10) To MN2PR12MB4301.namprd12.prod.outlook.com (2603:10b6:208:1d4::22) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MN2PR12MB4301:EE_|CY8PR12MB7681:EE_ X-MS-Office365-Filtering-Correlation-Id: 9bac2c3a-e235-45a2-4219-08db199138e1 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 00Rzs81X49nUye616YI0NYq6fAk+XJdvjdeHGgpu4KRKFLN5uGvkmMOYpFmrkwuNf72pXABGE/siNE78WyDNZk9Su6LF47iWf4sddsgwAmRbgS4X8mUVXYvGaa/odoC1CNB2e6rpswFLdrmYrhb6KmdMcOT1x93qPx7nxQGZqRKJrumuAFWk5SlkSEnGT6Mp39iD6Koluv/6fBy0OVFIBoFxW+fySioRxrkg7OFNPOnPoPGbYvLx2K9Y6zDGnW3GOhh0nKkiNh5JFYDNEWS79Eg3jglV3uFnNyRSTu5jwg2eGJqxYGJwrrsJUJWzuB6MQcqQ4xvaC5Q5QdZVGVP0S/6AUDynUHBGs2FLso6qY+A92W4Xkq7GGRhp4UPMfEn5lsK0T1wHCsHxMx8di1Cw0z4etvS2xr85nyM0jElyxX95nrvUtNKVNdhMRRGeCNRs6XJiQafbq6wk9GeylCJdbYMm4J9uQclvPpR+5uwj9D/22LkUnItj6JSoyJR+ExolxSX4XzmGQEuPyX18SSipT0w8AHrK/Jn3JzdX4m5jk6KdmUjWYCgWuq5nBqdAvxxvre8If5drgFvIWs0tLqhCFVq0AhQZCpUhGezMjQlduJRIzrkh5mj7UVZXC1wmwHjAZN69xAdv1Wf/UAhxpjc4ItsJIYFPF87LrBa3p3RQhoaOWwwQmMC2bRs6RsPkBNcyd7LQb6Akq7ntsUB7zjcomquyYsBD1C+Kz39c9GO4kto= 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)(39860400002)(396003)(376002)(346002)(366004)(451199018)(31686004)(36756003)(31696002)(86362001)(66476007)(8676002)(66556008)(41300700001)(5660300002)(4326008)(8936002)(6916009)(66946007)(44832011)(2906002)(38100700002)(6666004)(6486002)(966005)(478600001)(54906003)(316002)(26005)(83380400001)(2616005)(6512007)(186003)(6506007)(53546011)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VjU1VDl4Y0hrR2dodWtiS1pUeVdFV1d1MWkwYUlOZkVXVGJBSmh1QUpzUC9p?= =?utf-8?B?N1pIM2h6Ui9TYzJvUzdoOVZhakcyWkkzektBcVRHQnd1ekl1RTI0T2FvdTZn?= =?utf-8?B?RktWd284emtZNTVjZ3poa3lSQkU0aVZHSTA4TTBwNnJpQURWNlo1WUZMSHpv?= =?utf-8?B?N1NkZzdwQVFheGpyMlJOdXZMSDhNNTU3UXR3MkdWMFM2eWVHeHovbnI1N1M5?= =?utf-8?B?S3V5VUdPZW0xYVY0bzlrVmhIWnVrYWpaMnVaei8vSVNiU2pYTFJ4TEE0cVhR?= =?utf-8?B?THJicDdnTXlqL0VRY1BYaGZrVXBkRW0xdU5TbElnOTFZYzljQXI5dHFxc01w?= =?utf-8?B?OTVNL1g3Mk5vMFlIR0ZvbUZldmgvNUZIUUpPeWtFTXF4K3dxQjNPRzVDNkZu?= =?utf-8?B?NmxLb1dhS2IrWkpESVJYc2hDVmZyU1c0NXR2VUIvblJ0cGFoQ09MRHJoeUo1?= =?utf-8?B?UGNRaTZCRDhGMzg5a2puZ3BMU0ZSM2xOY1k2NkYzaVFxZG04Tmp0enptYi9k?= =?utf-8?B?aUNsYUlnZW1JL1RDY3NGNGtsb3BvYk03NFZNakE3Zk1nSGpSRnRqR2xjL2Ro?= =?utf-8?B?Tnlyb2ttSDFIRytJMWZURXEyb2MveGFFMUVsUk9kRm1od25PdW1IMG1JeU9j?= =?utf-8?B?VCtWdFo3SjFIa2xKakNQb0x6WVVaZGVRYnRtcFR2bEpRR3hjUnNBWHNyUDYv?= =?utf-8?B?d0d1Q2lESmxEMm91TWJEYnhwZE9jYTZ4NGtqYUtieE80cWI4bi9POUdQc3ZU?= =?utf-8?B?ellUNkVtdFkvOUREalp4YVV0N3pjM05xdEFkQ2ZXK2hpL0dQOGQvWFFielV3?= =?utf-8?B?Z3d5Z2ZrcWpyd0JHaFhHMXRYYkVnbUM0ZVN6aHdoRERWcUJMYVVDRWQ4MVhK?= =?utf-8?B?ZUhXeEVYVTdoNE5VUDV0SDBISVFrOWpuSlpmbEhBQmZCV25QZW10V2lyckRB?= =?utf-8?B?dWkrU0pMSUMxVncvNWEzeHErSms2SmxIRld3VE1UMmg0MnJPdGVMdG1ueGNW?= =?utf-8?B?ZzZXenFZTUhwQ0sxK241cm5xVGJXcDVMa3ZBSDdGKzBQMEMwb1pYV0ZvTzlY?= =?utf-8?B?M2J2OGxFWnVadGRCUERiVUlZNmRjandDdllUYk5QT2RieVphMmkvMHdBUVFj?= =?utf-8?B?bHV0UUVJY0VLNjA1cjkwNVUrRGprM1RJR1FaamJnZ05YSW1iYUp6eGVXYkpt?= =?utf-8?B?eElFYTdIeEdBSmxybDNvbjV4QnVOY0JEU0FOVHpCSnpCSjUyREJXZkkrekk0?= =?utf-8?B?ZWRxV3RhdVpNSUt6aEZBOU1VaWNtbi9xUkE3RzBBVm13SkNyeDNOS1JIalBF?= =?utf-8?B?aURuT0x3elFaZ2N4SjdzYitLOE5ucVIrYmJWTjdwbXFpckxTT3Y3NzZVNnVU?= =?utf-8?B?YTVMSkpnTHpPVDVXdXFkWWx3YVU3Zm5Yb3pQUHc2U1QrazdWR0ozck5sTWRF?= =?utf-8?B?ZUpQV1k3Z2NnSVEvOU5MTVB6dHVTTUFadVdFN3E5a216YWVXMmhjWTRGTFJu?= =?utf-8?B?bmtFOGw4cmMrb0JsVmRtS3dQVE9za3ZUQjBOOHNOYUFndXZmYjdPTTdrUC8z?= =?utf-8?B?SnVEc28wK1NnSzAxMTU3YWlaSmorSVNYU2NFU21qamwrbUppNEM4YVhUWk9k?= =?utf-8?B?RkZJV1hOSlF4aVh3dFpMRGFJZFdyUUlUNTlQUEhyVkxJNzlLMkhvUEJIS2xt?= =?utf-8?B?T0d2Nms2b2JmSjQrMWFJdXpoYnlUL2JNRytEbVhsQWZidFFqbFplN1RqMm5n?= =?utf-8?B?UXBZWDBVVUJOKzVEeFdXMUJBUWJNMUxLRmUyOUhmRXoraCtBaTNucUJUOFFF?= =?utf-8?B?V1B4Y3hQSnY1elhlczdtZm5wem1jVS9pTHlVRE9vUU5ZcnhzSE11OFROT1Bv?= =?utf-8?B?MkVDbktaUTZXWVFQbmZMRlpXUUhWUnd3eEU3OGFZWnhVTzNaN2RDZlhzRjFh?= =?utf-8?B?OE9ZSE5zL1QvK3Foa0ZLKzMvZDFtNlo1MnJPRUdZTk4rTWZWUVVsWUtQb2FV?= =?utf-8?B?eDJyZGR0TDRDNjNyWStRU3lZa0Z4ZzZQQlNTelphRktGZkRXdXR2UnM4T2c0?= =?utf-8?B?c0hMZjV4UFVsS1pJUlh2ZVA3eTkwMDdIRllicWlyRDd4WXJzN1A1Mlkwc0hj?= =?utf-8?Q?C5NC6wM0bqkm0oH5SMMBXwqYE?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9bac2c3a-e235-45a2-4219-08db199138e1 X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB4301.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Feb 2023 13:39:32.1108 (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: K8lDBOxTBolGnPDoAUW9ITjJehtR4H6BLeiJzccJFDDR7zdG4XYEHc7bTh1jR9ii X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB7681 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 1:17 PM, Jerin Jacob wrote: > On Tue, Feb 28, 2023 at 6:16 PM Ferruh Yigit wrote: >> >> On 2/28/2023 11:29 AM, David Marchand wrote: >>> On Tue, Feb 28, 2023 at 12:05 PM Ferruh Yigit 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") >>>>> >>>>> Signed-off-by: Ankur Dwivedi >>>> >>>> Hi Ankur, >>>> >>>> There is another bug report: https://bugs.dpdk.org/show_bug.cgi?id=1167 >>>> >>>> >>>> As far as I can see that is caused by '__rte_trace_point_register()' is >>>> calling 'register_fn()' [1]. >>>> >>>> At registering trace point stage, most of the pointers can be invalid, >>>> and this can crash other locations too. >>> >>> I remember hitting this issue when running with UBsan. >>> >>>> >>>> Why 'register_fn()' called withing the trace point register? Can we >>>> remove it? >>> >>> IIRC, this is used to evaluate the size of the trace point event. >>> >>> >> >> Yes, as checked with Jerin, it is used to evaluate size and some sanity >> checks fro size. >> >> We need either find a way to calculate size without really reading the >> pointer content during register phase, all convert all pointer tracing >> to emit_ptr(). >> >> I prefer first option if we can. > > Looking at the root of the issue. > > rte_trace_point_emit_* has two variant > a)trace point version - Which will emit the trace > b)trace register version - Which will find the size of trace > automatically with single definition of trace point to make life easy > for defining the trace point > > In this case, conf value is junk, as we are referencing the value at > registration time. Looks like in PPC arch, the stack content comes as > junk at this point and got this issue. > And other arch or other environment that adders is OK and since we're > just _reading_ the value. It is not making any issue. But it is a bug. > > RTE_TRACE_POINT was designed to just use > rte_trace_point_emit_u16(conf->my_value) so that in registration scope > "conf->my_value" will be omitted by compiler. > But there as issue in using bitfiled[1] as trace is not supporting > bitfield. Ankur added a local variable to work around the bitfiled > tracing. > > So couple of options, I can think of > > 1)Remove the bitfiled trace point( There are only some trace point > uses that, Just we need to remove bitfield items from that) > 2)It is possible to have anonymous union of type like u16, u32 for > tracing the the bitfields[2], but that would need change in public > structure > 3) Another option is to have a #define to define the scope of > registration. Something like [3] which is noisy. > > I think, we can just do 1 now for rc2 and (2) or (3) or some other > ideas we can think in next release. > +1 to go for option 1, specially at this phase of the release. Only limited number of traces are affected by this bitfield related issue anyway. btw, this is for the Bug 1167, I thought 1167 & 1162 share same root cause but they don't, so 1162 is still open. > > [1] > ../lib/ethdev/ethdev_trace.h: In function > ‘rte_eth_trace_tx_hairpin_queue_setup’: > ../lib/eal/include/rte_trace_point.h:378:21: error: cannot take > address of bit-field ‘peer_count’ > 378 | memcpy(mem, &(in), sizeof(in)); \ > | ^ > > > [2] > struct abc { > union { > uint32_t val; > struct { > val_5_bit:5 > > } > } > } > > [3] > > [main]dell[dpdk.org] $ git diff > diff --git a/lib/eal/include/rte_trace_point_register.h > b/lib/eal/include/rte_trace_point_register.h > index a9682d3f22..266350b29c 100644 > --- a/lib/eal/include/rte_trace_point_register.h > +++ b/lib/eal/include/rte_trace_point_register.h > @@ -16,6 +16,8 @@ extern "C" { > #include > #include > > +#define RTE_TRACE_SCOPE_IS_REGISTRATION > + > RTE_DECLARE_PER_LCORE(volatile int, trace_point_sz); > > #define RTE_TRACE_POINT_REGISTER(trace, name) \ > diff --git a/lib/ethdev/ethdev_trace.h b/lib/ethdev/ethdev_trace.h > index 53d1a71ff0..ba42c3d10a 100644 > --- a/lib/ethdev/ethdev_trace.h > +++ b/lib/ethdev/ethdev_trace.h > @@ -237,13 +237,23 @@ RTE_TRACE_POINT( > RTE_TRACE_POINT_ARGS(uint16_t port_id, uint16_t rx_queue_id, > uint16_t nb_rx_desc, const struct rte_eth_hairpin_conf *conf, > int ret), > - uint16_t peer_count = conf->peer_count; > - uint8_t tx_explicit = conf->tx_explicit; > - uint8_t manual_bind = conf->manual_bind; > - uint8_t use_locked_device_memory = conf->use_locked_device_memory; > - uint8_t use_rte_memory = conf->use_rte_memory; > - uint8_t force_memory = conf->force_memory; > > + uint16_t peer_count; > + uint8_t tx_explicit; > + uint8_t manual_bind; > + uint8_t use_locked_device_memory; > + uint8_t use_rte_memory; > + uint8_t force_memory; > +#ifdef RTE_TRACE_SCOPE_IS_REGISTRATION > + RTE_SET_USED(conf); > +#else > + peer_count = conf->peer_count; > + tx_explicit = conf->tx_explicit; > + manual_bind = conf->manual_bind; > + use_locked_device_memory = conf->use_locked_device_memory; > + use_rte_memory = conf->use_rte_memory; > + force_memory = conf->force_memory; > +#endif > rte_trace_point_emit_u16(port_id); > rte_trace_point_emit_u16(rx_queue_id); > rte_trace_point_emit_u16(nb_rx_desc); > [main]dell[dpdk.org] $