| Message ID | 20230406154347.4100700-1-stefanb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | util/error: Fix use-after-free errors reported by Coverity | expand |
On Thu, 6 Apr 2023 at 16:43, Stefan Berger <stefanb@linux.ibm.com> wrote: > > Fix use-after-free errors in the code path that called error_handle(). A > call to error_handle() will now either free the passed Error 'err' or > assign it to '*errp' if '*errp' is currently NULL. This ensures that 'err' > either has been freed or is assigned to '*errp' if this function returns. > Adjust the two callers of this function to not assign the 'err' to '*errp' > themselves, since this is now handled by error_handle(). > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Do we think this needs to be fixed for 8.0 ? thanks -- PMM
On 6/4/23 17:43, Stefan Berger wrote: > Fix use-after-free errors in the code path that called error_handle(). A > call to error_handle() will now either free the passed Error 'err' or > assign it to '*errp' if '*errp' is currently NULL. This ensures that 'err' > either has been freed or is assigned to '*errp' if this function returns. > Adjust the two callers of this function to not assign the 'err' to '*errp' > themselves, since this is now handled by error_handle(). > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > util/error.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Hi On Thu, Apr 6, 2023 at 7:43 PM Stefan Berger <stefanb@linux.ibm.com> wrote: > > Fix use-after-free errors in the code path that called error_handle(). A > call to error_handle() will now either free the passed Error 'err' or > assign it to '*errp' if '*errp' is currently NULL. This ensures that 'err' > either has been freed or is assigned to '*errp' if this function returns. > Adjust the two callers of this function to not assign the 'err' to '*errp' > themselves, since this is now handled by error_handle(). > Fixes: commit 3ffef1a55ca3 ("error: add global &error_warn destination") Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> worth including for 8.0 imho. > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > util/error.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/util/error.c b/util/error.c > index 5537245da6..e5e247209a 100644 > --- a/util/error.c > +++ b/util/error.c > @@ -46,6 +46,10 @@ static void error_handle(Error **errp, Error *err) > } > if (errp == &error_warn) { > warn_report_err(err); > + } else if (errp && !*errp) { > + *errp = err; > + } else { > + error_free(err); > } > } > > @@ -76,7 +80,6 @@ static void error_setv(Error **errp, > err->func = func; > > error_handle(errp, err); > - *errp = err; > > errno = saved_errno; > } > @@ -289,11 +292,6 @@ void error_propagate(Error **dst_errp, Error *local_err) > return; > } > error_handle(dst_errp, local_err); > - if (dst_errp && !*dst_errp) { > - *dst_errp = local_err; > - } else { > - error_free(local_err); > - } > } > > void error_propagate_prepend(Error **dst_errp, Error *err, > -- > 2.39.1 >
diff --git a/util/error.c b/util/error.c index 5537245da6..e5e247209a 100644 --- a/util/error.c +++ b/util/error.c @@ -46,6 +46,10 @@ static void error_handle(Error **errp, Error *err) } if (errp == &error_warn) { warn_report_err(err); + } else if (errp && !*errp) { + *errp = err; + } else { + error_free(err); } } @@ -76,7 +80,6 @@ static void error_setv(Error **errp, err->func = func; error_handle(errp, err); - *errp = err; errno = saved_errno; } @@ -289,11 +292,6 @@ void error_propagate(Error **dst_errp, Error *local_err) return; } error_handle(dst_errp, local_err); - if (dst_errp && !*dst_errp) { - *dst_errp = local_err; - } else { - error_free(local_err); - } } void error_propagate_prepend(Error **dst_errp, Error *err,
Fix use-after-free errors in the code path that called error_handle(). A call to error_handle() will now either free the passed Error 'err' or assign it to '*errp' if '*errp' is currently NULL. This ensures that 'err' either has been freed or is assigned to '*errp' if this function returns. Adjust the two callers of this function to not assign the 'err' to '*errp' themselves, since this is now handled by error_handle(). Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- util/error.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)