All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539
@ 2020-04-23 23:16 Philippe Mathieu-Daudé
  2020-04-23 23:16 ` [RFC PATCH 1/3] hw/net/tulip: Fix 'Descriptor Error' definition Philippe Mathieu-Daudé
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-23 23:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Helge Deller, Sven Schnelle, Jason Wang, Philippe Mathieu-Daudé

Attempt to fix the launchpad bug filled by Helge:

  In a qemu-system-hppa system, qemu release v5.0.0-rc,
  the tulip nic driver is broken.  The tulip nic is detected,
  even getting DHCP info does work.  But when trying to
  download bigger files via network, the tulip driver gets
  stuck.

Philippe Mathieu-Daudé (3):
  hw/net/tulip: Fix 'Descriptor Error' definition
  hw/net/tulip: Log descriptor overflows
  hw/net/tulip: Set descriptor error bit when lenght is incorrect

 hw/net/tulip.h |  2 +-
 hw/net/tulip.c | 32 ++++++++++++++++++++++++++++----
 2 files changed, 29 insertions(+), 5 deletions(-)

-- 
2.21.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH 1/3] hw/net/tulip: Fix 'Descriptor Error' definition
  2020-04-23 23:16 [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539 Philippe Mathieu-Daudé
@ 2020-04-23 23:16 ` Philippe Mathieu-Daudé
  2020-04-23 23:16 ` [RFC PATCH 2/3] hw/net/tulip: Log descriptor overflows Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-23 23:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Helge Deller, Sven Schnelle, Jason Wang, Philippe Mathieu-Daudé

Bit #14 is "DE" for 'Descriptor Error':

  When set, indicates a frame truncation caused by a frame
  that does not fit within the current descriptor buffers,
  and that the 21143 does not own the next descriptor.

  [Table 4-1. RDES0 Bit Fields Description]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/net/tulip.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/tulip.h b/hw/net/tulip.h
index 97521b21db..5271aad8d5 100644
--- a/hw/net/tulip.h
+++ b/hw/net/tulip.h
@@ -211,7 +211,7 @@
 #define RDES0_RF         BIT(11)
 #define RDES0_DT_SHIFT   12
 #define RDES0_DT_MASK    3
-#define RDES0_LE         BIT(14)
+#define RDES0_DE         BIT(14)
 #define RDES0_ES         BIT(15)
 #define RDES0_FL_SHIFT   16
 #define RDES0_FL_MASK    0x3fff
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH 2/3] hw/net/tulip: Log descriptor overflows
  2020-04-23 23:16 [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539 Philippe Mathieu-Daudé
  2020-04-23 23:16 ` [RFC PATCH 1/3] hw/net/tulip: Fix 'Descriptor Error' definition Philippe Mathieu-Daudé
