1. Patchew
  2. QEMU
  3. [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539
  4. View patch

[RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect

Philippe Mathieu-Daudé posted 3 patches 5 years, 6 months ago
[RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect
Posted by Philippe Mathieu-Daudé 5 years, 6 months ago 
When a frame lenght is incorrect, set the RDES0 'Error Summary'
and 'Frame too long' bits. Then stop the receive process and
trigger an abnormal interrupt. See [4.3.5 Receive Process].

Cc: Li Qiang <liq3ea@gmail.com>
Cc: Li Qiang <pangpei.lq@antfin.com>
Cc: Ziming Zhang <ezrakiez@gmail.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Prasad J Pandit <pjp@fedoraproject.org>
Fixes: 8ffb7265af ("check frame size and r/w data length")
Buglink: https://bugs.launchpad.net/bugs/1874539
Reported-by: Helge Deller <deller@gmx.de>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/net/tulip.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index 470f635acb..671f79b6f4 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -158,7 +158,7 @@ static void tulip_next_rx_descriptor(TULIPState *s,
     s->current_rx_desc &= ~3ULL;
 }
 
-static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
+static int tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
 {
     int len1 = (desc->control >> RDES1_BUF1_SIZE_SHIFT) & RDES1_BUF1_SIZE_MASK;
     int len2 = (desc->control >> RDES1_BUF2_SIZE_SHIFT) & RDES1_BUF2_SIZE_MASK;
@@ -177,7 +177,8 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
                           "(ofs: %u, len:%d, size:%zu)\n",
                           __func__, s->rx_frame_len, len,
                           sizeof(s->rx_frame));
-            return;
+            s->rx_frame_len = 0;
+            return -1;
         }
         pci_dma_write(&s->dev, desc->buf_addr1, s->rx_frame +
             (s->rx_frame_size - s->rx_frame_len), len);
@@ -197,12 +198,15 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
                           "(ofs: %u, len:%d, size:%zu)\n",
                           __func__, s->rx_frame_len, len,
                           sizeof(s->rx_frame));
-            return;
+            s->rx_frame_len = 0;
+            return -1;
         }
         pci_dma_write(&s->dev, desc->buf_addr2, s->rx_frame +
             (s->rx_frame_size - s->rx_frame_len), len);
         s->rx_frame_len -= len;
     }
+
+    return 0;
 }
 
 static bool tulip_filter_address(TULIPState *s, const uint8_t *addr)
@@ -274,7 +278,11 @@ static ssize_t tulip_receive(TULIPState *s, const uint8_t *buf, size_t size)
             s->rx_frame_len = s->rx_frame_size;
         }
 
