| Message ID | da5acadd07eabd5a6e9fc8870fecb435173b8f47.1626247332.git.alistair.francis@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/1] hw/riscv/boot: Check the error of fdt_pack() | expand |
On Wed, Jul 14, 2021 at 3:22 PM Alistair Francis <alistair.francis@wdc.com> wrote: > > Coverity reports that we don't check the error result of fdt_pack(), so > let's save the result and assert that it is 0. > > Fixes: Coverity CID 1458136 > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > hw/riscv/boot.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
On Wed, 14 Jul 2021 at 08:22, Alistair Francis <alistair.francis@wdc.com> wrote: > > Coverity reports that we don't check the error result of fdt_pack(), so > let's save the result and assert that it is 0. > > Fixes: Coverity CID 1458136 > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > hw/riscv/boot.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > index 0d38bb7426..25406a3f67 100644 > --- a/hw/riscv/boot.c > +++ b/hw/riscv/boot.c > @@ -182,7 +182,7 @@ uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) > { > uint32_t temp, fdt_addr; > hwaddr dram_end = dram_base + mem_size; > - int fdtsize = fdt_totalsize(fdt); > + int ret, fdtsize = fdt_totalsize(fdt); > > if (fdtsize <= 0) { > error_report("invalid device-tree"); > @@ -198,7 +198,8 @@ uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) > temp = MIN(dram_end, 3072 * MiB); > fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 16 * MiB); > > - fdt_pack(fdt); > + ret = fdt_pack(fdt); > + g_assert(ret == 0); > /* copy in the device tree */ > qemu_fdt_dumpdtb(fdt, fdtsize); Are we in the same situation as spapr.c where we believe the call will only fail if the fdt was corrupt somehow? If so, it would be nice to also have the comment from spapr.c: /* Should only fail if we've built a corrupted tree */ thanks -- PMM
On Wed, Jul 14, 2021 at 6:46 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Wed, 14 Jul 2021 at 08:22, Alistair Francis <alistair.francis@wdc.com> wrote: > > > > Coverity reports that we don't check the error result of fdt_pack(), so > > let's save the result and assert that it is 0. > > > > Fixes: Coverity CID 1458136 > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > hw/riscv/boot.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > > index 0d38bb7426..25406a3f67 100644 > > --- a/hw/riscv/boot.c > > +++ b/hw/riscv/boot.c > > @@ -182,7 +182,7 @@ uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) > > { > > uint32_t temp, fdt_addr; > > hwaddr dram_end = dram_base + mem_size; > > - int fdtsize = fdt_totalsize(fdt); > > + int ret, fdtsize = fdt_totalsize(fdt); > > > > if (fdtsize <= 0) { > > error_report("invalid device-tree"); > > @@ -198,7 +198,8 @@ uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) > > temp = MIN(dram_end, 3072 * MiB); > > fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 16 * MiB); > > > > - fdt_pack(fdt); > > + ret = fdt_pack(fdt); > > + g_assert(ret == 0); > > /* copy in the device tree */ > > qemu_fdt_dumpdtb(fdt, fdtsize); > > Are we in the same situation as spapr.c where we believe the call > will only fail if the fdt was corrupt somehow? If so, it would be > nice to also have the comment from spapr.c: > /* Should only fail if we've built a corrupted tree */ Yes, we are the same. I added the comment. Alistair > > thanks > -- PMM
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c index 0d38bb7426..25406a3f67 100644 --- a/hw/riscv/boot.c +++ b/hw/riscv/boot.c @@ -182,7 +182,7 @@ uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) { uint32_t temp, fdt_addr; hwaddr dram_end = dram_base + mem_size; - int fdtsize = fdt_totalsize(fdt); + int ret, fdtsize = fdt_totalsize(fdt); if (fdtsize <= 0) { error_report("invalid device-tree"); @@ -198,7 +198,8 @@ uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt) temp = MIN(dram_end, 3072 * MiB); fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 16 * MiB); - fdt_pack(fdt); + ret = fdt_pack(fdt); + g_assert(ret == 0); /* copy in the device tree */ qemu_fdt_dumpdtb(fdt, fdtsize);
Coverity reports that we don't check the error result of fdt_pack(), so let's save the result and assert that it is 0. Fixes: Coverity CID 1458136 Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- hw/riscv/boot.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)