Re: [Qemu-devel] [PATCH v4 1/2] rtl8139: Fix receive buffer overflow check

[Jason Wang]

On 09/01/2015 11:26 PM, Vladislav Yasevich wrote:
> rtl8139_do_receive() tries to check for the overflow condition
> by making sure that packet_size + 8 does not exceed the
> available buffer space.  The issue here is that RxBuffAddr,
> used to calculate available buffer space, is aligned to a
> a 4 byte boundry after every update.  So it is possible that
> every packet ends up being slightly padded when written
> to the receive buffer.  This padding is not taken into
> account when checking for overflow and we may end up missing
> the overflow condition can causing buffer overwrite.
>
> This patch takes alignment into consideration when
> checking for overflow condition.
>
> Signed-off-by: Vladislav Yasevich <address@hidden>
> ---
>  hw/net/rtl8139.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index edbb61c..8a33466 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -1148,7 +1148,9 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, 
> const uint8_t *buf, size_t
>  
>          /* if receiver buffer is empty then avail == 0 */
>  
> -        if (avail != 0 && size + 8 >= avail)
> +#define RX_ALIGN(x) (((x) + 3) & ~0x3)
> +
> +        if (avail != 0 && RX_ALIGN(size + 8) >= avail)
>          {
>              DPRINTF("rx overflow: rx buffer length %d head 0x%04x "
>                  "read 0x%04x === available 0x%04x need 0x%04x\n",
> @@ -1176,7 +1178,7 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, 
> const uint8_t *buf, size_t
>          rtl8139_write_buffer(s, (uint8_t *)&val, 4);
>  
>          /* correct buffer write pointer */
> -        s->RxBufAddr = MOD2((s->RxBufAddr + 3) & ~0x3, s->RxBufferSize);
> +        s->RxBufAddr = MOD2(RX_ALIGN(s->RxBufAddr), s->RxBufferSize);
>  
>          /* now we can signal we have received something */
>  

Reviewed-by: Jason Wang <address@hidden>