[PATCH 0/1] module: Prevent crash by resetting local_err in module_load_qom_all()

[PATCH 0/1] module: Prevent crash by resetting local_err in module_load_qom_all()

[Alexander Ivanov]

After updating QEMU modules previously executed QEMU processes crash
on module loading. It happens because error_setg() calls with a not NULL
errp argument.

There is a discussion - https://issues.redhat.com/browse/RHEL-29848

Alexander Ivanov (1):
  module: Prevent crash by resetting local_err in module_load_qom_all()

 util/module.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.43.0

[PATCH 1/1] module: Prevent crash by resetting local_err in module_load_qom_all()

[Alexander Ivanov]

Set local_err to NULL after it has been freed in error_report_err(). This
avoids triggering assert(*errp == NULL) failure in error_setv() when
local_err is reused in the loop.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 util/module.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/module.c b/util/module.c
index 32e263163c..3ff4aee2ca 100644
--- a/util/module.c
+++ b/util/module.c
@@ -369,6 +369,7 @@ void module_load_qom_all(void)
         }
         if (module_load("", modinfo->name, &local_err) < 0) {
             error_report_err(local_err);
+            local_err = NULL;
         }
     }
     module_loaded_qom_all = true;
-- 
2.43.0

Re: [PATCH 1/1] module: Prevent crash by resetting local_err in module_load_qom_all()

[Denis V. Lunev]

On 8/9/24 14:13, Alexander Ivanov wrote:
> Set local_err to NULL after it has been freed in error_report_err(). This
> avoids triggering assert(*errp == NULL) failure in error_setv() when
> local_err is reused in the loop.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>   util/module.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/util/module.c b/util/module.c
> index 32e263163c..3ff4aee2ca 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -369,6 +369,7 @@ void module_load_qom_all(void)
>           }
>           if (module_load("", modinfo->name, &local_err) < 0) {
>               error_report_err(local_err);
> +            local_err = NULL;
>           }
>       }
>       module_loaded_qom_all = true;
Reviewed-by: Denis V. Lunev <den@openvz.org>

Re: [PATCH 1/1] module: Prevent crash by resetting local_err in module_load_qom_all()

[Claudio Fontana]

On 8/9/24 14:13, Alexander Ivanov wrote:
> Set local_err to NULL after it has been freed in error_report_err(). This
> avoids triggering assert(*errp == NULL) failure in error_setv() when
> local_err is reused in the loop.
> 
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>  util/module.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/util/module.c b/util/module.c
> index 32e263163c..3ff4aee2ca 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -369,6 +369,7 @@ void module_load_qom_all(void)
>          }
>          if (module_load("", modinfo->name, &local_err) < 0) {
>              error_report_err(local_err);
> +            local_err = NULL;
>          }
>      }
>      module_loaded_qom_all = true;

Reviewed-by: Claudio Fontana <cfontana@suse.de>

Re: [PATCH 1/1] module: Prevent crash by resetting local_err in module_load_qom_all()

[Richard Henderson]

On 8/9/24 22:13, Alexander Ivanov wrote:
> Set local_err to NULL after it has been freed in error_report_err(). This
> avoids triggering assert(*errp == NULL) failure in error_setv() when
> local_err is reused in the loop.
> 
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>   util/module.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/util/module.c b/util/module.c
> index 32e263163c..3ff4aee2ca 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -369,6 +369,7 @@ void module_load_qom_all(void)
>           }
>           if (module_load("", modinfo->name, &local_err) < 0) {
>               error_report_err(local_err);
> +            local_err = NULL;
>           }
>       }
>       module_loaded_qom_all = true;

Better, IMO, to move local_err and its initialization into the loop.


r~

Re: [PATCH 1/1] module: Prevent crash by resetting local_err in module_load_qom_all()

[Alexander Ivanov]

On 8/12/24 00:05, Richard Henderson wrote:
> On 8/9/24 22:13, Alexander Ivanov wrote:
>> Set local_err to NULL after it has been freed in error_report_err(). 
>> This
>> avoids triggering assert(*errp == NULL) failure in error_setv() when
>> local_err is reused in the loop.
>>
>> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
>> ---
>>   util/module.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/util/module.c b/util/module.c
>> index 32e263163c..3ff4aee2ca 100644
>> --- a/util/module.c
>> +++ b/util/module.c
>> @@ -369,6 +369,7 @@ void module_load_qom_all(void)
>>           }
>>           if (module_load("", modinfo->name, &local_err) < 0) {
>>               error_report_err(local_err);
>> +            local_err = NULL;
>>           }
>>       }
>>       module_loaded_qom_all = true;
>
> Better, IMO, to move local_err and its initialization into the loop.
Yes, I agree, it makes sense.
>
>
> r~

-- 
Best regards,
Alexander Ivanov

Re: [PATCH 0/1] module: Prevent crash by resetting local_err in module_load_qom_all()

[Paolo Bonzini]

Queued, thanks.

Paolo