devargs: Fix crash due to global devargs uninitailization from secondary process

Message ID 20220214170811.1192-1-madhuker.mythri@oracle.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series devargs: Fix crash due to global devargs uninitailization from secondary process |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS

Commit Message

Madhuker Mythri Feb. 14, 2022, 5:08 p.m. UTC
  From: Madhuker Mythri <madhuker.mythri@oracle.com>

Failsafe pmd started crashing with global devargs syntax as devargs is
not memset to zero. Access it to in rte_devargs_parse() resulted in a
crash when called from secondary process.

Bugzilla Id: 933

Signed-off-by: Madhuker Mythri <madhuker.mythri@oracle.com>
---
 lib/eal/common/eal_common_devargs.c | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Ferruh Yigit Feb. 14, 2022, 5:33 p.m. UTC | #1
On 2/14/2022 5:08 PM, madhuker.mythri@oracle.com wrote:
> From: Madhuker Mythri <madhuker.mythri@oracle.com>
> 
> Failsafe pmd started crashing with global devargs syntax as devargs is
> not memset to zero. Access it to in rte_devargs_parse() resulted in a
> crash when called from secondary process.
> 
> Bugzilla Id: 933
> 
> Signed-off-by: Madhuker Mythri <madhuker.mythri@oracle.com>

This is duplication of Gaetan's patch:
https://patches.dpdk.org/project/dpdk/patch/20220210170131.2199922-1-grive@u256.net/

