Hi,

the following points:

1. Checking for an active waitqueue before waking is either
    unnecessary or a race condition
2. usb_set_interface() in the release code path may fail, but
   you must not abort in that case or there is a memleak
3. don't use interruptible_sleep_on() or friends
4. you must not pass a buffer on the stack to usb_bulk_msg()
   and friends

        Regards
                Oliver

You can import this changeset into BK by piping this whole message to:
'| bk receive [path to repository]' or apply the patch as usual.

===================================================================


[EMAIL PROTECTED], 2003-08-09 14:19:14+02:00, [EMAIL PROTECTED]
  - no DMA on stack
  - cleanup of waiting
  - fix memleak in race with physical disconnect


 cpia_usb.c |   28 +++++++++++++++++++---------
 1 files changed, 19 insertions(+), 9 deletions(-)


diff -Nru a/drivers/media/video/cpia_usb.c b/drivers/media/video/cpia_usb.c
--- a/drivers/media/video/cpia_usb.c    Sat Aug  9 14:19:51 2003
+++ b/drivers/media/video/cpia_usb.c    Sat Aug  9 14:19:51 2003
@@ -161,8 +161,7 @@
                                ucpia->workbuff->status = FRAME_EMPTY;
                                ucpia->workbuff->length = 0;
                  
-                               if (waitqueue_active(&ucpia->wq_stream))
-                                       wake_up_interruptible(&ucpia->wq_stream);
+                               wake_up_interruptible(&ucpia->wq_stream);
                        }
                }
        }
@@ -348,7 +347,10 @@
        databytes = (((int)command[7])<<8) | command[6];
 
        if (command[0] == DATA_IN) {
-               u8 buffer[8];
+               u8 *buffer;
+               buffer = kmalloc(8, GFP_KERNEL);
+               if (!buffer)
+                       return -ENOMEM;
 
                if (!data) {
                        DBG("Internal driver error: data is NULL\n");
@@ -356,10 +358,13 @@
                }
 
                err = ReadPacket(udev, command, buffer, 8);
-               if (err < 0)
+               if (err < 0) {
+                       kfree(buffer);
                        return err;
+               }
 
                memcpy(data, buffer, databytes);
+               kfree(buffer);
        } else if(command[0] == DATA_OUT)
                WritePacket(udev, command, data, databytes);
        else {
@@ -390,15 +395,22 @@
 {
        struct usb_cpia *ucpia = (struct usb_cpia *) privdata;
        struct framebuf *mybuff;
+       DEFINE_WAIT(wait);
 
        if (!ucpia || !ucpia->present)
                return -1;
-  
+               
+       prepare_to_wait(&ucpia->wq_stream, &wait, TASK_INTERRUPTIBLE);  
        if (ucpia->curbuff->status != FRAME_READY)
-               interruptible_sleep_on(&ucpia->wq_stream);
+               schedule();
        else
                DBG("Frame already waiting!\n");
+       finish_wait(&ucpia->wq_stream, &wait);
+       if (signal_pending(current))
+               return -EINTR;
+       
 
+       
        mybuff = ucpia->curbuff;
 
        if (!mybuff)
@@ -434,7 +446,6 @@
                ret = usb_set_interface(ucpia->dev, ucpia->iface, 0);
                if (ret < 0) {
                        printk(KERN_ERR "usb_set_interface error (ret = %d)\n", ret);
-                       return;
                }
        }
 
@@ -616,8 +627,7 @@
 
        ucpia->curbuff->status = FRAME_ERROR;
 
-       if (waitqueue_active(&ucpia->wq_stream))
-               wake_up_interruptible(&ucpia->wq_stream);
+       wake_up_interruptible(&ucpia->wq_stream);
 
        udev = interface_to_usbdev(intf);
        usb_driver_release_interface(&cpia_driver,

===================================================================


This BitKeeper patch contains the following changesets:
1.2214
## Wrapped with gzip_uu ##


begin 664 bkpatch28640
M'XL(`.?F-#\``ZU5;8_:1A#^S/Z*J2*ET/"RZS=LKD1W"21%]Q)$./[EMAIL PROTECTED]
M6/B%KM>A*.2_=]:^7IK>Z7IM8R%YF)UG9^;99]8OX+9$-6H5:?()%7L!OQ2E
M'K7(SJH8\WZ.U:[*^H7:T-JB*&AML"TR'#2`P5([EMAIL PROTECTED]:`JV4
MHY;HV_<>[EMAIL PROTECTED],[EMAIL PROTECTED]';+4[CRM,^SM5R*W)=KI?
M/EF<"R$LE]NN)]R3Y=NV<Q(>7\5DQM8P6*^\@&T4;LX;>%1DW\)M[G/7LJPA
M'YX<[EMAIL PROTECTED]@]X`,(9B6`DG%?<&G$.37OGC_``KP3T.'L#_[_F
MMRR"'N0%3*XOH,BAU#+:U;XH19E7>RC6<)")3O)-[5XGOT.&&2WN(,E!R0CA
[EMAIL PROTECTED]',HED"G%21D6>8Z39)3B^)5PV_THUZ_W+AS$N.7O]2*^Q,@[EMAIL PROTECTED]
M1`X^)[EMAIL PROTECTED])"$T(^^$N#PP/).CNWXWBE8^=**43JNC./[EMAIL PROTECTED],-
MA"4"(4ZV[?E!+:NG<8]K[7OT\G<!/J]^1PP=S^4GR[9<OU:ES1]H4CQ'DP'T
[EMAIL PROTECTED]/<84@"C+88[4I(=([EMAIL PROTECTED]/<AC"56)Y((J)ZW%),\BEVEZK(%D%)'4
M"*MJO48%JR/LLMK9A15&[EMAIL PROTECTED]:5:7.?]3D,L+76_R+^)-<HU+57B>K%,,R1=R'
M%&[EMAIL PROTECTED]&]L)D$F::[EMAIL PROTECTED]&I:HPQJV-B-!HQ&E!27:2YJ-&D=YDDU>*(QI+AJU
M?(">.M0_TOG\'X3S'R9G(CP'[EMAIL PROTECTED]"M>BY(S7\IL'VR\JDZ;T^_!:66J',
M.F=L8KN"0#/S<@A;[EMAIL PROTECTED];NC=OPGLVV_"^_?S</+Z>)F>M4Q(<D:VC\T
M<1V36Z&N5`Z]Z<V'[EMAIL PROTECTED]/-<'/P#OPV2!V:[K:VW=;G%&DQ^LV
MOAC3JLT'(4'MGTS?S6ZFX:\7LV7;W%YU/X%7IZ.72<=:>X5[J3#416AB'K+0
MA9=FH0O+BX^7X>QF.5TL;N?+V9NK:><,P&SI-UOZ=3$E236NB$]3B4/C0KRM
MDSPIMT\G,'29[DO2ATS#/9*B\TT[JI3"7'<,>_?D414+"C?[F^-IL8EC#\F8
B>,+0./.LFJ/GG_/]][&9LRH;QR*F2X8/V1]=0J#LF@<`````
`
end



-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to