* [PATCH 0/2] ppc/pnv: PNOR fixes @ 2020-01-07 17:18 Cédric Le Goater 2020-01-07 17:18 ` [PATCH 1/2] ppc/pnv: check return value of blk_pwrite() Cédric Le Goater 2020-01-07 17:18 ` [PATCH 2/2] ppc/pnv: fix check on return value of blk_getlength() Cédric Le Goater 0 siblings, 2 replies; 9+ messages in thread From: Cédric Le Goater @ 2020-01-07 17:18 UTC (permalink / raw) To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater Hello, Coverity found a couple of issues in the PnvPNOR model. Here are the fixes. Thanks, C. Cédric Le Goater (2): ppc/pnv: check return value of blk_pwrite() ppc/pnv: fix check on return value of blk_getlength() include/hw/ppc/pnv_pnor.h | 2 +- hw/ppc/pnv_pnor.c | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) -- 2.21.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] ppc/pnv: check return value of blk_pwrite() 2020-01-07 17:18 [PATCH 0/2] ppc/pnv: PNOR fixes Cédric Le Goater @ 2020-01-07 17:18 ` Cédric Le Goater 2020-01-07 17:40 ` Greg Kurz 2020-01-07 18:22 ` Philippe Mathieu-Daudé 2020-01-07 17:18 ` [PATCH 2/2] ppc/pnv: fix check on return value of blk_getlength() Cédric Le Goater 1 sibling, 2 replies; 9+ messages in thread From: Cédric Le Goater @ 2020-01-07 17:18 UTC (permalink / raw) To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater When updating the PNOR file contents, we should check for a possible failure of blk_pwrite(). Fixes Coverity issue CID 1412228. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/ppc/pnv_pnor.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/ppc/pnv_pnor.c b/hw/ppc/pnv_pnor.c index bfb1e92b0392..0e86ae2feae6 100644 --- a/hw/ppc/pnv_pnor.c +++ b/hw/ppc/pnv_pnor.c @@ -33,6 +33,7 @@ static uint64_t pnv_pnor_read(void *opaque, hwaddr addr, unsigned size) static void pnv_pnor_update(PnvPnor *s, int offset, int size) { int offset_end; + int ret; if (s->blk) { return; @@ -42,8 +43,11 @@ static void pnv_pnor_update(PnvPnor *s, int offset, int size) offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE); offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE); - blk_pwrite(s->blk, offset, s->storage + offset, - offset_end - offset, 0); + ret = blk_pwrite(s->blk, offset, s->storage + offset, + offset_end - offset, 0); + if (ret < 0) { + error_report("Could not update PNOR: %s", strerror(-ret)); + } } static void pnv_pnor_write(void *opaque, hwaddr addr, uint64_t data, -- 2.21.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ppc/pnv: check return value of blk_pwrite() 2020-01-07 17:18 ` [PATCH 1/2] ppc/pnv: check return value of blk_pwrite() Cédric Le Goater @ 2020-01-07 17:40 ` Greg Kurz 2020-01-07 18:22 ` Philippe Mathieu-Daudé 1 sibling, 0 replies; 9+ messages in thread From: Greg Kurz @ 2020-01-07 17:40 UTC (permalink / raw) To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, David Gibson On Tue, 7 Jan 2020 18:18:08 +0100 Cédric Le Goater <clg@kaod.org> wrote: > When updating the PNOR file contents, we should check for a possible > failure of blk_pwrite(). > > Fixes Coverity issue CID 1412228. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- Reviewed-by: Greg Kurz <groug@kaod.org> > hw/ppc/pnv_pnor.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/pnv_pnor.c b/hw/ppc/pnv_pnor.c > index bfb1e92b0392..0e86ae2feae6 100644 > --- a/hw/ppc/pnv_pnor.c > +++ b/hw/ppc/pnv_pnor.c > @@ -33,6 +33,7 @@ static uint64_t pnv_pnor_read(void *opaque, hwaddr addr, unsigned size) > static void pnv_pnor_update(PnvPnor *s, int offset, int size) > { > int offset_end; > + int ret; > > if (s->blk) { > return; > @@ -42,8 +43,11 @@ static void pnv_pnor_update(PnvPnor *s, int offset, int size) > offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE); > offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE); > > - blk_pwrite(s->blk, offset, s->storage + offset, > - offset_end - offset, 0); > + ret = blk_pwrite(s->blk, offset, s->storage + offset, > + offset_end - offset, 0); > + if (ret < 0) { > + error_report("Could not update PNOR: %s", strerror(-ret)); > + } > } > > static void pnv_pnor_write(void *opaque, hwaddr addr, uint64_t data, ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ppc/pnv: check return value of blk_pwrite() 2020-01-07 17:18 ` [PATCH 1/2] ppc/pnv: check return value of blk_pwrite() Cédric Le Goater 2020-01-07 17:40 ` Greg Kurz @ 2020-01-07 18:22 ` Philippe Mathieu-Daudé 2020-01-08 0:56 ` David Gibson 1 sibling, 1 reply; 9+ messages in thread From: Philippe Mathieu-Daudé @ 2020-01-07 18:22 UTC (permalink / raw) To: Cédric Le Goater, David Gibson; +Cc: qemu-ppc, qemu-devel On 1/7/20 6:18 PM, Cédric Le Goater wrote: > When updating the PNOR file contents, we should check for a possible > failure of blk_pwrite(). > > Fixes Coverity issue CID 1412228. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/ppc/pnv_pnor.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/pnv_pnor.c b/hw/ppc/pnv_pnor.c > index bfb1e92b0392..0e86ae2feae6 100644 > --- a/hw/ppc/pnv_pnor.c > +++ b/hw/ppc/pnv_pnor.c > @@ -33,6 +33,7 @@ static uint64_t pnv_pnor_read(void *opaque, hwaddr addr, unsigned size) > static void pnv_pnor_update(PnvPnor *s, int offset, int size) > { > int offset_end; > + int ret; > > if (s->blk) { > return; > @@ -42,8 +43,11 @@ static void pnv_pnor_update(PnvPnor *s, int offset, int size) > offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE); > offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE); > > - blk_pwrite(s->blk, offset, s->storage + offset, > - offset_end - offset, 0); > + ret = blk_pwrite(s->blk, offset, s->storage + offset, > + offset_end - offset, 0); > + if (ret < 0) { > + error_report("Could not update PNOR: %s", strerror(-ret)); Can you report the failed offset too please? Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > + } > } > > static void pnv_pnor_write(void *opaque, hwaddr addr, uint64_t data, > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ppc/pnv: check return value of blk_pwrite() 2020-01-07 18:22 ` Philippe Mathieu-Daudé @ 2020-01-08 0:56 ` David Gibson 0 siblings, 0 replies; 9+ messages in thread From: David Gibson @ 2020-01-08 0:56 UTC (permalink / raw) To: Philippe Mathieu-Daudé; +Cc: qemu-ppc, Cédric Le Goater, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1930 bytes --] On Tue, Jan 07, 2020 at 07:22:18PM +0100, Philippe Mathieu-Daudé wrote: > On 1/7/20 6:18 PM, Cédric Le Goater wrote: > > When updating the PNOR file contents, we should check for a possible > > failure of blk_pwrite(). > > > > Fixes Coverity issue CID 1412228. > > > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > > --- > > hw/ppc/pnv_pnor.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/hw/ppc/pnv_pnor.c b/hw/ppc/pnv_pnor.c > > index bfb1e92b0392..0e86ae2feae6 100644 > > --- a/hw/ppc/pnv_pnor.c > > +++ b/hw/ppc/pnv_pnor.c > > @@ -33,6 +33,7 @@ static uint64_t pnv_pnor_read(void *opaque, hwaddr addr, unsigned size) > > static void pnv_pnor_update(PnvPnor *s, int offset, int size) > > { > > int offset_end; > > + int ret; > > if (s->blk) { > > return; > > @@ -42,8 +43,11 @@ static void pnv_pnor_update(PnvPnor *s, int offset, int size) > > offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE); > > offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE); > > - blk_pwrite(s->blk, offset, s->storage + offset, > > - offset_end - offset, 0); > > + ret = blk_pwrite(s->blk, offset, s->storage + offset, > > + offset_end - offset, 0); > > + if (ret < 0) { > > + error_report("Could not update PNOR: %s", strerror(-ret)); > > Can you report the failed offset too please? Since this fixes a coverity warning, I'm applying now, and extending the error message can be a followup. > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > > + } > > } > > static void pnv_pnor_write(void *opaque, hwaddr addr, uint64_t data, > > > -- 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] 9+ messages in thread
* [PATCH 2/2] ppc/pnv: fix check on return value of blk_getlength() 2020-01-07 17:18 [PATCH 0/2] ppc/pnv: PNOR fixes Cédric Le Goater 2020-01-07 17:18 ` [PATCH 1/2] ppc/pnv: check return value of blk_pwrite() Cédric Le Goater @ 2020-01-07 17:18 ` Cédric Le Goater 2020-01-07 17:42 ` Greg Kurz ` (2 more replies) 1 sibling, 3 replies; 9+ messages in thread From: Cédric Le Goater @ 2020-01-07 17:18 UTC (permalink / raw) To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater blk_getlength() returns an int64_t but the result is stored in a uint32_t. Errors (negative values) won't be caught by the check in pnv_pnor_realize() and blk_blockalign() will allocate a very large buffer in such cases. Fixes Coverity issue CID 1412226. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- include/hw/ppc/pnv_pnor.h | 2 +- hw/ppc/pnv_pnor.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/hw/ppc/pnv_pnor.h b/include/hw/ppc/pnv_pnor.h index c3dd28643cae..4f96abdfb402 100644 --- a/include/hw/ppc/pnv_pnor.h +++ b/include/hw/ppc/pnv_pnor.h @@ -23,7 +23,7 @@ typedef struct PnvPnor { BlockBackend *blk; uint8_t *storage; - uint32_t size; + int64_t size; MemoryRegion mmio; } PnvPnor; diff --git a/hw/ppc/pnv_pnor.c b/hw/ppc/pnv_pnor.c index 0e86ae2feae6..b061106d1c0c 100644 --- a/hw/ppc/pnv_pnor.c +++ b/hw/ppc/pnv_pnor.c @@ -111,7 +111,7 @@ static void pnv_pnor_realize(DeviceState *dev, Error **errp) } static Property pnv_pnor_properties[] = { - DEFINE_PROP_UINT32("size", PnvPnor, size, 128 << 20), + DEFINE_PROP_INT64("size", PnvPnor, size, 128 << 20), DEFINE_PROP_DRIVE("drive", PnvPnor, blk), DEFINE_PROP_END_OF_LIST(), }; -- 2.21.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ppc/pnv: fix check on return value of blk_getlength() 2020-01-07 17:18 ` [PATCH 2/2] ppc/pnv: fix check on return value of blk_getlength() Cédric Le Goater @ 2020-01-07 17:42 ` Greg Kurz 2020-01-07 18:23 ` Philippe Mathieu-Daudé 2020-01-08 1:01 ` David Gibson 2 siblings, 0 replies; 9+ messages in thread From: Greg Kurz @ 2020-01-07 17:42 UTC (permalink / raw) To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, David Gibson On Tue, 7 Jan 2020 18:18:09 +0100 Cédric Le Goater <clg@kaod.org> wrote: > blk_getlength() returns an int64_t but the result is stored in a > uint32_t. Errors (negative values) won't be caught by the check in > pnv_pnor_realize() and blk_blockalign() will allocate a very large > buffer in such cases. > > Fixes Coverity issue CID 1412226. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- Reviewed-by: Greg Kurz <groug@kaod.org> > include/hw/ppc/pnv_pnor.h | 2 +- > hw/ppc/pnv_pnor.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/hw/ppc/pnv_pnor.h b/include/hw/ppc/pnv_pnor.h > index c3dd28643cae..4f96abdfb402 100644 > --- a/include/hw/ppc/pnv_pnor.h > +++ b/include/hw/ppc/pnv_pnor.h > @@ -23,7 +23,7 @@ typedef struct PnvPnor { > BlockBackend *blk; > > uint8_t *storage; > - uint32_t size; > + int64_t size; > MemoryRegion mmio; > } PnvPnor; > > diff --git a/hw/ppc/pnv_pnor.c b/hw/ppc/pnv_pnor.c > index 0e86ae2feae6..b061106d1c0c 100644 > --- a/hw/ppc/pnv_pnor.c > +++ b/hw/ppc/pnv_pnor.c > @@ -111,7 +111,7 @@ static void pnv_pnor_realize(DeviceState *dev, Error **errp) > } > > static Property pnv_pnor_properties[] = { > - DEFINE_PROP_UINT32("size", PnvPnor, size, 128 << 20), > + DEFINE_PROP_INT64("size", PnvPnor, size, 128 << 20), > DEFINE_PROP_DRIVE("drive", PnvPnor, blk), > DEFINE_PROP_END_OF_LIST(), > }; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ppc/pnv: fix check on return value of blk_getlength() 2020-01-07 17:18 ` [PATCH 2/2] ppc/pnv: fix check on return value of blk_getlength() Cédric Le Goater 2020-01-07 17:42 ` Greg Kurz @ 2020-01-07 18:23 ` Philippe Mathieu-Daudé 2020-01-08 1:01 ` David Gibson 2 siblings, 0 replies; 9+ messages in thread From: Philippe Mathieu-Daudé @ 2020-01-07 18:23 UTC (permalink / raw) To: Cédric Le Goater, David Gibson; +Cc: qemu-ppc, qemu-devel On 1/7/20 6:18 PM, Cédric Le Goater wrote: > blk_getlength() returns an int64_t but the result is stored in a > uint32_t. Errors (negative values) won't be caught by the check in > pnv_pnor_realize() and blk_blockalign() will allocate a very large > buffer in such cases. > > Fixes Coverity issue CID 1412226. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > include/hw/ppc/pnv_pnor.h | 2 +- > hw/ppc/pnv_pnor.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/hw/ppc/pnv_pnor.h b/include/hw/ppc/pnv_pnor.h > index c3dd28643cae..4f96abdfb402 100644 > --- a/include/hw/ppc/pnv_pnor.h > +++ b/include/hw/ppc/pnv_pnor.h > @@ -23,7 +23,7 @@ typedef struct PnvPnor { > BlockBackend *blk; > > uint8_t *storage; > - uint32_t size; > + int64_t size; > MemoryRegion mmio; > } PnvPnor; > > diff --git a/hw/ppc/pnv_pnor.c b/hw/ppc/pnv_pnor.c > index 0e86ae2feae6..b061106d1c0c 100644 > --- a/hw/ppc/pnv_pnor.c > +++ b/hw/ppc/pnv_pnor.c > @@ -111,7 +111,7 @@ static void pnv_pnor_realize(DeviceState *dev, Error **errp) > } > > static Property pnv_pnor_properties[] = { > - DEFINE_PROP_UINT32("size", PnvPnor, size, 128 << 20), > + DEFINE_PROP_INT64("size", PnvPnor, size, 128 << 20), If you respin the series please consider using '128 * MiB' here. Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > DEFINE_PROP_DRIVE("drive", PnvPnor, blk), > DEFINE_PROP_END_OF_LIST(), > }; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ppc/pnv: fix check on return value of blk_getlength() 2020-01-07 17:18 ` [PATCH 2/2] ppc/pnv: fix check on return value of blk_getlength() Cédric Le Goater 2020-01-07 17:42 ` Greg Kurz 2020-01-07 18:23 ` Philippe Mathieu-Daudé @ 2020-01-08 1:01 ` David Gibson 2 siblings, 0 replies; 9+ messages in thread From: David Gibson @ 2020-01-08 1:01 UTC (permalink / raw) To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1668 bytes --] On Tue, Jan 07, 2020 at 06:18:09PM +0100, Cédric Le Goater wrote: > blk_getlength() returns an int64_t but the result is stored in a > uint32_t. Errors (negative values) won't be caught by the check in > pnv_pnor_realize() and blk_blockalign() will allocate a very large > buffer in such cases. > > Fixes Coverity issue CID 1412226. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> Applied to ppc-for-5.0. > --- > include/hw/ppc/pnv_pnor.h | 2 +- > hw/ppc/pnv_pnor.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/hw/ppc/pnv_pnor.h b/include/hw/ppc/pnv_pnor.h > index c3dd28643cae..4f96abdfb402 100644 > --- a/include/hw/ppc/pnv_pnor.h > +++ b/include/hw/ppc/pnv_pnor.h > @@ -23,7 +23,7 @@ typedef struct PnvPnor { > BlockBackend *blk; > > uint8_t *storage; > - uint32_t size; > + int64_t size; > MemoryRegion mmio; > } PnvPnor; > > diff --git a/hw/ppc/pnv_pnor.c b/hw/ppc/pnv_pnor.c > index 0e86ae2feae6..b061106d1c0c 100644 > --- a/hw/ppc/pnv_pnor.c > +++ b/hw/ppc/pnv_pnor.c > @@ -111,7 +111,7 @@ static void pnv_pnor_realize(DeviceState *dev, Error **errp) > } > > static Property pnv_pnor_properties[] = { > - DEFINE_PROP_UINT32("size", PnvPnor, size, 128 << 20), > + DEFINE_PROP_INT64("size", PnvPnor, size, 128 << 20), > DEFINE_PROP_DRIVE("drive", PnvPnor, blk), > DEFINE_PROP_END_OF_LIST(), > }; -- 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] 9+ messages in thread
end of thread, other threads:[~2020-01-08 1:06 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-01-07 17:18 [PATCH 0/2] ppc/pnv: PNOR fixes Cédric Le Goater 2020-01-07 17:18 ` [PATCH 1/2] ppc/pnv: check return value of blk_pwrite() Cédric Le Goater 2020-01-07 17:40 ` Greg Kurz 2020-01-07 18:22 ` Philippe Mathieu-Daudé 2020-01-08 0:56 ` David Gibson 2020-01-07 17:18 ` [PATCH 2/2] ppc/pnv: fix check on return value of blk_getlength() Cédric Le Goater 2020-01-07 17:42 ` Greg Kurz 2020-01-07 18:23 ` Philippe Mathieu-Daudé 2020-01-08 1:01 ` David Gibson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).