Bug in fs/cd9660 raises questions about inode number computing

["Thomas Schmitt" <scdbackup%gmx.net@localhost>]

Hi,

i am exploring a bug in fs/cd9660.
A fix seems straightforward after i found its 32 bit rollover
trigger in the code.

But my exploration raises questions:

- Is the inode computation in sys/fs/cd9660/cd9660_node.c:isodirino()
  faulty (additionally to its rollover) ?

- Are the inode numbers of cd9660 too 64-bit-happy ?

-----------------------------------------------------------------

The bug shows up when mounting a multi-session ISO 9660 medium
which has its directory tree above 4 GiB. (Not possible with CD,
but with DVD or BD.)

  netbsd# mount -t cd9660 /dev/cd0a /mnt
  netbsd# ls -l /mnt
  -r-xr-xr-x  1 root  wheel  0 Jan  1  1970 /mnt
  netbsd# umount /mnt
  umount: /mnt: Invalid argument
  netbsd# umount /dev/cd0a
  umount: /mnt: Invalid argument
  netbsd# mount
  /dev/wd0a on / type ffs (local)
  kernfs on /kern type kernfs (local)
  ptyfs on /dev/pts type ptyfs (local)
  procfs on /proc type procfs (local)
  /dev/cd0a on /mnt type cd9660 (read-only, local)

I can get rid of the mount point only by reboot.

This does not happen with

  mount -t cd9660 -o norrip /dev/cd0a /mnt

making it clear that the bug sits in the Rock Ridge
interpretation. I did not find this spot yet.

But looking for possible 32 bit byte address rollovers i found
the initiating culprit in sys/fs/cd9660/cd9660_node.c :

  ino_t
  isodirino(struct iso_directory_record *isodir, struct iso_mnt *imp)
  {
        ino_t ino;

        ino = (isonum_733(isodir->extent) + isonum_711(isodir->ext_attr_length))
              << imp->im_bshift;
        return (ino);
  }

Although sizeof(ino_t) == 8, this rolls over at least on NetBSD i386
because none of the operands of the computation is 64 bit wide.

A test in userspace confirms:

  sizeof(ino_t) = 8
  ( 2113952 + 0 ) << 11 = 34406400
  ( ((uint64_t) 2113952) + 0 ) << 11 = 4329373696

Fixing the rollover makes the ISO mountable and umountable:

     ino = (((uint64_t) isonum_733(isodir->extent)) +
            isonum_711(isodir->ext_attr_length)) << imp->im_bshift;

---------------------------------------------------------------

The rollover seems not to be the only cause for the bad mount.
It would probably be harmless if not other inode computations
in cd9660 would have a 64 bit operand in their own expressions.

I suspect this causes inode numbers to be computed differently
on the same inode. Rock Ridge code obviously takes offense.

At least i can trigger the same problem without rollover just
by dividing the inode numbers of some computations by 32.
(This would still keep them unique because a directory record
 has at least 34 bytes. Idea learned from Linux iso9660.)

I tried to push the 32 bit limit to 128 TiB by this division,
because ls(1) option -i shows rolled over 32 bit numbers.
Probably other consumers of stat(2) don't expect true 64 bit
either.
I will try to consolidate all inode computations by a single macro
and then look whether it runs with division by 32.

And in general, what strange formula is used in isodirino() ?

My comment in my copy of NetBSD 6.99.40 says:

   This looks strange. Why add a byte offset to the block address
   and then multiply the sum by block size ?
        ino = (isonum_733(isodir->extent) +
               isonum_711(isodir->ext_attr_length)) << imp->im_bshift;

   One could fear that (isodir->extent << imp->im_bshift) coincides
   with the first directory entry which is not ".". So adding any
   number between 1 and 33 after multiplication/shifting would
   prevent the result from coinciding with the first or second entry.
   Well, ext_attr_length is normally 0 and "." is implicitely specified
   to be the first entry in a directory (by ECMA-119 6.8.2.2 and 9.3).

   The other inode computations look more plausible
     dbtob(bp->b_blkno) + entryoffsetinblock

So i changed the formula in isodirino() to what the others do.
It seems to work properly. Actually trivially because ext_attr_length
is 0 in my test ISO.

---------------------------------------------------------------

Because this mail is not yet long enough:

I am the upstream developer of libburn and libisofs.
NetBSD user since a few weeks by a virtual 6.1.3-i386 (with a
6.99.40 kernel meanwhile).
No kernel background, but ISO 9660 and SCSI/MMC for CD burners.

I recently ported libburn to NetBSD. My daring test user and
my friendly pkgsrc packager kept me interested in NetBSD cd9660
although i myself cannot get installed NetBSD on my real hardware
and thus have no real DVD drive to play with. Only qemu CD-ROM.

More proposals are to come, if you do not chase me away:

- Files larger than 4 GiB are misrepresented after mount_cd9660.

- mount_cd9660(8) lacks an option equivalent to FreeBSD's
  mount_cd9660 -s.

- Selfishly i could need ioctl(SCIOCCOMMAND) with ld(4) populated
  by virtio(4) so that i get a real DVD burner forwarded into my
  NetBSD. (virtio /dev/ld0d on DVD+RW writes with 16 KB/s. Ugh.)
  
---------------------------------------------------------------

Have a nice day :)

Thomas