runtime_events_consumer: guard agains short msg_length

msg_length zero could cause us to loop infinitely.

All messages have at least a header and a timestamp,
except EV_INTERNAL (0), which is a padding event.

Extend the testcase by corrupting message lengths.

This also avoids some memory sanitizer warnings.

Rejecting all msg_length < 2 would cause a failure in
test_dropped_events.ml, because padding events would be incorrectly
rejected.

Suggested-by: Miod Vallat <[email protected]>
Signed-off-by: Edwin Török <[email protected]>

Author: edwintorok

otherlibs/runtime_events/runtime_events_consumer.c

@@ -462,6 +462,13 @@ caml_runtime_events_read_poll(struct caml_runtime_events_cursor *cursor,
         continue;
       }
 
+      if (!msg_length
+          || (msg_length < 2
+              && RUNTIME_EVENTS_ITEM_TYPE(header) != EV_INTERNAL)) {
+        atomic_store(&cursor->cursor_in_poll, 0);
+        return E_CORRUPT_STREAM;
+      }
+
       if (RUNTIME_EVENTS_ITEM_IS_RUNTIME(header)) {
         switch (RUNTIME_EVENTS_ITEM_TYPE(header)) {
         case EV_BEGIN:

testsuite/tests/lib-runtime-events/test_corrupted.ml

@@ -114,5 +114,18 @@
            due to bounds error on an earlier offset
          *)
         Bytes.blit_string original 0 buf 0 (Bytes.length buf);
-      done
+        parse_corrupted path_pid
+      done;
+      for is_runtime = 0 to 1 do
+        for event_type = 0 to 15 (* event type is 4 bits *) do
+          for event_id = 0 to 64 (* event_id is 13 bits, but not all used yet *) do
+            for length = 0 to 1 (* short lengths trigger uninit read bugs *) do
+                (* modify just 1 event in the otherwise valid ring *)
+                write_event_header is_runtime event_type event_id length;
+                (* parse ring *)
+                parse_corrupted path_pid
+            done
+          done
+        done;
+      done;
     end