diff mbox series

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

Message ID 20240809121340.992049-2-alexander.ivanov@virtuozzo.com
State New
Headers show
Series module: Prevent crash by resetting local_err in module_load_qom_all() | expand

Commit Message

Alexander Ivanov Aug. 9, 2024, 12:13 p.m. UTC 
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(+)

Comments

Denis V. Lunev Aug. 9, 2024, 12:14 p.m. UTC | #1 
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>
Claudio Fontana Aug. 9, 2024, 1:22 p.m. UTC | #2 
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>
Richard Henderson Aug. 11, 2024, 10:05 p.m. UTC | #3 
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~
Alexander Ivanov Aug. 12, 2024, 7:59 a.m. UTC | #4 
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~
diff mbox series

Patch

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;