>Number:         16110
>Category:       kern
>Synopsis:       scsipi_complete() calls (*psw_done)() b/4 setting buffer fields
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Mar 28 13:47:00 PST 2002
>Closed-Date:
>Last-Modified:
>Originator:     Chris Jepeway
>Release:        NetBSD 1.5Y, locally modified
>Organization:
	Blasted Heath Consulting, LCC
>Environment:
System: NetBSD pos 1.5Y NetBSD 1.5Y (POS) #9: Mon Mar 11 14:01:26 EST 2002 jepeway@the-morrigan:/src/sys/arch/i386/compile/POS i386
Architecture: i386
Machine: i386
>Description:
	scsipi_complete() calls a periph switch done completion routine
	before setting fields in the xs's struct buf.  If that completion
	routine used any of the fields that don't get set, it'd be...hosed.

	At least one such routine *is* hosed: sddone() uses b_resid like this

	  disk_unbusy(&sd->sc_dk, xs->bp->b_bcount - xs->bp->b_resid);

	to count the # of bytes xferred to/from the disk.  Since that'll
	use b_resid b/4 scsipi_complete() has set it, sddone() won't tally
	partial xfers correctly.
	
	Other fns called through (*psw_done)() might have similar problems.
>How-To-Repeat:
	Really, by inspection.

	Or, you could change a disk driver so it sets b_resid to b_bcount,
	expecting the SCSI code below the driver to set b_resid to 0 for
	completed xfers.  You'd then scratch your head wondering why
	"iostat -x" didn't show all the xfers you knew were taking place.
	But, hey, who would do something like that? :)
>Fix:
	This patch illustrates/fixes the problem.  It's from a local
	repository forked from NetBSD-current at revision 1.61 of
	scsipi_base.c, so ignore its versions and line numbers:

Index: scsipi_base.c
===================================================================
RCS file: /cvsroot/src/sys/dev/scsipi/scsipi_base.c,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- scsipi_base.c	2002/02/27 23:22:54	1.3
+++ scsipi_base.c	2002/03/07 17:37:59	1.4
@@ -1532,9 +1532,10 @@
 	if (xs->error != XS_NOERROR)
 		scsipi_periph_thaw(periph, 1);
 
-
-	if (periph->periph_switch->psw_done)
-		periph->periph_switch->psw_done(xs);
+	/*
+	 * Set buffer fields in case the periph
+	 * switch done func uses them
+	 */
 	if ((bp = xs->bp) != NULL) {
 		if (error) {
 			bp->b_error = error;
@@ -1543,9 +1544,13 @@
 		} else {
 			bp->b_error = 0;
 			bp->b_resid = xs->resid;
-																		}
-		biodone(bp);
+		}
 	}
+
+	if (periph->periph_switch->psw_done)
+		periph->periph_switch->psw_done(xs);
+	if (bp)
+		biodone(bp);
 
 	if (xs->xs_control & XS_CTL_ASYNC)
 		scsipi_put_xs(xs);
>Release-Note:
>Audit-Trail:
>Unformatted: