kern/48815: mount_cd9660 option -o "norrip,gens" is quite unusable

[scdbackup%gmx.net@localhost]

>Number:         48815
>Category:       kern
>Synopsis:       mount_cd9660 option -o "norrip,gens" is quite unusable
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat May 17 14:50:00 +0000 2014
>Originator:     Thomas Schmitt
>Release:        6.1.3
>Organization:
>Environment:
NetBSD netbsd 6.1.3 NetBSD 6.1.3 (GENERIC) i386
>Description:
The options "norrip,gens" of mount_cd9660(8) cause peculiar behavior
of several shell tools, among them ls with not-working option -l.

ISO 9660 filenames end by a semicolon and a version number.
E.g. "SMALL_FILE.;1".
Usually these suffixes are not shown and not taken into respect
when looking up file names in the directory tree.

If Rock Ridge with its POSIX compliant file names is disabled,
and if option "gens" (ISOFSMNT_GENS) is in effect, then the
non-POSIX names are shown with semicolon and version number.

But they still cannot be looked up by names with such suffix.

The cause is this code snippet from sys/fs/cd9660/cd9660_util.c
function isofncmp(). It runs after both compared names matched
up to the first semicolon.
Now the version numbers are to be compared.

        fn++;
        for (i = 0; fnlen-- != 0; i = i * 10 + *fn++ - '0') {
                if (*fn < '0' || *fn > '9') {
                        return -1;
                }
        }

It increments fn, obviously to skip the semicolon. But this has been
done already by the call to static int wget() which produced the
16 bit variable which beared the semicolon.
Further, the third statement in for() increments fn again, before
the loop body can compare it with the digit range of ASCII.

So this is probably off track with its memory access by two bytes,
before it returns -1.

>How-To-Repeat:
It happens with any ISO 9660 filesystem which bears data files
(directories are supposed to have no version suffix):

  netbsd# mount_cd9660 -o norrip,nojoliet,gens /dev/cd0a /mnt/iso

  netbsd# ls /mnt/iso
  my              small_file.;1

  netbsd# ls -l /mnt/iso
  ls: small_file.;1: No such file or directory
  total 4
  dr-xr-xr-x  1 root  wheel  2048 May  6 15:30 my

  netbsd# ls /mnt/iso/small*
  ls: /mnt/iso/small_file.;1: No such file or directory

  netbsd# cat /mnt/iso/'small_file.;1' >/dev/null
  cat: /mnt/iso/small_file.;1: No such file or directory

The state is not totally useless, though:

  netbsd# cat /mnt/iso/small_file. | wc
         1       1       6

After the fix, the mounted test ISO yields:

  netbsd# ls -l /mnt/iso
  total 4
  dr-xr-xr-x  1 root  wheel  2048 May  6 15:30 my
  -r-xr-xr-x  1 root  wheel     6 May  6 15:34 small_file.;1

>Fix:
I believe it should be something like

        for (i = 0; fnlen-- != 0; fn++) {
                if (*fn < '0' || *fn > '9')
                        return -1;
                i = i * 10 + *fn - '0';
        }

although i'm still somewhat unhappy with the independent activities
on fnlen and fn. But the loop count seems to be ok. So i don't fiddle
with the second statement in for().

Patch proposal follows.