diff mbox series

[PATCH-for-9.0?,v2] hw/net/net_tx_pkt: Fix overrun in update_sctp_checksum()

Message ID 20240410070459.49112-1-philmd@linaro.org
State New
Headers show
Series [PATCH-for-9.0?,v2] hw/net/net_tx_pkt: Fix overrun in update_sctp_checksum() | expand

Commit Message

Philippe Mathieu-Daudé April 10, 2024, 7:04 a.m. UTC 
If a fragmented packet size is too short, do not try to
calculate its checksum.

Reproduced using:

  $ cat << EOF | qemu-system-i386 -display none -nodefaults \
                                  -machine q35,accel=qtest -m 32M \
                                  -device igb,netdev=net0 \
                                  -netdev user,id=net0 \
                                  -qtest stdio
  outl 0xcf8 0x80000810
  outl 0xcfc 0xe0000000
  outl 0xcf8 0x80000804
  outw 0xcfc 0x06
  write 0xe0000403 0x1 0x02
  writel 0xe0003808 0xffffffff
  write 0xe000381a 0x1 0x5b
  write 0xe000381b 0x1 0x00
  EOF
  Assertion failed: (offset == 0), function iov_from_buf_full, file util/iov.c, line 39.
  #1 0x5575e81e952a in iov_from_buf_full qemu/util/iov.c:39:5
  #2 0x5575e6500768 in net_tx_pkt_update_sctp_checksum qemu/hw/net/net_tx_pkt.c:144:9
  #3 0x5575e659f3e1 in igb_setup_tx_offloads qemu/hw/net/igb_core.c:478:11
  #4 0x5575e659f3e1 in igb_tx_pkt_send qemu/hw/net/igb_core.c:552:10
  #5 0x5575e659f3e1 in igb_process_tx_desc qemu/hw/net/igb_core.c:671:17
  #6 0x5575e659f3e1 in igb_start_xmit qemu/hw/net/igb_core.c:903:9
  #7 0x5575e659f3e1 in igb_set_tdt qemu/hw/net/igb_core.c:2812:5
  #8 0x5575e657d6a4 in igb_core_write qemu/hw/net/igb_core.c:4248:9

Cc: qemu-stable@nongnu.org
Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Fixes: f199b13bc1 ("igb: Implement Tx SCTP CSO")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2273
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Since v1: check at offset 8 (Akihiko)
---
 hw/net/net_tx_pkt.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Akihiko Odaki April 10, 2024, 7:06 a.m. UTC | #1 
On 2024/04/10 16:04, Philippe Mathieu-Daudé wrote:
> If a fragmented packet size is too short, do not try to
> calculate its checksum.
> 
> Reproduced using:
> 
>    $ cat << EOF | qemu-system-i386 -display none -nodefaults \
>                                    -machine q35,accel=qtest -m 32M \
>                                    -device igb,netdev=net0 \
>                                    -netdev user,id=net0 \
>                                    -qtest stdio
>    outl 0xcf8 0x80000810
>    outl 0xcfc 0xe0000000
>    outl 0xcf8 0x80000804
>    outw 0xcfc 0x06
>    write 0xe0000403 0x1 0x02
>    writel 0xe0003808 0xffffffff
>    write 0xe000381a 0x1 0x5b
>    write 0xe000381b 0x1 0x00
>    EOF
>    Assertion failed: (offset == 0), function iov_from_buf_full, file util/iov.c, line 39.
>    #1 0x5575e81e952a in iov_from_buf_full qemu/util/iov.c:39:5
>    #2 0x5575e6500768 in net_tx_pkt_update_sctp_checksum qemu/hw/net/net_tx_pkt.c:144:9
>    #3 0x5575e659f3e1 in igb_setup_tx_offloads qemu/hw/net/igb_core.c:478:11
>    #4 0x5575e659f3e1 in igb_tx_pkt_send qemu/hw/net/igb_core.c:552:10
>    #5 0x5575e659f3e1 in igb_process_tx_desc qemu/hw/net/igb_core.c:671:17
>    #6 0x5575e659f3e1 in igb_start_xmit qemu/hw/net/igb_core.c:903:9
>    #7 0x5575e659f3e1 in igb_set_tdt qemu/hw/net/igb_core.c:2812:5
>    #8 0x5575e657d6a4 in igb_core_write qemu/hw/net/igb_core.c:4248:9
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> Fixes: f199b13bc1 ("igb: Implement Tx SCTP CSO")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2273
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

> ---
> Since v1: check at offset 8 (Akihiko)
> ---
>   hw/net/net_tx_pkt.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
> index 2134a18c4c..b7b1de816d 100644
> --- a/hw/net/net_tx_pkt.c
> +++ b/hw/net/net_tx_pkt.c
> @@ -141,6 +141,10 @@ bool net_tx_pkt_update_sctp_checksum(struct NetTxPkt *pkt)
>       uint32_t csum = 0;
>       struct iovec *pl_start_frag = pkt->vec + NET_TX_PKT_PL_START_FRAG;
>   
> +    if (iov_size(pl_start_frag, pkt->payload_frags) < 8 + sizeof(csum)) {
> +        return false;
> +    }
> +
>       if (iov_from_buf(pl_start_frag, pkt->payload_frags, 8, &csum, sizeof(csum)) < sizeof(csum)) {
>           return false;
>       }
Mauro Matteo Cascella April 10, 2024, 7:35 a.m. UTC | #2 
Hi,

