[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 14/30] stellaris_enet: avoid buffer overrun o
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v4 14/30] stellaris_enet: avoid buffer overrun on incoming migration (part 2) |
Date: |
Tue, 1 Apr 2014 10:43:32 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
* Michael S. Tsirkin (address@hidden) wrote:
> CVE-2013-4532
>
> s->tx_fifo_len is read from the wire and later used as an index into
> s->tx_fifo[] when a DATA command is issued by the guest. If
> s->tx_fifo_len is greater than the length of s->tx_fifo[], or less
> than 0, the buffer can be overrun/underrun by arbitrary data written out
> by the guest upon resuming its execution.
>
> Fix this by failing migration if the value from the wire would make
> guest access the array out of bounds.
>
> Reported-by: Michael Roth <address@hidden>
> Reported-by: Peter Maydell <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
> hw/net/stellaris_enet.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
> index 182fd3e..aed00fd 100644
> --- a/hw/net/stellaris_enet.c
> +++ b/hw/net/stellaris_enet.c
> @@ -358,7 +358,7 @@ static void stellaris_enet_save(QEMUFile *f, void *opaque)
> static int stellaris_enet_load(QEMUFile *f, void *opaque, int version_id)
> {
> stellaris_enet_state *s = (stellaris_enet_state *)opaque;
> - int i, v;
> + int i, v, sz;
>
> if (version_id != 1)
> return -EINVAL;
> @@ -374,7 +374,13 @@ static int stellaris_enet_load(QEMUFile *f, void
> *opaque, int version_id)
> s->mrxd = qemu_get_be32(f);
> s->np = qemu_get_be32(f);
> s->tx_frame_len = qemu_get_be32(f);
> - s->tx_fifo_len = qemu_get_be32(f);
> + v = qemu_get_be32(f);
> + /* How many bytes does data use in tx fifo. */
> + sz = s->tx_frame_len == -1 ? 2 : 4;
> + if (v < 0 || v >= ARRAY_SIZE(s->tx_fifo) - sz) {
> + return -EINVAL;
> + }
> + s->tx_fifo_len = v;
> qemu_get_buffer(f, s->tx_fifo, sizeof(s->tx_fifo));
I don't understand the magic '2' in that ?2:4 - but there again the 'DATA' write
case is pretty hairy.
Dave
> for (i = 0; i < 31; i++) {
> s->rx[i].len = qemu_get_be32(f);
> --
> MST
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- Re: [Qemu-devel] [PATCH v4 14/30] stellaris_enet: avoid buffer overrun on incoming migration (part 2),
Dr. David Alan Gilbert <=