-        tulip_copy_rx_bytes(s, &desc);
+        if (tulip_copy_rx_bytes(s, &desc)) {
+            desc.status |= RDES0_ES | RDES0_TL; /* Error: frame too long */
+            s->csr[5] |= CSR5_RPS; /* Receive process stopped */
+            tulip_update_int(s);
+        }
 
         if (!s->rx_frame_len) {
             desc.status |= s->rx_status;
-- 
2.21.1
Re: [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect
Posted by Eric Blake 5 years, 6 months ago 
On 4/23/20 6:16 PM, Philippe Mathieu-Daudé wrote:
> When a frame lenght is incorrect, set the RDES0 'Error Summary'

length (here, and in the subject)

> and 'Frame too long' bits. Then stop the receive process and
> trigger an abnormal interrupt. See [4.3.5 Receive Process].
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org
Re: [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect
Posted by Jason Wang 5 years, 6 months ago 
On 2020/4/24 上午7:16, Philippe Mathieu-Daudé wrote:
> When a frame lenght is incorrect, set the RDES0 'Error Summary'
> and 'Frame too long' bits. Then stop the receive process and
> trigger an abnormal interrupt. See [4.3.5 Receive Process].
>
> Cc: Li Qiang <liq3ea@gmail.com>
> Cc: Li Qiang <pangpei.lq@antfin.com>
> Cc: Ziming Zhang <ezrakiez@gmail.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Prasad J Pandit <pjp@fedoraproject.org>
> Fixes: 8ffb7265af ("check frame size and r/w data length")
> Buglink: https://bugs.launchpad.net/bugs/1874539
> Reported-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


Hi Philippe:

It's still unclear to me that how this fixes the stuck. Did you mean 
guest trigger the error condition and then recvoer by abnormal interrupt?

If yes, this sounds still like a bug somewhere in the code.

Thanks


> ---
>   hw/net/tulip.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
> index 470f635acb..671f79b6f4 100644
> --- a/hw/net/tulip.c
> +++ b/hw/net/tulip.c
> @@ -158,7 +158,7 @@ static void tulip_next_rx_descriptor(TULIPState *s,
>       s->current_rx_desc &= ~3ULL;
>   }
>   
> -static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
> +static int tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
>   {
>       int len1 = (desc->control >> RDES1_BUF1_SIZE_SHIFT) & RDES1_BUF1_SIZE_MASK;
>       int len2 = (desc->control >> RDES1_BUF2_SIZE_SHIFT) & RDES1_BUF2_SIZE_MASK;
> @@ -177,7 +177,8 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
>                             "(ofs: %u, len:%d, size:%zu)\n",
>                             __func__, s->rx_frame_len, len,
>                             sizeof(s->rx_frame));
> -            return;
> +            s->rx_frame_len = 0;
> +            return -1;
>           }
>           pci_dma_write(&s->dev, desc->buf_addr1, s->rx_frame +
>               (s->rx_frame_size - s->rx_frame_len), len);
> @@ -197,12 +198,15 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
>                             "(ofs: %u, len:%d, size:%zu)\n",
>                             __func__, s->rx_frame_len, len,
>                             sizeof(s->rx_frame));
> -            return;
> +            s->rx_frame_len = 0;
> +            return -1;
>           }
>           pci_dma_write(&s->dev, desc->buf_addr2, s->rx_frame +
>               (s->rx_frame_size - s->rx_frame_len), len);
>           s->rx_frame_len -= len;
>       }
> +
> +    return 0;
>   }
>   
>   static bool tulip_filter_address(TULIPState *s, const uint8_t *addr)
> @@ -274,7 +278,11 @@ static ssize_t tulip_receive(TULIPState *s, const uint8_t *buf, size_t size)
>               s->rx_frame_len = s->rx_frame_size;
>           }
>   
> -        tulip_copy_rx_bytes(s, &desc);
> +        if (tulip_copy_rx_bytes(s, &desc)) {
> +            desc.status |= RDES0_ES | RDES0_TL; /* Error: frame too long */
> +            s->csr[5] |= CSR5_RPS; /* Receive process stopped */
> +            tulip_update_int(s);
> +        }
>   
>           if (!s->rx_frame_len) {
>               desc.status |= s->rx_status;
Re: [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect
Posted by Helge Deller 5 years, 6 months ago 
On 24.04.20 04:16, Jason Wang wrote:
> 
> On 2020/4/24 上午7:16, Philippe Mathieu-Daudé wrote:
>> When a frame lenght is incorrect, set the RDES0 'Error Summary'
>> and 'Frame too long' bits. Then stop the receive process and
>> trigger an abnormal interrupt. See [4.3.5 Receive Process].
>>
>> Cc: Li Qiang <liq3ea@gmail.com>
>> Cc: Li Qiang <pangpei.lq@antfin.com>
>> Cc: Ziming Zhang <ezrakiez@gmail.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: Prasad J Pandit <pjp@fedoraproject.org>
>> Fixes: 8ffb7265af ("check frame size and r/w data length")
>> Buglink: https://bugs.launchpad.net/bugs/1874539
>> Reported-by: Helge Deller <deller@gmx.de>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> 
> Hi Philippe:
> 
> It's still unclear to me that how this fixes the stuck. Did you mean guest trigger the error condition and then recvoer by abnormal interrupt?
> 
> If yes, this sounds still like a bug somewhere in the code.


Sadly the patches do not fix the bug.


Helge


> 
>> ---
>>   hw/net/tulip.c | 16 ++++++++++++----
>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
>> index 470f635acb..671f79b6f4 100644
>> --- a/hw/net/tulip.c
>> +++ b/hw/net/tulip.c
>> @@ -158,7 +158,7 @@ static void tulip_next_rx_descriptor(TULIPState *s,
>>       s->current_rx_desc &= ~3ULL;
>>   }
>>   -static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
>> +static int tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
>>   {
>>       int len1 = (desc->control >> RDES1_BUF1_SIZE_SHIFT) & RDES1_BUF1_SIZE_MASK;
>>       int len2 = (desc->control >> RDES1_BUF2_SIZE_SHIFT) & RDES1_BUF2_SIZE_MASK;
>> @@ -177,7 +177,8 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
>>                             "(ofs: %u, len:%d, size:%zu)\n",
>>                             __func__, s->rx_frame_len, len,
>>                             sizeof(s->rx_frame));
>> -            return;
>> +            s->rx_frame_len = 0;
>> +            return -1;
>>           }
>>           pci_dma_write(&s->dev, desc->buf_addr1, s->rx_frame +
>>               (s->rx_frame_size - s->rx_frame_len), len);
>> @@ -197,12 +198,15 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
>>                             "(ofs: %u, len:%d, size:%zu)\n",
>>                             __func__, s->rx_frame_len, len,
>>                             sizeof(s->rx_frame));
>> -            return;
>> +            s->rx_frame_len = 0;
>> +            return -1;
>>           }
>>           pci_dma_write(&s->dev, desc->buf_addr2, s->rx_frame +
>>               (s->rx_frame_size - s->rx_frame_len), len);
>>           s->rx_frame_len -= len;
>>       }
>> +
>> +    return 0;
>>   }
>>     static bool tulip_filter_address(TULIPState *s, const uint8_t *addr)
>> @@ -274,7 +278,11 @@ static ssize_t tulip_receive(TULIPState *s, const uint8_t *buf, size_t size)
>>               s->rx_frame_len = s->rx_frame_size;
>>           }
>>   -        tulip_copy_rx_bytes(s, &desc);
>> +        if (tulip_copy_rx_bytes(s, &desc)) {
>> +            desc.status |= RDES0_ES | RDES0_TL; /* Error: frame too long */
>> +            s->csr[5] |= CSR5_RPS; /* Receive process stopped */
>> +            tulip_update_int(s);
>> +        }
>>             if (!s->rx_frame_len) {
>>               desc.status |= s->rx_status;
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.