* [PATCH v2 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c
@ 2022-04-09 20:08 Daniel Henrique Barboza
2022-04-09 20:08 ` [PATCH v2 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c Daniel Henrique Barboza
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Daniel Henrique Barboza @ 2022-04-09 20:08 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, clg, david
Changes from v1:
- clarified in the commit message which kind of errors we aim to prevent
- changed the H_HARDWARE return to g_assert() exit
- v1 link: https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00569.html
Daniel Henrique Barboza (1):
hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c
hw/ppc/spapr_nvdimm.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c
2022-04-09 20:08 [PATCH v2 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c Daniel Henrique Barboza
@ 2022-04-09 20:08 ` Daniel Henrique Barboza
2022-07-20 13:16 ` Greg Kurz
2022-05-04 18:51 ` [PATCH v2 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c Daniel Henrique Barboza
2022-07-20 12:34 ` Peter Maydell
2 siblings, 1 reply; 7+ messages in thread
From: Daniel Henrique Barboza @ 2022-04-09 20:08 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 (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(-)
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);
--
2.35.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c
2022-04-09 20:08 [PATCH v2 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c Daniel Henrique Barboza
2022-04-09 20:08 ` [PATCH v2 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c Daniel Henrique Barboza
@ 2022-05-04 18:51 ` Daniel Henrique Barboza
2022-07-20 12:34 ` Peter Maydell
2 siblings, 0 replies; 7+ messages in thread
From: Daniel Henrique Barboza @ 2022-05-04 18:51 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david, clg
Ping
On 4/9/22 17:08, Daniel Henrique Barboza wrote:
> Changes from v1:
> - clarified in the commit message which kind of errors we aim to prevent
> - changed the H_HARDWARE return to g_assert() exit
> - v1 link: https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00569.html
>
> Daniel Henrique Barboza (1):
> hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c
>
> hw/ppc/spapr_nvdimm.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c
2022-04-09 20:08 [PATCH v2 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c Daniel Henrique Barboza
2022-04-09 20:08 ` [PATCH v2 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c Daniel Henrique Barboza
2022-05-04 18:51 ` [PATCH v2 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c Daniel Henrique Barboza
@ 2022-07-20 12:34 ` Peter Maydell
2022-07-20 12:55 ` Daniel Henrique Barboza
2 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2022-07-20 12:34 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: qemu-devel, qemu-ppc, clg, david, Greg Kurz
On Sat, 9 Apr 2022 at 21:11, Daniel Henrique Barboza
<danielhb413@gmail.com> wrote:
>
> Changes from v1:
> - clarified in the commit message which kind of errors we aim to prevent
> - changed the H_HARDWARE return to g_assert() exit
> - v1 link: https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00569.html
>
> Daniel Henrique Barboza (1):
> hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c
>
> hw/ppc/spapr_nvdimm.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
Hi -- I was just going through old-but-unclosed Coverity issues, and
it looks like this patch fixing one got lost somewhere along the line.
Could somebody pick it up for the ppc tree, please?
thanks
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c
2022-07-20 12:34 ` Peter Maydell
@ 2022-07-20 12:55 ` Daniel Henrique Barboza
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Henrique Barboza @ 2022-07-20 12:55 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, qemu-ppc, clg, david, Greg Kurz
On 7/20/22 09:34, Peter Maydell wrote:
> On Sat, 9 Apr 2022 at 21:11, Daniel Henrique Barboza
> <danielhb413@gmail.com> wrote:
>>
>> Changes from v1:
>> - clarified in the commit message which kind of errors we aim to prevent
>> - changed the H_HARDWARE return to g_assert() exit
>> - v1 link: https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00569.html
>>
>> Daniel Henrique Barboza (1):
>> hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c
>>
>> hw/ppc/spapr_nvdimm.c | 18 ++++++++++++++----
>> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> Hi -- I was just going through old-but-unclosed Coverity issues, and
> it looks like this patch fixing one got lost somewhere along the line.
> Could somebody pick it up for the ppc tree, please?
I can pick it for the next PR. Just need someone else to ack it.
Thanks,
Daniel
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c
2022-04-09 20:08 ` [PATCH v2 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c Daniel Henrique Barboza
@ 2022-07-20 13:16 ` Greg Kurz
2022-07-20 13:58 ` Daniel Henrique Barboza
0 siblings, 1 reply; 7+ messages in thread
From: Greg Kurz @ 2022-07-20 13:16 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: qemu-devel, qemu-ppc, clg, david
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);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c
2022-07-20 13:16 ` Greg Kurz
@ 2022-07-20 13:58 ` Daniel Henrique Barboza
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Henrique Barboza @ 2022-07-20 13:58 UTC (permalink / raw)
To: Greg Kurz; +Cc: qemu-devel, qemu-ppc, clg, david
On 7/20/22 10:16, Greg Kurz wrote:
> 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>
Thanks Greg! I'll queue this up as an eligible fix for the soft freeze.
Daniel
>
>> 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);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-07-20 14:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-09 20:08 [PATCH v2 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c Daniel Henrique Barboza
2022-04-09 20:08 ` [PATCH v2 1/1] hw/ppc: check if spapr_drc_index() returns NULL in spapr_nvdimm.c Daniel Henrique Barboza
2022-07-20 13:16 ` Greg Kurz
2022-07-20 13:58 ` Daniel Henrique Barboza
2022-05-04 18:51 ` [PATCH v2 0/1] Coverity fixes in hw/ppc/spapr_nvdimm.c Daniel Henrique Barboza
2022-07-20 12:34 ` Peter Maydell
2022-07-20 12:55 ` Daniel Henrique Barboza
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.