From nobody@FreeBSD.ORG Mon Oct 23 10:10:34 2000 Return-Path: Received: by hub.freebsd.org (Postfix, from userid 32767) id 70DB537B479; Mon, 23 Oct 2000 10:10:34 -0700 (PDT) Message-Id: <20001023171034.70DB537B479@hub.freebsd.org> Date: Mon, 23 Oct 2000 10:10:34 -0700 (PDT) From: daveg@chiaro.com Sender: nobody@FreeBSD.ORG To: freebsd-gnats-submit@FreeBSD.org Subject: Incorrect handling of end-of-media in atapi cdrom driver X-Send-Pr-Version: www-1.0 >Number: 22245 >Category: kern >Synopsis: Incorrect handling of end-of-media in atapi cdrom driver >Confidential: no >Severity: critical >Priority: high >Responsible: sos >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Mon Oct 23 10:20:00 PDT 2000 >Closed-Date: Mon May 28 11:54:03 PDT 2001 >Last-Modified: Mon May 28 11:55:10 PDT 2001 >Originator: Dave Gillam >Release: 4.0 >Organization: Chiaro Networks >Environment: FreeBSD bsd3.chiaro.com 4.0-RELEASE FreeBSD 4.0-RELEASE #0: Tue Sep 5 10:38:30 CDT 2000 daveg@bsd3.chiaro.com:/usr/src/sys/compile/bsd3 i386 >Description: The atapi cdrom driver incorrecly handles the end-of-media (EOM) condition. It can attempt to read one block past the EOM when it is reading the last file on the CD. >How-To-Repeat: 1. create a cd with one file on it that is 1343775 bytes long dd if=/dev/zero of=/tmp/f bs=1343775 count=1 mkisofs -A "cd" -D -l -J -v -r -o /tmp/cdimage.raw /tmp/f cdrecord dev=0,0,0 speed=4 -multi -v /tmp/cdimage.raw 2. copy to file from the cd using /bin/cp (and cp must be compiled with -DVM_AND_BUFFER_CACHE_SYNCHRONIZED, which 4.0 is) the cp command will fail because: - since the file is less than 8meg, cp mmaps the file, which causes the the vnode pager to page in the file. - the block size of the cd is 2k and the page size is 4k. the file ends in the first 2k of the last page. the last page in of the file will result in two reads. - the first one works fine (it is the last block on the cd and it contains 287 bytes of file data). the next one attempts to read past the EOM. it should just return EOF, but instead a read command is issued to the controller. the read gets an error and the following is printed in the console: acd0: READ_BIG - ILLEGAL REQUEST asc=64 ascq=00 error=01 - the error is passed up to the vnode pager in the struct buf. this causes the page in to fail and the vnode pager prints the following on the console: vm_fault: pager read error, pid 292 (cp) - since the page in fails, the cp fails. Using dd or the like will not detect this problem since they do not attempt to read past the EOM like the vnode pager can. >Fix: this is a classic off-by-one error on the EOM detection code in acd_start() *** atapi-cd.c 2000/05/08 23:00:21 1.1 --- atapi-cd.c 2000/10/23 16:52:57 *************** *** 1117,1125 **** if (bp->b_flags & B_READ) { /* if transfer goes beyond EOM adjust it to be within limits */ ! if (lba + count > cdp->info.volsize) { /* if we are entirely beyond EOM return EOF */ ! if ((count = cdp->info.volsize - lba) <= 0) { bp->b_resid = bp->b_bcount; biodone(bp); return; --- 1117,1139 ---- if (bp->b_flags & B_READ) { /* if transfer goes beyond EOM adjust it to be within limits */ ! ! /* ! * The value in cdp->info.volsize is not the last LBA as the name ! * would lead you to believe. It is really the next available (empty) ! * LBA. Also the cdp->info.volsize is numbered 1 to N, not 0 to ! * (N-1). For example, if you have a disk that has 687 2K blocks of ! * data on it the LBA's are numbered 0 to 686. The physical blocks ! * used on the cdrom are 1 to 687 and the value in cdp->info.volsize ! * is 688. ! * ! * So the check needs to be (lba + count >= volsize) and (count = ! * (volsize - 1) - lba). Not (lba + count > volsize) and (count = ! * volsize - lba) which is what FreeBSD 4.0 has. ! */ ! if (lba + count >= cdp->info.volsize) { /* if we are entirely beyond EOM return EOF */ ! if ((count = (cdp->info.volsize - 1) - lba) <= 0) { bp->b_resid = bp->b_bcount; biodone(bp); return; >Release-Note: >Audit-Trail: State-Changed-From-To: open->analyzed State-Changed-By: sos State-Changed-When: Mon Oct 23 12:37:24 PDT 2000 State-Changed-Why: Hmm, well, the _real_ problem is the size put in volinfo, this is unfortunately not done equal by all drives, some gives the number of blocks offset 1 some offset 0, so the fix in the PR is just as wrong as the current code. I guess there is no real solution to this, other than trying the last block and see what gives, the drive could hide the error and let the rest of the system live happily... http://www.freebsd.org/cgi/query-pr.cgi?pr=22245 Responsible-Changed-From-To: freebsd-bugs->sos Responsible-Changed-By: kris Responsible-Changed-When: Fri May 25 03:25:23 PDT 2001 Responsible-Changed-Why: sos is Mr ATA http://www.FreeBSD.org/cgi/query-pr.cgi?pr=22245 State-Changed-From-To: analyzed->closed State-Changed-By: sos State-Changed-When: Mon May 28 11:54:03 PDT 2001 State-Changed-Why: This is belived to be fixed in -current some time ago http://www.FreeBSD.org/cgi/query-pr.cgi?pr=22245 >Unformatted: