netdev - Re: Active URB submitted twice in pegasus driver

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]

Message-ID: <20130326185218.GD10317@xanatos>
Date:	Tue, 26 Mar 2013 11:52:18 -0700
From:	Sarah Sharp <sarah.a.sharp@...ux.intel.com>
To:	Petko Manolov <petkan@...leusys.com>
Cc:	linux-usb@...r.kernel.org, Greg KH <gregkh@...uxfoundation.org>,
	netdev@...r.kernel.org,
	Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: Active URB submitted twice in pegasus driver

On Tue, Mar 26, 2013 at 07:28:53PM +0200, Petko Manolov wrote:
> On Tue, 26 Mar 2013, Sarah Sharp wrote:
> 
> >On Tue, Mar 26, 2013 at 05:22:07PM +0200, Petko Manolov wrote:
> >>On Mon, 25 Mar 2013, Sarah Sharp wrote:
> >After taking a brief glance at the pegasus code,
> >pegasus_set_multicast looks broken.  It sets the control URB
> >status to zero and calls ctrl_callback(), which does some stuff if
> >the URB status is zero.  It doesn't look like
> >pegasus_set_multicast() is an URB callback function, so why in the
> >world is it touching a control URB that could possibly be in
> >flight else where?
> 
> Beats me, i've got nothing to do with this driver. :-P
> 
> Anyway, playing with the URB status looks rather stupid and is
> definitely a bug.  I'll make a patch that fixes the problem.
> 
> >It looks like the control URB is used to do things like get or set
> >registers, so what's stopping the upper layers from calling get
> >registers (which will submit the control URB and schedule a wait
> >queue), and then pegasus_set_multicast(), which will go overwrite
> >the active URB status?  USB device drivers should not be writing
> >or reading fields in a submitted URB until the completion
> >handler's callback function is called.
> 
> Err, see my previous comment.  It can't be me.  Maybe i've been too
> drunk. Or stupid.  Or both... :-)

Nah, everybody makes silly coding mistakes.  I was just trying to
clarify, since I don't know the driver.  Send me a patch and I'll see if
it fixes the warning.

But considering the multicast code is pretty old, I bet I'm running into
a different bug...

Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html