--/04w6evG8XlLl3ft
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Dear folks,

as part of my challenge to get a good and working CD disklabel correctly i 
stumbled on a note in scsipi/cd.c :

#ifdef notyet /* have to fix bounds_check_with_label() first */
        lp->d_partitions[0].p_size = lp->d_secperunit;
#else
        lp->d_partitions[0].p_size =
            lp->d_secperunit * (lp->d_secsize / DEV_BSIZE);
#endif
        lp->d_partitions[0].p_cdsession = lastsession;
        lp->d_partitions[0].p_fstype = FS_ISO9660;

Having looked at the `bounds_check_with_label()' implementations (MD) for 
all arch's i stumbled on the following:

a) all archs have a MD function apart from amd64, i386, xen, ibmnws who use 
   kern/subr_disk_mbr.c

b) apart from some obvious small errors and mechanical missers, i stubled 
   on two flavours of cylinder calculations for disksort :

        bp->b_cylinder = (bp->b_blkno + p->p_offset) / lp->d_secpercyl;
   and
	bp->b_cylinder = (bp->b_blkno + p->p_offset)
			 / (lp->d_secsize / DEV_BSIZE) / lp->d_secpercyl;
 
Does this mean that archs that do have a MD bounds_check_with_label never 
have MBR support/access to f.e. msdosfs formatted USB keys? Is this 
relevant or will msdosfs work fine without an MBR?

What is the correct cylinder calculation? d_secpercyl is defined as the 
number of sectors per cylinder in buf.h so i would go for the first since 
in the second d_secpercyl is interpreted as B_DEVSIZE units, not sectors.

Attached is a patch to fix B_DEVSIZE assumptions in bounds_check_with_label 
code. Apart from the proposed change for arch/atari that seemed to have 
split its support from other sector sizes depending on whether it is a 
access to a raw device or not its quite mechanical. Some architectures f.e. 
checked in units of lp->d_secpercyl but truncated in case of overflow with 
DEB_BSHIFT:

--- sys/arch/amd64/amd64/disksubr.c     8 Oct 2003 04:25:44 -0000       1.8
+++ sys/arch/amd64/amd64/disksubr.c     7 Sep 2005 15:48:08 -0000
@@ -478,9 +478,7 @@
        struct disklabel *lp = dk->dk_label;
        struct partition *p = lp->d_partitions + DISKPART(bp->b_dev);
        int labelsector = lp->d_partitions[2].p_offset + LABELSECTOR;
-       int sz;
-
-       sz = howmany(bp->b_bcount, lp->d_secsize);
+       int sz = howmany(bp->b_bcount, lp->d_secsize);
 
        if (bp->b_blkno + sz > p->p_size) {
                sz = p->p_size - bp->b_blkno;
@@ -495,7 +493,7 @@
                        goto bad;
                }
                /* Otherwise, truncate request. */
-               bp->b_bcount = sz << DEV_BSHIFT;
+               bp->b_bcount = sz * lp->d_secsize;
        }

etc.

The patch can be found at the following URL since attachment bounced:

http://www.13thmonkey.org/bounds_check_with_label-diff

Comments are much appreciated,
Reinoud

--/04w6evG8XlLl3ft
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (NetBSD)

iQEVAwUBQx8gB4KcNwBDyKpoAQI+cQgAtJWGXj2KrbO1zBs9iX2BzyO/ZTkn7yiF
XSUYFrIrSktUu5EGpkG7BLcoB6AGAhKTw6rjTpqkbqTyqFxrZHvj3g7aNbancGFV
SaDQhXrRpF1R2Js3G29jOlu4yXfBzSrgujsXoxLv4zG56Kj5XMSEHtOTUA08e1cZ
4HaDjI5xRvyMYPtmKxcUEL0lrljL+9koQsaat4Qb1nB0exfsL+Jpv+2UQziV36eW
oZOCIWyovNdwwi1Dm+MB5oV8hDVrMBjJ2rBzgI/8qirXRDRZtEpiTb35Z06eEjUx
FtWq3rbDCz0tFGGn1FlWT0NQ5X/ImLUguKK8g0B73kq5E6rpnuJu9g==
=Qjd0
-----END PGP SIGNATURE-----

--/04w6evG8XlLl3ft--