Patchwork rtl8139: validate rx ring before receiving packets

login
register
mail settings
Submitter Jason Wang
Date May 17, 2012, 5:25 a.m.
Message ID <20120517052542.17932.58011.stgit@dhcp-8-146.nay.redhat.com>
Download mbox | patch
Permalink /patch/159811/
State New
Headers show

Comments

Jason Wang - May 17, 2012, 5:25 a.m.
Commit ff71f2e8cacefae99179993204172bc65e4303df prevent the possible
crash during initialization of linux driver by checking the operating
mode.This seems too strict as:

- the real card could still work in mode other than normal
- some buggy driver who does not set correct opmode after eeprom
 access

So, considering rx ring address were reset to zero (which could be
safely trated as an address not intened to DMA to), in order to
both letting old guest work and preventing the unexpected DMA to
guest, we can forbid packet receiving when rx ring address is zero.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/rtl8139.c |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)
Avi Kivity - June 27, 2012, 1:33 p.m.
On 05/17/2012 08:25 AM, Jason Wang wrote:
> Commit ff71f2e8cacefae99179993204172bc65e4303df prevent the possible
> crash during initialization of linux driver by checking the operating
> mode.This seems too strict as:
> 
> - the real card could still work in mode other than normal
> - some buggy driver who does not set correct opmode after eeprom
>  access
> 
> So, considering rx ring address were reset to zero (which could be
> safely trated as an address not intened to DMA to), in order to
> both letting old guest work and preventing the unexpected DMA to
> guest, we can forbid packet receiving when rx ring address is zero.
> 

Tested-by: Avi Kivity <avi@redhat.com>

This unbreaks kvm-autotest.
Anthony Liguori - June 27, 2012, 9:33 p.m.
On 05/17/2012 12:25 AM, Jason Wang wrote:
> Commit ff71f2e8cacefae99179993204172bc65e4303df prevent the possible
> crash during initialization of linux driver by checking the operating
> mode.This seems too strict as:
>
> - the real card could still work in mode other than normal
> - some buggy driver who does not set correct opmode after eeprom
>   access
>
> So, considering rx ring address were reset to zero (which could be
> safely trated as an address not intened to DMA to), in order to
> both letting old guest work and preventing the unexpected DMA to
> guest, we can forbid packet receiving when rx ring address is zero.
>
> Signed-off-by: Jason Wang<jasowang@redhat.com>

Applied.  Thanks.

Regards,

Anthony Liguori

> ---
>   hw/rtl8139.c |   22 ++++++++++++----------
>   1 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index eb22d04..3879121 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -781,6 +781,13 @@ static inline dma_addr_t rtl8139_addr64(uint32_t low, uint32_t high)
>   #endif
>   }
>
> +/* Workaround for buggy guest driver such as linux who allocates rx
> + * rings after the receiver were enabled. */
> +static bool rtl8139_cp_rx_valid(RTL8139State *s)
> +{
> +    return !(s->RxRingAddrLO == 0&&  s->RxRingAddrHI == 0);
> +}
> +
>   static int rtl8139_can_receive(VLANClientState *nc)
>   {
>       RTL8139State *s = DO_UPCAST(NICState, nc, nc)->opaque;
> @@ -791,11 +798,8 @@ static int rtl8139_can_receive(VLANClientState *nc)
>         return 1;
>       if (!rtl8139_receiver_enabled(s))
>         return 1;
> -    /* network/host communication happens only in normal mode */
> -    if ((s->Cfg9346&  Chip9346_op_mask) != Cfg9346_Normal)
> -	return 0;
>
> -    if (rtl8139_cp_receiver_enabled(s)) {
> +    if (rtl8139_cp_receiver_enabled(s)&&  rtl8139_cp_rx_valid(s)) {
>           /* ??? Flow control not implemented in c+ mode.
>              This is a hack to work around slirp deficiencies anyway.  */
>           return 1;
> @@ -836,12 +840,6 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
>           return -1;
>       }
>
> -    /* check whether we are in normal mode */
> -    if ((s->Cfg9346&  Chip9346_op_mask) != Cfg9346_Normal) {
> -        DPRINTF("not in normal op mode\n");
> -        return -1;
> -    }
> -
>       /* XXX: check this */
>       if (s->RxConfig&  AcceptAllPhys) {
>           /* promiscuous: receive all */
> @@ -946,6 +944,10 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
>
>       if (rtl8139_cp_receiver_enabled(s))
>       {
> +        if (!rtl8139_cp_rx_valid(s)) {
> +            return size;
> +        }
> +
>           DPRINTF("in C+ Rx mode ================\n");
>
>           /* begin C+ receiver mode */
>
>
>
>

Patch

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index eb22d04..3879121 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -781,6 +781,13 @@  static inline dma_addr_t rtl8139_addr64(uint32_t low, uint32_t high)
 #endif
 }
 
+/* Workaround for buggy guest driver such as linux who allocates rx
+ * rings after the receiver were enabled. */
+static bool rtl8139_cp_rx_valid(RTL8139State *s)
+{
+    return !(s->RxRingAddrLO == 0 && s->RxRingAddrHI == 0);
+}
+
 static int rtl8139_can_receive(VLANClientState *nc)
 {
     RTL8139State *s = DO_UPCAST(NICState, nc, nc)->opaque;
@@ -791,11 +798,8 @@  static int rtl8139_can_receive(VLANClientState *nc)
       return 1;
     if (!rtl8139_receiver_enabled(s))
       return 1;
-    /* network/host communication happens only in normal mode */
-    if ((s->Cfg9346 & Chip9346_op_mask) != Cfg9346_Normal)
-	return 0;
 
-    if (rtl8139_cp_receiver_enabled(s)) {
+    if (rtl8139_cp_receiver_enabled(s) && rtl8139_cp_rx_valid(s)) {
         /* ??? Flow control not implemented in c+ mode.
            This is a hack to work around slirp deficiencies anyway.  */
         return 1;
@@ -836,12 +840,6 @@  static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
         return -1;
     }
 
-    /* check whether we are in normal mode */
-    if ((s->Cfg9346 & Chip9346_op_mask) != Cfg9346_Normal) {
-        DPRINTF("not in normal op mode\n");
-        return -1;
-    }
-
     /* XXX: check this */
     if (s->RxConfig & AcceptAllPhys) {
         /* promiscuous: receive all */
@@ -946,6 +944,10 @@  static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
 
     if (rtl8139_cp_receiver_enabled(s))
     {
+        if (!rtl8139_cp_rx_valid(s)) {
+            return size;
+        }
+
         DPRINTF("in C+ Rx mode ================\n");
 
         /* begin C+ receiver mode */