usb/110989: [patch] Handling of quirk IGNORE_RESIDUE is umass.c is broken

Hans Petter Selasky hselasky at c2i.net
Thu Mar 29 06:51:04 UTC 2007


On Thursday 29 March 2007 02:17, Michael Gmelin wrote:
> >Number:         110989
> >Category:       usb
> >Synopsis:       [patch] Handling of quirk IGNORE_RESIDUE is umass.c is
> > broken Confidential:   no
> >Severity:       serious
> >Priority:       medium
> >Responsible:    freebsd-usb
> >State:          open
> >Quarter:
> >Keywords:
> >Date-Required:
> >Class:          sw-bug
> >Submitter-Id:   current-users
> >Arrival-Date:   Thu Mar 29 00:30:09 GMT 2007
> >Closed-Date:
> >Last-Modified:
> >Originator:     Michael Gmelin
> >Release:        FreeBSD 6.2-RELEASE-p3 i386
> >Organization:
>
> /bin/done digital solutions GmbH
>
> >Environment:
>
> FreeBSD bombat.bindone.de 6.2-RELEASE-p3 FreeBSD 6.2-RELEASE-p3 #21: Wed
> Mar 28 04:08:44 CEST 2007    
> root at bombat.bindone.de:/usr/src/sys/i386/compile/bombat  i386
>
> >Description:
>
> I had to add a new device to usbdevs/umass.c which requires the
> IGNORE_RESIDUE quirk to be set (extra PR will follow). It didn't work,
> because IGNORE_RESIDUE isn't handled properly in umass.c (it isn't really
> handled at all, since Residue is set in lines are 1668-1672 in umass.c in
> the following was:
>
> int Residue;
> Residue = UGETDW(sc->csw.dCSWDataResidue);
> if (Residue == 0 &&
> 	sc->transfer_datalen - sc->transfer_actlen != 0)
> 		Residue = sc->transfer_datalen - sc->transfer_actlen;
>
> The patch below fixes this issue (tested and proven to work).
>
> >How-To-Repeat:
>
> Use a really broken USB device which returns "random" values for
> sc->csw.dCSWDataResidue (like devices that use the SuperTop IDEDEVICE USB
> controller, e.g. the ICY BOX IB-220U-Wh). Every attempt to use the device
> will lead to error messages, like:
>
> dd if=/dev/zero of=/dev/da0 count=10
> da0: end of device
>
> or
>
> disklabel da0
> read: Unknown error
> etc.
>
> >Fix:
>
> Apply the attached patch, which forces  residue to be calculated if
> IGNORE_RESIDUE is set.
>
>
>
> Patch attached with submission follows:
>
> --- umass.c.orig	Thu Mar 29 02:07:04 2007
> +++ umass.c	Thu Mar 29 02:08:06 2007
> @@ -1666,7 +1666,10 @@
>  		}
>
>  		int Residue;
> -		Residue = UGETDW(sc->csw.dCSWDataResidue);
> +		if (sc->quirks & IGNORE_RESIDUE)
> +			Residue = 0;
> +		else
> +			Residue = UGETDW(sc->csw.dCSWDataResidue);
>  		if (Residue == 0 &&
>  		    sc->transfer_datalen - sc->transfer_actlen != 0)
>  			Residue = sc->transfer_datalen - sc->transfer_actlen;
>
> >Release-Note:
> >Audit-Trail:
> >Unformatted:
>

I think that we should auto-detect bad residue. If the residue is some fixed 
constant way off the actual length, then maybe something like this is better:

Residue = UGETDW(sc->csw.dCSWDataResidue);

if (Residue > sc->transfer_datalen) {
    Residue = sc->transfer_datalen - sc->transfer_actlen;
}

--HPS


More information about the freebsd-usb mailing list