[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/1] hw/ppc: check if spapr_drc_index() returns NULL in sp
From: |
Greg Kurz |
Subject: |
Re: [PATCH v2 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c |
Date: |
Wed, 20 Jul 2022 15:16:49 +0200 |
On Sat, 9 Apr 2022 17:08:56 -0300
Daniel Henrique Barboza <danielhb413@gmail.com> wrote:
> spapr_nvdimm_flush_completion_cb() and flush_worker_cb() are using the
> DRC object returned by spapr_drc_index() without checking it for NULL.
> In this case we would be dereferencing a NULL pointer when doing
> SPAPR_NVDIMM(drc->dev) and PC_DIMM(drc->dev).
>
> This can happen if, during a scm_flush(), the DRC object is wrongly
> freed/released (e.g. a bug in another part of the code).
> spapr_drc_index() would then return NULL in the callbacks.
>
> Fixes: Coverity CID 1487108, 1487178
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
> hw/ppc/spapr_nvdimm.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
LGTM
Reviewed-by: Greg Kurz <groug@kaod.org>
> diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
> index c4c97da5de..04a64cada3 100644
> --- a/hw/ppc/spapr_nvdimm.c
> +++ b/hw/ppc/spapr_nvdimm.c
> @@ -447,9 +447,15 @@ static int flush_worker_cb(void *opaque)
> {
> SpaprNVDIMMDeviceFlushState *state = opaque;
> SpaprDrc *drc = spapr_drc_by_index(state->drcidx);
> - PCDIMMDevice *dimm = PC_DIMM(drc->dev);
> - HostMemoryBackend *backend = MEMORY_BACKEND(dimm->hostmem);
> - int backend_fd = memory_region_get_fd(&backend->mr);
> + PCDIMMDevice *dimm;
> + HostMemoryBackend *backend;
> + int backend_fd;
> +
> + g_assert(drc != NULL);
> +
> + dimm = PC_DIMM(drc->dev);
> + backend = MEMORY_BACKEND(dimm->hostmem);
> + backend_fd = memory_region_get_fd(&backend->mr);
>
> if (object_property_get_bool(OBJECT(backend), "pmem", NULL)) {
> MemoryRegion *mr = host_memory_backend_get_memory(dimm->hostmem);
> @@ -475,7 +481,11 @@ static void spapr_nvdimm_flush_completion_cb(void
> *opaque, int hcall_ret)
> {
> SpaprNVDIMMDeviceFlushState *state = opaque;
> SpaprDrc *drc = spapr_drc_by_index(state->drcidx);
> - SpaprNVDIMMDevice *s_nvdimm = SPAPR_NVDIMM(drc->dev);
> + SpaprNVDIMMDevice *s_nvdimm;
> +
> + g_assert(drc != NULL);
> +
> + s_nvdimm = SPAPR_NVDIMM(drc->dev);
>
> state->hcall_ret = hcall_ret;
> QLIST_REMOVE(state, node);
- Re: [PATCH v2 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c,
Greg Kurz <=