Re: [Qemu-devel] [PATCH] net: mcf: check buffer descriptor length

[Paolo Bonzini]

On 21/09/2016 19:50, P J P wrote:
> +-- On Wed, 21 Sep 2016, Paolo Bonzini wrote --+
> | On 21/09/2016 15:45, P J P wrote:
> | >          DPRINTF("tx_bd %x flags %04x len %d data %08x\n",
> | >                  addr, bd.flags, bd.length, bd.data);
> | > -        if ((bd.flags & FEC_BD_R) == 0) {
> | > +        if (!bd.length || (bd.flags & FEC_BD_R) == 0) {
> | >              /* Run out of descriptors to transmit.  */
> | >              break;
> | >          }
> | 
> | Is this a bug?
> 
>   Yes, a guest user can control the contents of buffer descriptor 'bd' and 
> could set its length to zero and bd.flags to FEC_BD_R; Thus making the loop 
> run infinite iterations.

Not exactly, because addr changes on every call to mcf_fec_read_bd.

You can add a limit (e.g. 1024 or 2048 descriptors), but the patches are
incorrect.

Paolo

> Thank you.
> --
> Prasad J Pandit / Red Hat Product Security Team
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
>