linux-kernel - Re: [PATCH] usb-storage: Add support for unusual device by Sierra Wireless

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 14 May 2007 15:19:43 -0700
From:	Pete Zaitcev <zaitcev@...hat.com>
To:	Kevin Lloyd <klloyd@...rrawireless.com>
Cc:	gregkh@...e.de, mdharm-usb@...-eyed-alien.net,
	usb-storage@...ts.one-eyed-alien.net, linux-kernel@...r.kernel.org,
	linux-usb-users@...ts.sourceforge.net
Subject: Re: [PATCH] usb-storage: Add support for unusual device by Sierra
 Wireless

On Mon, 14 May 2007 14:44:49 -0700, Kevin Lloyd <klloyd@...rrawireless.com> wrote:

> +		schedule_timeout_uninterruptible(delay_t*HZ);
> +		retval = sierra_ms_change_mode(us, SWIMS_SET_MODE_Modem);
> +		if (retval == -ETIMEDOUT || retval == -ETIME) {
> +			US_DEBUGP("sierra_ms: Command timed out.\n");
> +			retries--;

This is strange. The two values should not typically be mixed this way.

ETIMEOUT happens when we (host) sent the setup packet, device took it,
and then posted NAKs for all data-in that we did, so eventually (after 5s),
we unlinked the URB. This means that the device works, just doesn't
want to perform the command in time. The bus itself functions fine.

ETIME means that something has violated the bus protocol. It's like
-EBUS for MMIO on PCI. Usually it means that the firmware in the device
is completely dead. You can call this a timeout in some sense, but
in reality it's no different from EILSEQ. We just report a different
code for finer shades of debugging. It happens in less than a millisecond.

I think you just need to test for ETIMEDOUT for that message.

> +	int result;
> +	unsigned char *blankArray;
> +	blankArray = kmalloc(0, GFP_KERNEL);

> +	result = usb_stor_control_msg(us, us->send_ctrl_pipe,
> +				SWIMS_USB_REQUEST_SetMode,
> +				SWIMS_USB_REQUEST_TYPE_SetMode,
> +				eSocMode,
> +				SWIMS_USB_INDEX_SetMode,
> +				blankArray,
> +				0,
> +				5*HZ);

The blankArray is completely unnecessary here. If something hits
a NULL despite the zero transfer size, it has to be fixed.

-- Pete
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/