[PATCH] devargs: Fix crash due to global devargs uninitailization from secondary process

[PATCH] devargs: Fix crash due to global devargs uninitailization from secondary process

[madhuker.mythri]

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(+)

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) {
-- 
2.32.0.windows.1

Re: [PATCH] devargs: Fix crash due to global devargs uninitailization from secondary process

[Ferruh Yigit]

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) {

RE: [External] : Re: [PATCH] devargs: Fix crash due to global devargs uninitailization from secondary process

[Madhuker Mythri]

>-----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) {

Re: [External] : Re: [PATCH] devargs: Fix crash due to global devargs uninitailization from secondary process

[Thomas Monjalon]

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