* Michael S. Tsirkin (m...@redhat.com) wrote:
> CVE-2013-4532
>
> s->tx_frame_len is read from the wire and can later used as an index
> into s->tx_fifo[] for memset() when a DATA command is issued by the guest.
>
> In this case s->tx_frame_len is checked to avoid an overrun, but if the
> value is negative a subsequently executed guest can underrun the buffer
> with zeros via the memset() call.
>
> Additionally, tx_frame_len is used to validate that tx_fifo_len
> doesn't exceed the fifo bounds - the assumption being that data model
> never makes it exceed 2032.
>
> Fix this by failing migration if the incoming value of s->tx_frame_len
> is less than -1 (the emulation code allows from -1 as a special case)
> or if it exceeds 2032.
>
> Reported-by: Michael Roth <mdr...@linux.vnet.ibm.com>
> Reported-by: Peter Maydell <peter.mayd...@linaro.org>
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> ---
> hw/net/stellaris_enet.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
> index aed00fd..90ff950 100644
> --- a/hw/net/stellaris_enet.c
> +++ b/hw/net/stellaris_enet.c
> @@ -373,7 +373,11 @@ static int stellaris_enet_load(QEMUFile *f, void
> *opaque, int version_id)
> s->mtxd = qemu_get_be32(f);
> s->mrxd = qemu_get_be32(f);
> s->np = qemu_get_be32(f);
> - s->tx_frame_len = qemu_get_be32(f);
> + v = qemu_get_be32(f);
> + if (v < -1 || s->tx_frame_len > 2032) {
> + return -EINVAL;
> + }
> + s->tx_frame_len = v;
I think that's wrong; 'stellaris_enet_write' does the following:
s->tx_frame_len = value & 0xffff;
if (s->tx_frame_len > 2032) {
DPRINTF("TX frame too long (%d)\n", s->tx_frame_len);
s->tx_frame_len = 0;
s->ris |= SE_INT_TXER;
stellaris_enet_update(s);
} else {
DPRINTF("Start TX frame len=%d\n", s->tx_frame_len);
/* The value written does not include the ethernet header. */
s->tx_frame_len += 14;
if ((s->tctl & SE_TCTL_CRC) == 0)
s->tx_frame_len += 4;
So lets say that tx_frame_len is initially 2032 when written; 14 is added to it
at this point, and if the CRC flag is set then another 4. Thus it seems a user
can set the value in tx_frame_len to 2032+14+4=2050 - which is a bit worrying
given the buffer is only 2048 bytes.
Dave
> v = qemu_get_be32(f);
> /* How many bytes does data use in tx fifo. */
> sz = s->tx_frame_len == -1 ? 2 : 4;
> --
> MST
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
