Re: [PATCH] net: eepro100: validate various address values

[Stefan Weil]

Am 19.02.21 um 09:08 schrieb Stefan Weil:

> Okay, I can confirm the infinite recursion now.
>
> The test case triggers memory writes by the hardware which cause new 
> actions of the same hardware and so on.
>
> I don't know how the real hardware would handle that case.
>
> For QEMU we can extend the current code which tries to prevent endless 
> loops: the device status EEPRO100State can be extended by a recursion 
> counter to limit the number of recursions, or maybe a boolean flag 
> could be used to stop any recursion of action_command(). I prefer the 
> second variant (no recursion at all) and suggest to add a diagnostic 
> message as well like it is done for the endless loop case.


If there are no recursions in normal use, the following patch should work:

diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 16e95ef9cc..2474cf3dc2 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -279,6 +279,9 @@ typedef struct {
     /* Quasi static device properties (no need to save them). */
     uint16_t stats_size;
     bool has_extended_tcb_support;
+
+    /* Flag to avoid recursions. */
+    bool busy;
 } EEPRO100State;

 /* Word indices in EEPROM. */
@@ -837,6 +840,14 @@ static void action_command(EEPRO100State *s)
        Therefore we limit the number of iterations. */
     unsigned max_loop_count = 16;

+    if (s->busy) {
+        /* Prevent recursions. */
+        logout("recursion in %s:%u\n", __FILE__, __LINE__);
+        return;
+    }
+
+    s->busy = true;
+
     for (;;) {
         bool bit_el;
         bool bit_s;
@@ -933,6 +944,7 @@ static void action_command(EEPRO100State *s)
     }
     TRACE(OTHER, logout("CU list empty\n"));
     /* List is empty. Now CU is idle or suspended. */
+    s->busy = false;
 }

 static void eepro100_cu_command(EEPRO100State * s, uint8_t val)