Subject: kern/4665: scsi cd unable to play the last audio track because of fencepost error
To: None <gnats-bugs@gnats.netbsd.org>
From: Chris Baird <cjb@brushtail.apana.org.au>
List: netbsd-bugs
Date: 12/11/1997 15:07:16
>Number:         4665
>Category:       kern
>Synopsis:       scsi cd unable to play the last audio track because of fencepost error
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Dec 10 23:05:00 1997
>Last-Modified:
>Originator:     Chris Baird
>Organization:
Weaselworks Europa P/L
>Release:        1.3_ALPHA 26 Nov 1997
>Environment:
System: NetBSD brushtail.apana.org.au 1.3_ALPHA NetBSD 1.3_ALPHA (BRUSHTAIL) #2: Tue Dec 9 12:58:09 EST 1997 root@brushtail.apana.org.au:/export/src/NetBSD-1.3A/src/sys/arch/i386/compile/BRUSHTAIL i386

>Description:

It was a cold and stormy night.  Being a cold and stormy night, this
would allow the girlfriend and myself the chance to listening to our
favourite audio CDs at a volume a bit louder than the usual (it being
cold and stormy, none of the neighbours would be able to hear the
racket over the weather :).  But the housemate who owns the stereo
system had earlier that night departed with the CD-player, and now
the night's entertainment possibilities was all depending on a humble
NetBSD system and some speaker cables strung across the room...

I've (we've) discovered a "fencepost" error in cd_play_tracks() that
prevents using the CDIOCPLAYTRACKS ioctl to play the last track on an
audio CD.

sys/dev/scsipi/cd.c/cd_play_track() calls cd_play_msf() with the range
of blocks to play as "starttrack[0],(++endtrack)[0]"-- the last argument
referring to a block that does not physically exist (even though the
CD's TOC mentions it).  The code should most likely want to pass the
range "starttrack[0],endtrack[len]" instead.

For an example, attempting to play the last audio track of NIN's Halo11
("Oh Gomez, they're playing our song!"), will call cd_play_msf() with
the parameters:

    cd_play_msf(cd, startmin=34, startdec=36, startframe=5, \
                    endmin=39, endsec=53, endframe=34)

..and report a "Logical Block Address Out of Range".  Another micky
program passing arguments directly to cd_play_msf() proved that the last
actual block on the disc was (m39,s53,f33).

This problem was verified with several other audio CDs from different
genres (TISM, Peter Gabriel, Enya..) and the problem did not exist in
the NetBSD-1.2 kernel (from which I upgraded on the 1st of December).

And if it is relevant, the CDROM drive is a single-spin external SCSI
drive on loan from a Sun IPX workstation ("model 411").


>How-To-Repeat:

Place an audio CD into a audio-capable scsi cd device and attempt to
play the last track on the disc with a controller program that relies on
cd_play_tracks().  Here's the simple program I've been using...

/* commands a scsi cdrom device to play audio. Probably NetBSD specific.
 * --
 * usage: cdplay [-s startingtrack] [-e endtrack] [-d device] [-i]
 * Defaults to playing the entire disc in cd0. Plays a single track if just
 * the starting track specified, otherwise plays all tracks in range s to e.
 * "-i" reports the number of tracks on the disc.
 * --
 * [$Id]
 * Chris Baird,, <cjb@brushtail.apana.org.au> Released to the public domain.
 */

#include <sys/ioctl.h>
#include <sys/types.h>
#include <sys/cdio.h>
#include <err.h>
#include <stdio.h>
#include <unistd.h>

extern int errno;
extern const char *const sys_errlist[];
extern char *optarg;
extern int optind;

main(int argc, char* argv[])
{
    struct ioc_play_track cmd;
    struct ioc_toc_header tocheader;
    struct ioc_vol volume[4];
    char cddev[32] = "/dev/cd0a";
    int s=0, e=0, v=255, i=0, c;
    FILE* d;

    while ((c = getopt(argc, argv, "iv:s:e:d:")) != -1)
	switch(c) {
	case 'i':
	    i = 1;
	    break;
	case 'v':
	    v = atoi(optarg);
	    break;
	case 's':
	    s = atoi(optarg);
	    break;
	case 'e':
	    e = atoi(optarg);
	    break;
	case 'd':
	    strncpy (cddev+5, optarg, 26);
	    break;
	}

    if ((d = fopen(cddev, "r")) == NULL)
	err(1, NULL);

    if ((ioctl (d->_file, CDIOREADTOCHEADER, &tocheader)) == -1)
	err(1, NULL);

    if (i != 0)
    {
	fprintf (stderr, "start_track=%d, end_track=%d, len=%d\n",
		 tocheader.starting_track, tocheader.ending_track, tocheader.len);
	exit(0);
    }

    cmd.start_index = 1;
    cmd.end_index = 1;
    cmd.start_track = tocheader.starting_track;
    cmd.end_track = tocheader.ending_track;

    if (s != 0)
	if ((s < tocheader.starting_track) || (s > tocheader.ending_track))
	    errx (1, "bad starting track %d", s);
	else
	{
	    cmd.start_track = s;
	    cmd.end_track = s;
	}

    if (e != 0)
	if ((e < tocheader.starting_track) || (e > tocheader.ending_track) || (e < s))
	    errx (1, "bad ending track %d", e);
	else
	    cmd.end_track = e;

    if ((v >= 0) && (v <= 255))
    {
	volume->vol[0] = volume->vol[1] = volume->vol[2] = volume->vol[3] = v;
	ioctl (d->_file, CDIOCSETVOL, &volume);
    }

    if ((ioctl (d->_file, CDIOCPLAYTRACKS, &cmd)) == -1)
	err (1, NULL);

    fclose (d);
}



>Fix:

cd_play_track() should call cd_play_msf() with the range of blocks to
be played as "starttrack[0],endtrack[len]", rather than the existing
"starttrack[0],(++endtrack)[0]"

>Audit-Trail:
>Unformatted: