bin/98005: loader corrupts other files when rewriting nextboot.conf

[Ian Dowse]

The following reply was made to PR bin/98005; it has been noted by GNATS.

From: Ian Dowse <iedowse at iedowse.com>
To: Yar Tikhiy <yar at comp.chem.msu.su>
Cc: FreeBSD-gnats-submit at FreeBSD.org
Subject: Re: bin/98005: loader corrupts other files when rewriting nextboot.conf 
Date: Sat, 27 May 2006 13:05:01 +0100

 In message <200605271037.k4RAbtYw063346 at comp.chem.msu.su>, Yar Tikhiy writes:
 >	When nextboot is in effect, loader(8) writes the modified
 >	contents of the nextboot.conf file, or whatever nextboot_conf
 >	is set to, to an incorrect location on the disk.  The
 >	location overwritten is in the block after the one actually
 >	belonging to the nextboot.conf file.
 >
 >	This problem is likely to be caused by an off-by-one bug in
 >	the stand-alone FS access library used by loader(8).
 
 You could try the following (I haven't tested it), but it's pretty
 obvious how the bug happened if you compare bd_write() with the
 bd_read() function that it was copied from. Looks like the author
 of bd_write() was more interested in writing a little song in the
 comments than writing to the correct part of the disk ;-)
 
 The bug probably wasn't noticed originally because it only affected
 the LBA access case.
 
 Ian
 
 Index: i386/libi386/biosdisk.c
 ===================================================================
 RCS file: /dump/FreeBSD-CVS/src/sys/boot/i386/libi386/biosdisk.c,v
 retrieving revision 1.46
 diff -u -r1.46 biosdisk.c
 --- i386/libi386/biosdisk.c	19 Dec 2005 09:00:11 -0000	1.46
 +++ i386/libi386/biosdisk.c	27 May 2006 11:53:34 -0000
 @@ -1037,9 +1037,6 @@
  	*/
  	if (bbuf != NULL)
  	    bcopy(p, breg, x * BIOSDISK_SECSIZE);
 -	p += (x * BIOSDISK_SECSIZE);
 -	dblk += x;
 -	resid -= x;
  
  	/* Loop retrying the operation a couple of times.  The BIOS may also retry. */
  	for (retry = 0; retry < 3; retry++) {
 @@ -1103,6 +1100,9 @@
  	if (result) {
  	    return(-1);
  	}
 +	p += (x * BIOSDISK_SECSIZE);
 +	dblk += x;
 +	resid -= x;
      }
  	
  /*    hexdump(dest, (blks * BIOSDISK_SECSIZE)); */