Better buf validation in bounds_check_with_label()

[Matthew Dempsky]

A bunch of disk drivers currently check whether a buf request is
sector-aligned (e.g., vnd(4)) or that a request is for a multiple of
secsize (e.g., sd(4)), but none of them check both.

Instead of doing this in each disk driver, the logical place to
validate this is in bounds_check_with_label().

While here, I rewrote bounds_check_with_label() to be a bit easier to
read and be safer with integer arithmetic to avoid overflows (though I
don't think in practice this is actually a risk).

Once this goes in, I'll start updating the xxstrategy() methods to
remove these redundant checks.

ok?


Index: subr_disk.c
===================================================================
RCS file: /home/mdempsky/anoncvs/cvs/src/sys/kern/subr_disk.c,v
retrieving revision 1.127
diff -u -p -r1.127 subr_disk.c
--- subr_disk.c 30 Jun 2011 16:28:05 -0000      1.127
+++ subr_disk.c 4 Jul 2011 23:59:39 -0000
@@ -687,25 +687,30 @@ int
 bounds_check_with_label(struct buf *bp, struct disklabel *lp)
 {
        struct partition *p = &lp->d_partitions[DISKPART(bp->b_dev)];
-       daddr64_t sz = howmany(bp->b_bcount, DEV_BSIZE);
+       daddr64_t partblocks, sz;
 
-       /* Avoid division by zero, negative offsets and negative sizes. */
-       if (lp->d_secpercyl == 0 || bp->b_blkno < 0 || sz < 0)
+       /* Avoid division by zero, negative offsets, and negative sizes. */
+       if (lp->d_secpercyl == 0 || bp->b_blkno < 0 || bp->b_bcount < 0)
                goto bad;
 
-       /* beyond partition? */
-       if (bp->b_blkno + sz > DL_SECTOBLK(lp, DL_GETPSIZE(p))) {
-               sz = DL_SECTOBLK(lp, DL_GETPSIZE(p)) - bp->b_blkno;
-               if (sz == 0) {
-                       /* If exactly at end of disk, return EOF. */
-                       bp->b_resid = bp->b_bcount;
-                       return (-1);
-               }
-               if (sz < 0)
-                       /* If past end of disk, return EINVAL. */
-                       goto bad;
+       /* Ensure transfer is a whole number of aligned sectors. */
+       if ((bp->b_blkno % DL_BLKSPERSEC(lp)) != 0 ||
+           (bp->b_bcount % lp->d_secsize) != 0)
+               goto bad;
+
+       /* Ensure transfer starts within partition boundary. */
+       partblocks = DL_SECTOBLK(lp, DL_GETPSIZE(p));
+       if (bp->b_blkno > partblocks)
+               goto bad;
+
+       /* If exactly at end of partition or 0-byte request, return EOF. */
+       if (bp->b_blkno == partblocks || bp->b_bcount == 0)
+               goto done;
 
-               /* Otherwise, truncate request. */
+       /* Truncate request if it exceeds past the end of the partition. */
+       sz = bp->b_bcount >> DEV_BSHIFT;
+       if (sz > partblocks - bp->b_blkno) {
+               sz = partblocks - bp->b_blkno;
                bp->b_bcount = sz << DEV_BSHIFT;
        }
 
@@ -714,9 +719,11 @@ bounds_check_with_label(struct buf *bp, 
            DL_SECTOBLK(lp, lp->d_secpercyl);
        return (1);
 
-bad:
+ bad:
        bp->b_error = EINVAL;
        bp->b_flags |= B_ERROR;
+ done:
+       bp->b_resid = bp->b_bcount;
        return (-1);
 }