Re: [PATCH] net: e1000: check transmit descriptor field values

[Alexander Bulekov]

On 210210 2022, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
> 
> While processing transmit (tx) descriptors in process_tx_desc()
> various descriptor fields are not checked properly. This may lead
> to infinite loop like issue. Add checks to avoid them.
> 

+CC Peter Maydell

Is this a DMA re-entracy/stack-overflow issue? IIRC the plan was to have
some sort of wider fix for these issues. There are bunch of these
reports floating around at this point, I believe.

> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> Reported-by: Cheolwoo Myung <cwmyung@snu.ac.kr>
> Reported-by: Ruhr-University Bochum <bugs-syssec@rub.de>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/net/e1000.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index d8da2f6528..15949a3d64 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -667,9 +667,11 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>  
>      addr = le64_to_cpu(dp->buffer_addr);
>      if (tp->cptse) {
> +        assert(tp->tso_props.hdr_len);
>          msh = tp->tso_props.hdr_len + tp->tso_props.mss;
>          do {
>              bytes = split_size;
> +            assert(msh > tp->size);
>              if (tp->size + bytes > msh)
>                  bytes = msh - tp->size;
>  
> @@ -681,22 +683,26 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>                  memmove(tp->header, tp->data, tp->tso_props.hdr_len);
>              }
>              tp->size = sz;
> +            assert(tp->size);   /* sz may get truncated */
>              addr += bytes;
>              if (sz == msh) {
>                  xmit_seg(s);
>                  memmove(tp->data, tp->header, tp->tso_props.hdr_len);
>                  tp->size = tp->tso_props.hdr_len;
>              }
> +            assert(split_size >= bytes);
>              split_size -= bytes;
>          } while (bytes && split_size);
>      } else {
>          split_size = MIN(sizeof(tp->data) - tp->size, split_size);
> +        assert(tp->size && split_size);
>          pci_dma_read(d, addr, tp->data + tp->size, split_size);
>          tp->size += split_size;
>      }
>  
>      if (!(txd_lower & E1000_TXD_CMD_EOP))
>          return;
> +    assert(tp->size && tp->tso_props.hdr_len);
>      if (!(tp->cptse && tp->size < tp->tso_props.hdr_len)) {
>          xmit_seg(s);
>      }
> -- 
> 2.29.2
>