@ 2020-04-23 23:16 ` Philippe Mathieu-Daudé
  2020-04-23 23:16 ` [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect Philippe Mathieu-Daudé
  2020-04-24 15:27 ` [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539 Helge Deller
  3 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-23 23:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Helge Deller, Sven Schnelle, Jason Wang, Philippe Mathieu-Daudé

Log with GUEST_ERROR what the guest is doing wrong.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/net/tulip.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index 1295f51d07..470f635acb 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -172,6 +172,11 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
         }
 
         if (s->rx_frame_len + len > sizeof(s->rx_frame)) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: descriptor overflow "
+                          "(ofs: %u, len:%d, size:%zu)\n",
+                          __func__, s->rx_frame_len, len,
+                          sizeof(s->rx_frame));
             return;
         }
         pci_dma_write(&s->dev, desc->buf_addr1, s->rx_frame +
@@ -187,6 +192,11 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
         }
 
         if (s->rx_frame_len + len > sizeof(s->rx_frame)) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: descriptor overflow "
+                          "(ofs: %u, len:%d, size:%zu)\n",
+                          __func__, s->rx_frame_len, len,
+                          sizeof(s->rx_frame));
             return;
         }
         pci_dma_write(&s->dev, desc->buf_addr2, s->rx_frame +
@@ -584,6 +594,9 @@ static int tulip_copy_tx_buffers(TULIPState *s, struct tulip_descriptor *desc)
     int len2 = (desc->control >> TDES1_BUF2_SIZE_SHIFT) & TDES1_BUF2_SIZE_MASK;
 
     if (s->tx_frame_len + len1 > sizeof(s->tx_frame)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: descriptor overflow (ofs: %u, len:%d, size:%zu)\n",
+                      __func__, s->tx_frame_len, len1, sizeof(s->tx_frame));
         return -1;
     }
     if (len1) {
@@ -593,6 +606,9 @@ static int tulip_copy_tx_buffers(TULIPState *s, struct tulip_descriptor *desc)
     }
 
     if (s->tx_frame_len + len2 > sizeof(s->tx_frame)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: descriptor overflow (ofs: %u, len:%d, size:%zu)\n",
+                      __func__, s->tx_frame_len, len2, sizeof(s->tx_frame));
         return -1;
     }
     if (len2) {
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect
  2020-04-23 23:16 [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539 Philippe Mathieu-Daudé
  2020-04-23 23:16 ` [RFC PATCH 1/3] hw/net/tulip: Fix 'Descriptor Error' definition Philippe Mathieu-Daudé
  2020-04-23 23:16 ` [RFC PATCH 2/3] hw/net/tulip: Log descriptor overflows Philippe Mathieu-Daudé
@ 2020-04-23 23:16 ` Philippe Mathieu-Daudé
  2020-04-24  2:16   ` Jason Wang
  2020-04-24 14:26   ` Eric Blake
  2020-04-24 15:27 ` [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539 Helge Deller
  3 siblings, 2 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-23 23:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Prasad J Pandit, Helge Deller, Li Qiang,
	Philippe Mathieu-Daudé,
	Li Qiang, Sven Schnelle, Jason Wang, Ziming Zhang

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



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect
  2020-04-23 23:16 ` [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect Philippe Mathieu-Daudé
@ 2020-04-24  2:16   ` Jason Wang
  2020-04-24 13:42     ` Helge Deller
  2020-04-24 14:26   ` Eric Blake
  1 sibling, 1 reply; 13+ messages in thread
From: Jason Wang @ 2020-04-24  2:16 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Prasad J Pandit, Helge Deller, Li Qiang, Li Qiang, Sven Schnelle,
	Ziming Zhang


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;



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect
  2020-04-24  2:16   ` Jason Wang
@ 2020-04-24 13:42     ` Helge Deller
  0 siblings, 0 replies; 13+ messages in thread
From: Helge Deller @ 2020-04-24 13:42 UTC (permalink / raw)
  To: Jason Wang, Philippe Mathieu-Daudé, qemu-devel
  Cc: Li Qiang, Sven Schnelle, Li Qiang, Prasad J Pandit, Ziming Zhang

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;
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect
  2020-04-23 23:16 ` [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect Philippe Mathieu-Daudé
  2020-04-24  2:16   ` Jason Wang
@ 2020-04-24 14:26   ` Eric Blake
  1 sibling, 0 replies; 13+ messages in thread
From: Eric Blake @ 2020-04-24 14:26 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Prasad J Pandit, Jason Wang, Li Qiang, Li Qiang, Sven Schnelle,
	Helge Deller, Ziming Zhang

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539
  2020-04-23 23:16 [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539 Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2020-04-23 23:16 ` [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect Philippe Mathieu-Daudé
@ 2020-04-24 15:27 ` Helge Deller
  2020-04-26  2:49   ` Jason Wang
  3 siblings, 1 reply; 13+ messages in thread
From: Helge Deller @ 2020-04-24 15:27 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Jason Wang, Helge Deller, Sven Schnelle, qemu-devel

* Philippe Mathieu-Daudé <f4bug@amsat.org>:
> Attempt to fix the launchpad bug filled by Helge:
>
>   In a qemu-system-hppa system, qemu release v5.0.0-rc,
>   the tulip nic driver is broken.  The tulip nic is detected,
>   even getting DHCP info does work.  But when trying to
>   download bigger files via network, the tulip driver gets
>   stuck.
>
> Philippe Mathieu-Daudé (3):
>   hw/net/tulip: Fix 'Descriptor Error' definition
>   hw/net/tulip: Log descriptor overflows
>   hw/net/tulip: Set descriptor error bit when lenght is incorrect
>
>  hw/net/tulip.h |  2 +-
>  hw/net/tulip.c | 32 ++++++++++++++++++++++++++++----
>  2 files changed, 29 insertions(+), 5 deletions(-)

Philippe, thanks for your efforts. Sadly your patch did not fixed the
bug itself, but it had some nice cleanups which should be included at
some point.

Regarding the tulip hang reported by me, the patch below does fix the
issue.

[PATCH] Fix tulip rx hang
Cc: Prasad J Pandit <pjp@fedoraproject.org>
Fixes: 8ffb7265af ("check frame size and r/w data length")
Buglink: https://bugs.launchpad.net/bugs/1874539
Signed-off-by: Helge Deller <deller@gmx.de>

Commit 8ffb7265af ("check frame size and r/w data length") introduced
checks to prevent accesses outside of the rx/tx buffers. But the new
checks were plain wrong. rx_frame_len does count backwards, and the
surrounding code ensures that rx_frame_len will not be bigger than
rx_frame_size. Remove those checks again.

diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index 1295f51d07..59d21defcc 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -171,9 +171,6 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
             len = s->rx_frame_len;
         }

-        if (s->rx_frame_len + len > sizeof(s->rx_frame)) {
-            return;
-        }
         pci_dma_write(&s->dev, desc->buf_addr1, s->rx_frame +
             (s->rx_frame_size - s->rx_frame_len), len);
         s->rx_frame_len -= len;
@@ -186,9 +183,6 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
             len = s->rx_frame_len;
         }

-        if (s->rx_frame_len + len > sizeof(s->rx_frame)) {
-            return;
-        }
         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;


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539
  2020-04-24 15:27 ` [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539 Helge Deller
@ 2020-04-26  2:49   ` Jason Wang
  2020-04-26  7:57     ` Peter Maydell
  0 siblings, 1 reply; 13+ messages in thread
From: Jason Wang @ 2020-04-26  2:49 UTC (permalink / raw)
  To: Helge Deller, Philippe Mathieu-Daudé; +Cc: Sven Schnelle, qemu-devel


On 2020/4/24 下午11:27, Helge Deller wrote:
> * Philippe Mathieu-Daudé <f4bug@amsat.org>:
>> Attempt to fix the launchpad bug filled by Helge:
>>
>>    In a qemu-system-hppa system, qemu release v5.0.0-rc,
>>    the tulip nic driver is broken.  The tulip nic is detected,
>>    even getting DHCP info does work.  But when trying to
>>    download bigger files via network, the tulip driver gets
>>    stuck.
>>
>> Philippe Mathieu-Daudé (3):
>>    hw/net/tulip: Fix 'Descriptor Error' definition
>>    hw/net/tulip: Log descriptor overflows
>>    hw/net/tulip: Set descriptor error bit when lenght is incorrect
>>
>>   hw/net/tulip.h |  2 +-
>>   hw/net/tulip.c | 32 ++++++++++++++++++++++++++++----
>>   2 files changed, 29 insertions(+), 5 deletions(-)
> Philippe, thanks for your efforts. Sadly your patch did not fixed the
> bug itself, but it had some nice cleanups which should be included at
> some point.
>
> Regarding the tulip hang reported by me, the patch below does fix the
> issue.
>
> [PATCH] Fix tulip rx hang
> Cc: Prasad J Pandit <pjp@fedoraproject.org>
> Fixes: 8ffb7265af ("check frame size and r/w data length")
> Buglink: https://bugs.launchpad.net/bugs/1874539
> Signed-off-by: Helge Deller <deller@gmx.de>
>
> Commit 8ffb7265af ("check frame size and r/w data length") introduced
> checks to prevent accesses outside of the rx/tx buffers. But the new
> checks were plain wrong. rx_frame_len does count backwards, and the
> surrounding code ensures that rx_frame_len will not be bigger than
> rx_frame_size. Remove those checks again.
>
> diff --git a/hw/net/tulip.c b/hw/net/tulip.c
> index 1295f51d07..59d21defcc 100644
> --- a/hw/net/tulip.c
> +++ b/hw/net/tulip.c
> @@ -171,9 +171,6 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
>               len = s->rx_frame_len;
>           }
>
> -        if (s->rx_frame_len + len > sizeof(s->rx_frame)) {
> -            return;
> -        }
>           pci_dma_write(&s->dev, desc->buf_addr1, s->rx_frame +
>               (s->rx_frame_size - s->rx_frame_len), len);
>           s->rx_frame_len -= len;
> @@ -186,9 +183,6 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
>               len = s->rx_frame_len;
>           }
>
> -        if (s->rx_frame_len + len > sizeof(s->rx_frame)) {
> -            return;
> -        }
>           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;
>

Looks good to me.

Would you please send a formal patch and cc Peter.

Consider we are about to release 5.0, it's better for him to apply the 
patch directly.

Thanks



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539
  2020-04-26  2:49   ` Jason Wang
@ 2020-04-26  7:57     ` Peter Maydell
  2020-04-27  3:32       ` Jason Wang
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Maydell @ 2020-04-26  7:57 UTC (permalink / raw)
  To: Jason Wang
  Cc: Helge Deller, Sven Schnelle, Philippe Mathieu-Daudé,
	QEMU Developers

On Sun, 26 Apr 2020 at 03:50, Jason Wang <jasowang@redhat.com> wrote:

> Looks good to me.
>
> Would you please send a formal patch and cc Peter.
>
> Consider we are about to release 5.0, it's better for him to apply the
> patch directly.

I am not applying any further patches for 5.0 unless they come
with an attached rock-solid justification for why we should
delay the release again for them.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539
  2020-04-26  7:57     ` Peter Maydell
@ 2020-04-27  3:32       ` Jason Wang
  2020-05-12  7:13         ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 13+ messages in thread
From: Jason Wang @ 2020-04-27  3:32 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Helge Deller, Sven Schnelle, Philippe Mathieu-Daudé,
	QEMU Developers


On 2020/4/26 下午3:57, Peter Maydell wrote:
> On Sun, 26 Apr 2020 at 03:50, Jason Wang<jasowang@redhat.com>  wrote:
>
>> Looks good to me.
>>
>> Would you please send a formal patch and cc Peter.
>>
>> Consider we are about to release 5.0, it's better for him to apply the
>> patch directly.
> I am not applying any further patches for 5.0 unless they come
> with an attached rock-solid justification for why we should
> delay the release again for them.
>
> thanks
> -- PMM


Ok.

I will queue that patch for 5.1.

Thanks


>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539
  2020-04-27  3:32       ` Jason Wang
@ 2020-05-12  7:13         ` Philippe Mathieu-Daudé
  2020-05-13  2:18           ` Jason Wang
  0 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-12  7:13 UTC (permalink / raw)
  To: Jason Wang, Peter Maydell; +Cc: Helge Deller, Sven Schnelle, QEMU Developers

Hi Jason,

On 4/27/20 5:32 AM, Jason Wang wrote:
> On 2020/4/26 下午3:57, Peter Maydell wrote:
>> On Sun, 26 Apr 2020 at 03:50, Jason Wang<jasowang@redhat.com>  wrote:
>>
>>> Looks good to me.
>>>
>>> Would you please send a formal patch and cc Peter.
>>>
>>> Consider we are about to release 5.0, it's better for him to apply the
>>> patch directly.
>> I am not applying any further patches for 5.0 unless they come
>> with an attached rock-solid justification for why we should
>> delay the release again for them.
>>
>> thanks
>> -- PMM
> 
> 
> Ok.
> 
> I will queue that patch for 5.1.

Can you queue patches #1 and #2?

> 
> Thanks
> 
> 
>>
> 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539
  2020-05-12  7:13         ` Philippe Mathieu-Daudé
@ 2020-05-13  2:18           ` Jason Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Wang @ 2020-05-13  2:18 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Peter Maydell
  Cc: Helge Deller, Sven Schnelle, QEMU Developers


On 2020/5/12 下午3:13, Philippe Mathieu-Daudé wrote:
> Hi Jason,
>
> On 4/27/20 5:32 AM, Jason Wang wrote:
>> On 2020/4/26 下午3:57, Peter Maydell wrote:
>>> On Sun, 26 Apr 2020 at 03:50, Jason Wang<jasowang@redhat.com>  wrote:
>>>
>>>> Looks good to me.
>>>>
>>>> Would you please send a formal patch and cc Peter.
>>>>
>>>> Consider we are about to release 5.0, it's better for him to apply the
>>>> patch directly.
>>> I am not applying any further patches for 5.0 unless they come
>>> with an attached rock-solid justification for why we should
>>> delay the release again for them.
>>>
>>> thanks
>>> -- PMM
>>
>>
>> Ok.
>>
>> I will queue that patch for 5.1.
>
> Can you queue patches #1 and #2?


Queued.

Thanks


>
>>
>> Thanks
>>
>>
>>>
>>
>>
>



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-05-13  2:19 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-23 23:16 [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539 Philippe Mathieu-Daudé
2020-04-23 23:16 ` [RFC PATCH 1/3] hw/net/tulip: Fix 'Descriptor Error' definition Philippe Mathieu-Daudé
2020-04-23 23:16 ` [RFC PATCH 2/3] hw/net/tulip: Log descriptor overflows Philippe Mathieu-Daudé
2020-04-23 23:16 ` [RFC PATCH 3/3] hw/net/tulip: Set descriptor error bit when lenght is incorrect Philippe Mathieu-Daudé
2020-04-24  2:16   ` Jason Wang
2020-04-24 13:42     ` Helge Deller
2020-04-24 14:26   ` Eric Blake
2020-04-24 15:27 ` [RFC PATCH 0/3] hw/net/tulip: Fix LP#1874539 Helge Deller
2020-04-26  2:49   ` Jason Wang
2020-04-26  7:57     ` Peter Maydell
2020-04-27  3:32       ` Jason Wang
2020-05-12  7:13         ` Philippe Mathieu-Daudé
2020-05-13  2:18           ` Jason Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.