* [PATCH for-7.1 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c
@ 2022-04-05 20:34 Daniel Henrique Barboza
2022-04-05 20:34 ` [PATCH for-7.1 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c Daniel Henrique Barboza
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Henrique Barboza @ 2022-04-05 20:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, clg, david
Hi,
This is a simple patch to fix 2 Coverity issues in
hw/ppc/spapr_nvdimm.c. Aiming it to 7.1 because it's not critical enough
for 7.0.
Daniel Henrique Barboza (1):
hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c
hw/ppc/spapr_nvdimm.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH for-7.1 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c
2022-04-05 20:34 [PATCH for-7.1 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c Daniel Henrique Barboza
@ 2022-04-05 20:34 ` Daniel Henrique Barboza
2022-04-06 3:03 ` David Gibson
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Henrique Barboza @ 2022-04-05 20:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, clg, david
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 by another part of the code (i.e. hotunplug the device).
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 | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index c4c97da5de..e92d92fdae 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -447,9 +447,19 @@ 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;
+
+ if (!drc) {
+ error_report("papr_scm: Could not find nvdimm device with DRC 0x%u",
+ state->drcidx);
+ return H_HARDWARE;
+ }
+
+ 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 +485,15 @@ 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;
+
+ if (!drc) {
+ error_report("papr_scm: Could not find nvdimm device with DRC 0x%u",
+ state->drcidx);
+ return;
+ }
+
+ s_nvdimm = SPAPR_NVDIMM(drc->dev);
state->hcall_ret = hcall_ret;
QLIST_REMOVE(state, node);
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH for-7.1 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c
2022-04-05 20:34 ` [PATCH for-7.1 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c Daniel Henrique Barboza
@ 2022-04-06 3:03 ` David Gibson
0 siblings, 0 replies; 3+ messages in thread
From: David Gibson @ 2022-04-06 3:03 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: qemu-ppc, qemu-devel, clg
[-- Attachment #1: Type: text/plain, Size: 3005 bytes --]
On Tue, Apr 05, 2022 at 05:34:16PM -0300, Daniel Henrique Barboza 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 by another part of the code (i.e. hotunplug the device).
> spapr_drc_index() would then return NULL in the callbacks.
I'm not entirely clear if you're saying this would only happen due to
a bug elsewhere in the code, or if there's some unusual race case or
set of guest/user actions that could trigger this.
>
> Fixes: Coverity CID 1487108, 1487178
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
> hw/ppc/spapr_nvdimm.c | 26 ++++++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
> index c4c97da5de..e92d92fdae 100644
> --- a/hw/ppc/spapr_nvdimm.c
> +++ b/hw/ppc/spapr_nvdimm.c
> @@ -447,9 +447,19 @@ 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;
> +
> + if (!drc) {
> + error_report("papr_scm: Could not find nvdimm device with DRC 0x%u",
> + state->drcidx);
> + return H_HARDWARE;
If this does indicate a bug elswhere in qemu, this should probably be
an assert rather than an H_HARDWARE.
> + }
> +
> + 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 +485,15 @@ 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;
> +
> + if (!drc) {
> + error_report("papr_scm: Could not find nvdimm device with DRC 0x%u",
> + state->drcidx);
> + return;
> + }
> +
> + s_nvdimm = SPAPR_NVDIMM(drc->dev);
>
> state->hcall_ret = hcall_ret;
> QLIST_REMOVE(state, node);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-06 9:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-05 20:34 [PATCH for-7.1 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c Daniel Henrique Barboza
2022-04-05 20:34 ` [PATCH for-7.1 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c Daniel Henrique Barboza
2022-04-06 3:03 ` David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.