Re: [Qemu-devel] [Patch] eepro100 issues

[Stefan Weil]

The crashs are caused by transmit descriptors with packet sizes
larger than the buffer which is defined in the driver. Using netperf
or rsync will always result in a buffer overflow and crash QEMU
with the current eepro100 driver.

Up to now I could not find the reason for these large packet sizes.

My patch now just ignores data which does not fit in a normal frame.
Netperf and rsync no longer crash when the patch is applied.

Stefan



Juergen Lock schrieb:
> Hi again!
>
> I also played with the new eepro100 nic and found these things:
>
> 1. i82557b seem to work with
> http://debian.tu-bs.de/project/sidux/release/SIDUX-2007-01-200702210759-CHAOS.ISO
> (see
> http://www.sidux.com/Article116.html
> , it is based on debian sid), but
>
> 2. i82551 and i82559er with the same livecd sooner or later will cause
> qemu to crash like this with no backtrace available:

--- ../branches/head/hw/eepro100.c      2007-04-07 22:53:04.000000000 +0200
+++ hw/eepro100.c       2007-04-08 22:43:13.000000000 +0200
@@ -729,6 +729,7 @@
                 logout
                     ("TBD (simplified mode): buffer address 0x%08x, size 
0x%04x\n",
                      tx_buffer_address, tx_buffer_size);
+                assert(size + tx_buffer_size <= sizeof(buf));
                 cpu_physical_memory_read(tx_buffer_address, &buf[size],
                                          tx_buffer_size);
                 size += tx_buffer_size;
@@ -749,9 +750,13 @@
                         logout
                             ("TBD (extended mode): buffer address 0x%08x, size 
0x%04x\n",
                              tx_buffer_address, tx_buffer_size);
-                        cpu_physical_memory_read(tx_buffer_address, &buf[size],
-                                                 tx_buffer_size);
-                        size += tx_buffer_size;
+                        if (size + tx_buffer_size > sizeof(buf)) {
+                            logout("bad extended TCB with size 0x%04x\n", 
tx_buffer_size);
+                        } else {
+                            cpu_physical_memory_read(tx_buffer_address, 
&buf[size],
+                                                     tx_buffer_size);
+                            size += tx_buffer_size;
+                        }
                         if (tx_buffer_el & 1) {
                             break;
                         }
@@ -766,14 +771,20 @@
                     logout
                         ("TBD (flexible mode): buffer address 0x%08x, size 
0x%04x\n",
                          tx_buffer_address, tx_buffer_size);
-                    cpu_physical_memory_read(tx_buffer_address, &buf[size],
-                                             tx_buffer_size);
-                    size += tx_buffer_size;
+                    if (size + tx_buffer_size > sizeof(buf)) {
+                        logout("bad flexible TCB with size 0x%04x\n", 
tx_buffer_size);
+                    } else {
+                        cpu_physical_memory_read(tx_buffer_address, &buf[size],
+                                                 tx_buffer_size);
+                        size += tx_buffer_size;
+                    }
                     if (tx_buffer_el & 1) {
                         break;
                     }
                 }
             }
+            logout("%p sending frame, len=%d,%s\n", s, size, nic_dump(buf, 
size));
+            assert(size <= sizeof(buf));
             qemu_send_packet(s->vc, buf, size);
             s->statistics.tx_good_frames++;
             /* Transmit with bad status would raise an CX/TNO interrupt.