On Wed, Apr 10, 2024 at 9:05 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> If a fragmented packet size is too short, do not try to
> calculate its checksum.

This was assigned CVE-2024-3567.

Thanks,

> Reproduced using:
>
>   $ cat << EOF | qemu-system-i386 -display none -nodefaults \
>                                   -machine q35,accel=qtest -m 32M \
>                                   -device igb,netdev=net0 \
>                                   -netdev user,id=net0 \
>                                   -qtest stdio
>   outl 0xcf8 0x80000810
>   outl 0xcfc 0xe0000000
>   outl 0xcf8 0x80000804
>   outw 0xcfc 0x06
>   write 0xe0000403 0x1 0x02
>   writel 0xe0003808 0xffffffff
>   write 0xe000381a 0x1 0x5b
>   write 0xe000381b 0x1 0x00
>   EOF
>   Assertion failed: (offset == 0), function iov_from_buf_full, file util/iov.c, line 39.
>   #1 0x5575e81e952a in iov_from_buf_full qemu/util/iov.c:39:5
>   #2 0x5575e6500768 in net_tx_pkt_update_sctp_checksum qemu/hw/net/net_tx_pkt.c:144:9
>   #3 0x5575e659f3e1 in igb_setup_tx_offloads qemu/hw/net/igb_core.c:478:11
>   #4 0x5575e659f3e1 in igb_tx_pkt_send qemu/hw/net/igb_core.c:552:10
>   #5 0x5575e659f3e1 in igb_process_tx_desc qemu/hw/net/igb_core.c:671:17
>   #6 0x5575e659f3e1 in igb_start_xmit qemu/hw/net/igb_core.c:903:9
>   #7 0x5575e659f3e1 in igb_set_tdt qemu/hw/net/igb_core.c:2812:5
>   #8 0x5575e657d6a4 in igb_core_write qemu/hw/net/igb_core.c:4248:9
>
> Cc: qemu-stable@nongnu.org
> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> Fixes: f199b13bc1 ("igb: Implement Tx SCTP CSO")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2273
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Since v1: check at offset 8 (Akihiko)
> ---
>  hw/net/net_tx_pkt.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
> index 2134a18c4c..b7b1de816d 100644
> --- a/hw/net/net_tx_pkt.c
> +++ b/hw/net/net_tx_pkt.c
> @@ -141,6 +141,10 @@ bool net_tx_pkt_update_sctp_checksum(struct NetTxPkt *pkt)
>      uint32_t csum = 0;
>      struct iovec *pl_start_frag = pkt->vec + NET_TX_PKT_PL_START_FRAG;
>
> +    if (iov_size(pl_start_frag, pkt->payload_frags) < 8 + sizeof(csum)) {
> +        return false;
> +    }
> +
>      if (iov_from_buf(pl_start_frag, pkt->payload_frags, 8, &csum, sizeof(csum)) < sizeof(csum)) {
>          return false;
>      }
> --
> 2.41.0
>
Jason Wang April 10, 2024, 8:24 a.m. UTC | #3 
On Wed, Apr 10, 2024 at 3:06 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> On 2024/04/10 16:04, Philippe Mathieu-Daudé wrote:
> > If a fragmented packet size is too short, do not try to
> > calculate its checksum.
> >
> > Reproduced using:
> >
> >    $ cat << EOF | qemu-system-i386 -display none -nodefaults \
> >                                    -machine q35,accel=qtest -m 32M \
> >                                    -device igb,netdev=net0 \
> >                                    -netdev user,id=net0 \
> >                                    -qtest stdio
> >    outl 0xcf8 0x80000810
> >    outl 0xcfc 0xe0000000
> >    outl 0xcf8 0x80000804
> >    outw 0xcfc 0x06
> >    write 0xe0000403 0x1 0x02
> >    writel 0xe0003808 0xffffffff
> >    write 0xe000381a 0x1 0x5b
> >    write 0xe000381b 0x1 0x00
> >    EOF
> >    Assertion failed: (offset == 0), function iov_from_buf_full, file util/iov.c, line 39.
> >    #1 0x5575e81e952a in iov_from_buf_full qemu/util/iov.c:39:5
> >    #2 0x5575e6500768 in net_tx_pkt_update_sctp_checksum qemu/hw/net/net_tx_pkt.c:144:9
> >    #3 0x5575e659f3e1 in igb_setup_tx_offloads qemu/hw/net/igb_core.c:478:11
> >    #4 0x5575e659f3e1 in igb_tx_pkt_send qemu/hw/net/igb_core.c:552:10
> >    #5 0x5575e659f3e1 in igb_process_tx_desc qemu/hw/net/igb_core.c:671:17
> >    #6 0x5575e659f3e1 in igb_start_xmit qemu/hw/net/igb_core.c:903:9
> >    #7 0x5575e659f3e1 in igb_set_tdt qemu/hw/net/igb_core.c:2812:5
> >    #8 0x5575e657d6a4 in igb_core_write qemu/hw/net/igb_core.c:4248:9
> >
> > Cc: qemu-stable@nongnu.org
> > Reported-by: Zheyu Ma <zheyuma97@gmail.com>
> > Fixes: f199b13bc1 ("igb: Implement Tx SCTP CSO")
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2273
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Fixes: CVE-2024-3567
Acked-by: Jason Wang <jasowang@redhat.com>

