Re: [PATCH 2/6] block/mirror: fix use after free of local_err

[Max Reitz]

On 24.03.20 16:36, Vladimir Sementsov-Ogievskiy wrote:
> local_err is used again in mirror_exit_common() after
> bdrv_set_backing_hd(), so we must zero it. Otherwise try to set
> non-NULL local_err will crash.

OK, but wouldn’t it be better hygiene to set it to NULL every time it is
freed?  (There is a second instance of error_report_err() in this
function.  I’m a bit worried we might introduce another local_err use
after that one at some point in the future, and forget to run the cocci
script then.)

Are the cocci scripts run regularly by someone?  E.g. as part of a pull
to master?

Max

> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  block/mirror.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 447051dbc6..6203e5946e 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -678,6 +678,7 @@ static int mirror_exit_common(Job *job)
>              bdrv_set_backing_hd(target_bs, backing, &local_err);
>              if (local_err) {
>                  error_report_err(local_err);
> +                local_err = NULL;
>                  ret = -EPERM;
>              }
>          }
>

signature.asc
Description: OpenPGP digital signature