Re: [linux-usb-devel] Re: Vicam/3com homeconnect usb camera driver

From: Oliver Neukum (oliver@neukum.name)
Date: Tue Oct 08 2002 - 11:30:04 EST


On Tuesday 08 October 2002 07:40, John Tyner wrote:
> This is a resend of the patch I sent earlier. I worked on the one earlier
> while away from home and was unable to test it. This one has been fixed,
> tested, and should be correct. Sorry for the confusion.
>
> Patch is against 2.5.41.
>
> Thanks,
> John

Hi,

+ if ( waitqueue_active( &vicam_v4l_priv->no_users ) ) {
+ wake_up( &vicam_v4l_priv->no_users );

never ever do this. Always do the wake_up unconditionally.

ioctl() - VIDIOSYNC: this may hang if you unplug at the wrong moment,
as there's nobody to up the semaphore in that case.
You should do the up in the disconnect handler and check for disconnection
in the ioctl so you can return -ENODEV in that case.

in disconnect:

+ /* make sure no one will submit another urb */
+ clear_bit( 0, &vicam_v4l_priv->vicam_present );

But it does not guard against control messages in flight.
You need a semaphore to do that.

+ usb_unlink_urb( vicam_v4l_priv->urb );
+ down( &vicam_v4l_priv->busy_mutex );

This can hang for two reasons,
- you might not be the only user of that semaphore
- usb_unlink_urb may fail due to there being no queued urb,
  eg. if the completion handler is running - you need to check the return
  value

+ if ( vdev->users ) {
+ sleep_on( &vicam_v4l_priv->no_users );
+ }

_Very_ bad idea.
First, never use sleep_on. Use the appropriate macro.
Second, you have blocked khubd without an upper time limit.
That you _must_ _not_ in any case do.

+ kfree( vicam_v4l_priv );
+ kfree( vicam_usb_priv );

Potentionally deadly if the device is open, you need to defer it to release
in that case

Sorry for all that trouble.

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



This archive was generated by hypermail 2b29 : Tue Oct 15 2002 - 22:00:28 EST