Re: [PATCH 1/4] fs/iso9660: Add check to prevent infinite loop

[Thomas Schmitt]

Hi,

i wrote:
> > (Are we aware of the file size limit of 32 GiB - 14 KiB - 1 imposed by
> >   struct grub_fshelp_node { ... struct grub_iso9660_dir dirents[8]; ... }
> > ? )

Lidong Chen wrote:
> I am not familiar with this file size limit. Do we need to add a check
> somewhere?

Good question. The answer probably disproves my statement because the
struct definition seems not to match exactly its usage:

Assessment happens in grub_iso9660_iterate_dir():

        while (dirent.flags & FLAG_MORE_EXTENTS)
          {
            ...
            if (node->have_dirents >= node->alloc_dirents)
              {

At this point an overflow of currently allocated .dirents[] was detected.

                struct grub_fshelp_node *new_node;
                grub_size_t sz;

                if (grub_mul (node->alloc_dirents, 2, &node->alloc_dirents) ||
                    grub_sub (node->alloc_dirents, ARRAY_SIZE (node->dirents), 
&sz) ||
                    grub_mul (sz, sizeof (node->dirents[0]), &sz) ||
                    grub_add (sz, sizeof (struct grub_fshelp_node), &sz))
                  goto fail_0;

                new_node = grub_realloc (node, sz);

I understand the computations in the if-clause as:
- The number of allocated dirents is doubled.
- The new_node size is the size of the new number of .dirents minus 8
  .dirent sizes for the eight .dirents which are part of the
  grub_fshelp_node definition,
- plus the defined size of the grub_fshelp_node.

The new_node gets allocated with that size, which provides enough space
for the new dirent and many of its potential successors.


So i retract my statement. Data file size seems quite unlimited.
At some point grub_mul() or grub_realloc() will throw an error if the number
of .dirents is too high for grub_size_t or the machine's memory.


Have a nice day :)

Thomas