[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] nvme: check size before memcpy
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] nvme: check size before memcpy |
Date: |
Fri, 26 Oct 2018 11:31:57 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 |
On 22/10/2018 14:14, P J P wrote:
> From: Prasad J Pandit <address@hidden>
>
> While in nvme_mmio_read, memcpy could read past the 'n->bar'
> buffer, if addr offset was pointing towards its tail end.
> Add check to avoid OOB access.
>
> Reported-by: Caihongzhu <address@hidden>
> Signed-off-by: Prasad J Pandit <address@hidden>
> ---
> hw/block/nvme.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index fc7dacb816..87afc19b61 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1059,7 +1059,7 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr
> addr, unsigned size)
> /* should RAZ, fall through for now */
> }
>
> - if (addr < sizeof(n->bar)) {
> + if (addr + size <= sizeof(n->bar)) {
> memcpy(&val, ptr + addr, size);
> } else {
> NVME_GUEST_ERR(nvme_ub_mmiord_invalid_ofs,
>
Do you have a reproducer? In particular, I think this cannot happen
because memory_region_dispatch_read will block accesses beyond 4 bytes,
and earlier code in this function already check that accesses are
aligned to 32 bits.
We could clarify it with
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index fc7dacb816..427e69a78d 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1167,7 +1167,7 @@ static const MemoryRegionOps nvme_mmio_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
.impl = {
.min_access_size = 2,
- .max_access_size = 8,
+ .max_access_size = 4,
},
};
but if my understanding is right then there is no bug.
Paolo