USB speedtouch: don't open a connection if no firmware
PATCH 1/1 USB speedtouch: don't open a connection if no firmware
People using Alcatel's closed source firmware loader "speedmgmt" may need to sleep longer before launching pppd.
drivers/usb/misc/speedtch.c +9 -7
@@ -816,9 +816,6 @@return -ENODEV ; } -if (!instance->firmware_loaded) -return -EAGAIN ; -if (vcc->qos.aal !=ATM_AAL5 ) {dbg ("unsupported ATM type %d!" , vcc->qos.aal);return -EINVAL ;
@@ -933,6 +930,11 @@if (vcc->qos.aal !=ATM_AAL5 )return -EINVAL ; +if (!instance->firmware_loaded) { +dbg ("firmware not loaded!" ); +return -EAGAIN ; + } +down (&instance->serialize);/* vs self, udsl_atm_close */ if (udsl_find_vcc (instance, vpi, vci)) {
@@ -967,13 +969,12 @@dbg ("Allocated new SARLib vcc 0x%p with vpi %d vci %d" , new, vpi, vci); -MOD_INC_USE_COUNT ; - -if (instance->firmware_loaded) -udsl_fire_receivers (instance); +udsl_fire_receivers (instance);dbg ("udsl_atm_open successful" ); +MOD_INC_USE_COUNT ; +return 0 ; }
@@ -1041,6 +1042,7 @@int ret;if ((ret =usb_set_interface (instance->usb_dev,1 ,1 )) <0 ) { +dbg ("usb_set_interface returned %d!" , ret);up (&instance->serialize);return ret; }
Quoted 7 lines · Duncan Sands
drivers/usb/misc/speedtch.c +0
-3@@ -967,13 +969,12 @@dbg ("Allocated new SARLib vcc 0x%p with vpi %d vci %d" , new, vpi, vci); -MOD_INC_USE_COUNT ; - -if (instance->firmware_loaded)
drivers/usb/misc/speedtch.c line 980
Quoted 10 lines
drivers/usb/misc/speedtch.c +3
-1-udsl_fire_receivers (instance); +udsl_fire_receivers (instance);dbg ("udsl_atm_open successful" ); +MOD_INC_USE_COUNT ; +return 0 ; }
Any way you can convert this driver to not use MOD_INC_USE_COUNT, as it's racy and not really supported anymore? But if you _really_ have to use it, you need to call it at the first possible chance to make any race window smaller.
thanks,
greg k-h
Quoted 4 lines
Quoted 4 lines
drivers/usb/misc/speedtch.c +1
-0+udsl_fire_receivers (instance);dbg ("udsl_atm_open successful" );
Quoted 9 lines
drivers/usb/misc/speedtch.c line 979
Quoted 4 lines
drivers/usb/misc/speedtch.c +2
-0+MOD_INC_USE_COUNT ; +return 0 ; }Any way you can convert this driver to not use
MOD_INC_USE_COUNT, as it's racy and not really supported anymore? But if you _really_ have to use it, you need to call it at the first possible chance to make any race window smaller.
Hi Greg, I'm waiting on the fixes to the ATM layer (coming soon to a kernel near you). As for the position of MOD_INC_USE_COUNT, did you ever hear of anyone getting bitten by a race like this? If it makes you feel better, I will move it up, probably just before I take the semaphore (since that is the first place we can sleep). I will do it tomorrow, OK?
All the best,
Duncan.
Quoted 14 lines · Duncan Sands
Quoted 4 lines
Quoted 4 lines
drivers/usb/misc/speedtch.c +1
-0+udsl_fire_receivers (instance);dbg ("udsl_atm_open successful" );Quoted 9 lines
drivers/usb/misc/speedtch.c line 979
Quoted 4 lines
drivers/usb/misc/speedtch.c +2
-0+MOD_INC_USE_COUNT ; +return 0 ; }Any way you can convert this driver to not use
MOD_INC_USE_COUNT, as it's racy and not really supported anymore? But if you _really_ have to use it, you need to call it at the first possible chance to make any race window smaller.
Quoted 2 lines
Hi Greg, I'm waiting on the fixes to the ATM layer (coming soon to a kernel near you).
Ah, ok, that makes sense.
Quoted 4 lines
As for the position of
MOD_INC_USE_COUNT, did you ever hear
of anyone getting bitten by a race like this? If it makes you feel better, I will move it up, probably just before I take the semaphore (since that is the first place we can sleep). I will do it tomorrow, OK?
Yes, it needs to be before any function that can sleep. I'll hold off applying this patch then.
thanks,
greg k-h
Quoted 5 lines
Quoted 2 lines
Hi Greg, I'm waiting on the fixes to the ATM layer (coming soon to a kernel near you).
Ah, ok, that makes sense.
Quoted 7 lines
Quoted 4 lines
As for the position of
MOD_INC_USE_COUNT, did you ever hear of anyone getting bitten by a race like this? If it makes you feel better, I will move it up, probably just before I take the semaphore (since that is the first place we can sleep). I will do it tomorrow, OK?Yes, it needs to be before any function that can sleep. I'll hold off applying this patch then.
How about this one instead. MOD_INC_USE_COUNT is placed before I call any functions that can sleep. Let's just hope that the call to me doesn't come after some sleeping in the higher layers...
Duncan.
drivers/usb/misc/speedtch.c +11 -4
@@ -933,15 +933,24 @@if (vcc->qos.aal !=ATM_AAL5 )return -EINVAL ; +if (!instance->firmware_loaded) { +dbg ("firmware not loaded!" ); +return -EAGAIN ; + } + +MOD_INC_USE_COUNT ; +down (&instance->serialize);/* vs self, udsl_atm_close */ if (udsl_find_vcc (instance, vpi, vci)) {up (&instance->serialize); +MOD_DEC_USE_COUNT ;return -EADDRINUSE ; }if (!(new =kmalloc (sizeof (struct udsl_vcc_data ),GFP_KERNEL ))) {up (&instance->serialize); +MOD_DEC_USE_COUNT ;return -ENOMEM ; }
@@ -967,10 +976,7 @@dbg ("Allocated new SARLib vcc 0x%p with vpi %d vci %d" , new, vpi, vci); -MOD_INC_USE_COUNT ; - -if (instance->firmware_loaded) -udsl_fire_receivers (instance); +udsl_fire_receivers (instance);dbg ("udsl_atm_open successful" );
@@ -1041,6 +1047,7 @@int ret;if ((ret =usb_set_interface (instance->usb_dev,1 ,1 )) <0 ) { +dbg ("usb_set_interface returned %d!" , ret);up (&instance->serialize);return ret; }
Quoted 10 lines · Duncan Sands
Quoted 10 lines
Quoted 2 lines
Hi Greg, I'm waiting on the fixes to the ATM layer (coming soon to a kernel near you).
Ah, ok, that makes sense.
Quoted 4 lines
As for the position of
MOD_INC_USE_COUNT, did you ever hear of anyone getting bitten by a race like this? If it makes you feel better, I will move it up, probably just before I take the semaphore (since that is the first place we can sleep). I will do it tomorrow, OK?
Quoted 6 lines
Quoted 2 lines
Yes, it needs to be before any function that can sleep. I'll hold off applying this patch then.
How about this one instead.
MOD_INC_USE_COUNTis placed before I call any functions that can sleep. Let's just hope that the call to me doesn't come after some sleeping in the higher layers...
Much better, thanks. Applied.
greg k-h