Re: [PATCH v3 0/5] fs/iso9660: Fix out-of-bounds read

[Thomas Schmitt]

Hi,

Glenn Washburn wrote:
> Why does only one suffice? It
> sounds like they test different code paths. Is it possible that there
> is a future code regression such that one iso succeeds and the other
> fails?

They follow different code paths before hunk 4 of patch 5 fixes the
bug that CE and ST at the start of a continuation area are ignored:

@@ -331,6 +340,13 @@ grub_iso9660_susp_iterate (grub_fshelp_node_t node, 
grub_off_t off,
            return err;

          entry = (struct grub_iso9660_susp_entry *) sua;
+         /*
+          * The hook function will not process CE or ST.
+          * Advancing to the next entry would skip them.
+          */
+         if (grub_strncmp ((char *) entry->sig, "CE", 2) == 0
+             || grub_strncmp ((char *) entry->sig, "ST", 2) == 0)
+           continue;
        }

       if (hook (entry, hook_arg))

After this change, the first three hunks of patch 5 prevent that the
now common code path is an endless loop.

So a behavioral difference of ce_loop.iso and ce_loop2.iso is to expect
only if above patch hunk #4 gets reverted.


> Ok, so there should be no output on success then for both ce_loop and
>ce_loop2, correct? (for "grub-fstest <iso> ls /" )

Yes. Actually i would have expected an error message to be emitted.
But somehow grub-fstest does not show the text from:

+             return grub_error (GRUB_ERR_BAD_FS,
+                                "suspecting endless CE loop");


Have a nice day :)

Thomas