Package: hurd
Version: N/A

Hi,

I hope I don't make fuss about nothing, in case I don't understand the code.
Anyway, it looks strange.

In isofs/lookup.c (diskfs_get_directs), there is the first for-loop, which
seems to check if there are enough entries available.

I want to draw your attention to the "Ignore and skip RE entries" comment.

  /* Skip to ENTRY */
  dirbuf = disk_image + (dp->dn->file_start << store->log2_block_size);
  bufp = dirbuf;
  for (i = 0; i < entry; i ++)
    {
      struct rrip_lookup rr;

      ep = (struct dirrect *) bufp;
      rrip_lookup (ep, &rr, 0);

      /* Ignore and skip RE entries */
      if (rr.valid & VALID_RE)
        {
          bufp = bufp + ep->len;
          release_rrip (&rr);
          continue;
        }

     ....
   }

It seems as if those RE entries should not be counted. This is supported by
the following while-loop, which copies the valid entries into the return
buffer. However, this can't work! because a continue in the for-loop will
nevertheless increment the loop variable i. It looks as if the code was
cut&pasted from the while loop, or an earlier while loop was changed into a
for loop or so.

If I had to guess, I would add a i-- in the if-block.

But this is not all. The code looks suspicious in a second way, too.
The value the incremented bufp points to is not checked for null.
I don't know the ISO9660 standard, but if it could be that a RE entry is at
the end of a logical sector, this code will overrun, because the validity
check at the end of the for-loop is skipped (because of the continue):

      ...

      bufp = bufp + ep->len;

      /* If BUFP points at a null, then we have hit the last
         record in this logical sector.  In that case, skip up to
         the next logical sector. */
      if (*(char *)bufp == '\0')
        bufp = (void *) (((long) bufp & ~(logical_sector_size - 1))
                         + logical_sector_size);
    }

If my analysis is true, the code needs to be rearranged a bit.

Thanks,
Marcus

-- 
`Rhubarb is no Egyptian god.' Debian http://www.debian.org Check Key server 
Marcus Brinkmann              GNU    http://www.gnu.org    for public PGP Key 
[EMAIL PROTECTED],     [EMAIL PROTECTED]    PGP Key ID 36E7CD09
http://homepage.ruhr-uni-bochum.de/Marcus.Brinkmann/       [EMAIL PROTECTED]

Reply via email to