> ---
>   lib/eal/common/eal_common_devargs.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c
> index 69004b0a2d..ad60febad9 100644
> --- a/lib/eal/common/eal_common_devargs.c
> +++ b/lib/eal/common/eal_common_devargs.c
> @@ -190,6 +190,7 @@ rte_devargs_parse(struct rte_devargs *da, const char *dev)
>   
>   	if (da == NULL)
>   		return -EINVAL;
> +	memset(da, 0, sizeof(*da));
>   
>   	/* First parse according global device syntax. */
>   	if (rte_devargs_layers_parse(da, dev) == 0) {
  
Madhuker Mythri Feb. 15, 2022, 11:20 a.m. UTC | #2
>-----Original Message-----
>From: Ferruh Yigit <ferruh.yigit@intel.com> 
>Sent: 14 फरवरी 2022 23:03
>To: Madhuker Mythri <madhuker.mythri@oracle.com>
>Cc: dev@dpdk.org; Gaetan Rivet <grive@u256.net>; Thomas Monjalon <thomas@monjalon.net>; David Marchand <david.marchand@redhat.com>
>Subject: [External] : Re: [PATCH] devargs: Fix crash due to global devargs uninitailization from secondary process
>
>On 2/14/2022 5:08 PM, madhuker.mythri@oracle.com wrote:
>> From: Madhuker Mythri <madhuker.mythri@oracle.com>
>> 
>> Failsafe pmd started crashing with global devargs syntax as devargs is 
>> not memset to zero. Access it to in rte_devargs_parse() resulted in a 
>> crash when called from secondary process.
>> 
>> Bugzilla Id: 933
>> 
>> Signed-off-by: Madhuker Mythri <madhuker.mythri@oracle.com>
>
>This is duplication of Gaetan's patch:
>https://urldefense.com/v3/__https://patches.dpdk.org/project/dpdk/patch/20220210170131.2199922-1->grive@u256.net/__;!!ACWV5N9M2RV99hQ!aE5DMiBds1eptcxnoYR6KWszXGgqYHaQduFAfUDwH4ps-h0eJIQ5Wk2JBZGMFh5DBZSZ$ 

Hi Ferruh,

Initially, I had filed this bug: https://bugs.dpdk.org/show_bug.cgi?id=933 , as part of the testing on Azure/Hyper-V platforms.
After analysis, found the root-cause of this crash is due to global devargs un-initialization in failsafe-pmd probe() and published the patch as follows:
https://patchwork.dpdk.org/project/dpdk/patch/20220210071052.527-1-madhuker.mythri@oracle.com/

From your review comments got to know, that we can memset inside the rte_devargs_parse(), instead of setting outside before this API call.
Got delayed in testing the same code changes on Azure/Hyper-V platforms and publishing this patch.
So, please consider this patch, as I had found the root-cause of this issue and tested on affected platforms.

Thanks,
Madhuker.
>> ---
>>   lib/eal/common/eal_common_devargs.c | 1 +
>>   1 file changed, 1 insertion(+)
>> 
>> diff --git a/lib/eal/common/eal_common_devargs.c 
>> b/lib/eal/common/eal_common_devargs.c
>> index 69004b0a2d..ad60febad9 100644
>> --- a/lib/eal/common/eal_common_devargs.c
>> +++ b/lib/eal/common/eal_common_devargs.c
>> @@ -190,6 +190,7 @@ rte_devargs_parse(struct rte_devargs *da, const 
>> char *dev)
>>   
>>   	if (da == NULL)
>>   		return -EINVAL;
>> +	memset(da, 0, sizeof(*da));
>>   
>>   	/* First parse according global device syntax. */
>>   	if (rte_devargs_layers_parse(da, dev) == 0) {
  
Thomas Monjalon Feb. 27, 2022, 6:31 p.m. UTC | #3
15/02/2022 12:20, Madhuker Mythri:
> From: Ferruh Yigit <ferruh.yigit@intel.com> 
> >On 2/14/2022 5:08 PM, madhuker.mythri@oracle.com wrote:
> >> From: Madhuker Mythri <madhuker.mythri@oracle.com>
> >> 
> >> Failsafe pmd started crashing with global devargs syntax as devargs is 
> >> not memset to zero. Access it to in rte_devargs_parse() resulted in a 
> >> crash when called from secondary process.
> >> 
> >> Bugzilla Id: 933
> >> 
> >> Signed-off-by: Madhuker Mythri <madhuker.mythri@oracle.com>
> >
> >This is duplication of Gaetan's patch:
> >https://urldefense.com/v3/__https://patches.dpdk.org/project/dpdk/patch/20220210170131.2199922-1->grive@u256.net/__;!!ACWV5N9M2RV99hQ!aE5DMiBds1eptcxnoYR6KWszXGgqYHaQduFAfUDwH4ps-h0eJIQ5Wk2JBZGMFh5DBZSZ$ 
> 
> Hi Ferruh,
> 
> Initially, I had filed this bug: https://bugs.dpdk.org/show_bug.cgi?id=933 , as part of the testing on Azure/Hyper-V platforms.
> After analysis, found the root-cause of this crash is due to global devargs un-initialization in failsafe-pmd probe() and published the patch as follows:
> https://patchwork.dpdk.org/project/dpdk/patch/20220210071052.527-1-madhuker.mythri@oracle.com/
> 
> From your review comments got to know, that we can memset inside the rte_devargs_parse(), instead of setting outside before this API call.
> Got delayed in testing the same code changes on Azure/Hyper-V platforms and publishing this patch.
> So, please consider this patch, as I had found the root-cause of this issue and tested on affected platforms.
> 
> Thanks,
> Madhuker.

Applied with these explanations from Gaetan:

devargs: fix crash with uninitialized parsing

The function rte_devargs_parse() previously was safe to call with
non-initialized devargs structure as parameter.

When adding the support for the global device syntax,
this assumption was broken.
Restore it by forcing memset as part of the call itself.

Bugzilla ID: 933
Fixes: b344eb5d941a ("devargs: parse global device syntax")
Cc: stable@dpdk.org

Signed-off-by: Madhuker Mythri <madhuker.mythri@oracle.com>
Signed-off-by: Gaetan Rivet <grive@u256.net>

Thanks all
  

Patch

diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c
index 69004b0a2d..ad60febad9 100644
--- a/lib/eal/common/eal_common_devargs.c
+++ b/lib/eal/common/eal_common_devargs.c
@@ -190,6 +190,7 @@  rte_devargs_parse(struct rte_devargs *da, const char *dev)
 
 	if (da == NULL)
 		return -EINVAL;
+	memset(da, 0, sizeof(*da));
 
 	/* First parse according global device syntax. */
 	if (rte_devargs_layers_parse(da, dev) == 0) {