Peter, would you want to pick this for 9.0?

Thanks

>
> > ---
> > Since v1: check at offset 8 (Akihiko)
> > ---
> >   hw/net/net_tx_pkt.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
> > index 2134a18c4c..b7b1de816d 100644
> > --- a/hw/net/net_tx_pkt.c
> > +++ b/hw/net/net_tx_pkt.c
> > @@ -141,6 +141,10 @@ bool net_tx_pkt_update_sctp_checksum(struct NetTxPkt *pkt)
> >       uint32_t csum = 0;
> >       struct iovec *pl_start_frag = pkt->vec + NET_TX_PKT_PL_START_FRAG;
> >
> > +    if (iov_size(pl_start_frag, pkt->payload_frags) < 8 + sizeof(csum)) {
> > +        return false;
> > +    }
> > +
> >       if (iov_from_buf(pl_start_frag, pkt->payload_frags, 8, &csum, sizeof(csum)) < sizeof(csum)) {
> >           return false;
> >       }
>
Philippe Mathieu-Daudé April 10, 2024, 8:27 a.m. UTC | #4 
On 10/4/24 09:35, Mauro Matteo Cascella wrote:
> Hi,
> 
> On Wed, Apr 10, 2024 at 9:05 AM Philippe Mathieu-Daudé
> <philmd@linaro.org> wrote:
>>
>> If a fragmented packet size is too short, do not try to
>> calculate its checksum.
> 
> This was assigned CVE-2024-3567.

Thanks for the quick reaction!

>> Reproduced using:
>>
>>    $ cat << EOF | qemu-system-i386 -display none -nodefaults \
>>                                    -machine q35,accel=qtest -m 32M \
>>                                    -device igb,netdev=net0 \
>>                                    -netdev user,id=net0 \
>>                                    -qtest stdio
>>    outl 0xcf8 0x80000810
>>    outl 0xcfc 0xe0000000
>>    outl 0xcf8 0x80000804
>>    outw 0xcfc 0x06
>>    write 0xe0000403 0x1 0x02
>>    writel 0xe0003808 0xffffffff
>>    write 0xe000381a 0x1 0x5b
>>    write 0xe000381b 0x1 0x00
>>    EOF
>>    Assertion failed: (offset == 0), function iov_from_buf_full, file util/iov.c, line 39.
>>    #1 0x5575e81e952a in iov_from_buf_full qemu/util/iov.c:39:5
>>    #2 0x5575e6500768 in net_tx_pkt_update_sctp_checksum qemu/hw/net/net_tx_pkt.c:144:9
>>    #3 0x5575e659f3e1 in igb_setup_tx_offloads qemu/hw/net/igb_core.c:478:11
>>    #4 0x5575e659f3e1 in igb_tx_pkt_send qemu/hw/net/igb_core.c:552:10
>>    #5 0x5575e659f3e1 in igb_process_tx_desc qemu/hw/net/igb_core.c:671:17
>>    #6 0x5575e659f3e1 in igb_start_xmit qemu/hw/net/igb_core.c:903:9
>>    #7 0x5575e659f3e1 in igb_set_tdt qemu/hw/net/igb_core.c:2812:5
>>    #8 0x5575e657d6a4 in igb_core_write qemu/hw/net/igb_core.c:4248:9
>>
>> Cc: qemu-stable@nongnu.org
>> Reported-by: Zheyu Ma <zheyuma97@gmail.com>
>> Fixes: f199b13bc1 ("igb: Implement Tx SCTP CSO")
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2273
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> Since v1: check at offset 8 (Akihiko)
>> ---
>>   hw/net/net_tx_pkt.c | 4 ++++
>>   1 file changed, 4 insertions(+)

Patch queued.
diff mbox series

Patch

diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
index 2134a18c4c..b7b1de816d 100644
--- a/hw/net/net_tx_pkt.c
+++ b/hw/net/net_tx_pkt.c
@@ -141,6 +141,10 @@  bool net_tx_pkt_update_sctp_checksum(struct NetTxPkt *pkt)
     uint32_t csum = 0;
     struct iovec *pl_start_frag = pkt->vec + NET_TX_PKT_PL_START_FRAG;
 
+    if (iov_size(pl_start_frag, pkt->payload_frags) < 8 + sizeof(csum)) {
+        return false;
+    }
+
     if (iov_from_buf(pl_start_frag, pkt->payload_frags, 8, &csum, sizeof(csum)) < sizeof(csum)) {
         return false;
     }