ChangeSet 1.1254.1.7, 2003/05/29 15:25:37-07:00, hch@lst.de
[PATCH] fix scsi_register_host abuse in usb scanner drivers
They should be using scsi_add_host directly. I had to rewrite
half of the drivers, though to fix horrible braindamage like
leaving dangling scsi structures around after ->disconnect.
Gettig rid of the remaining scsi_register_host callers is required
for the scsi stack to move forward so please try to forward this
to Linus in a timely mannor, thanks!
drivers/usb/image/hpusbscsi.c | 223 ++++++++++----------------------
drivers/usb/image/hpusbscsi.h | 22 ---
drivers/usb/image/microtek.c | 291 ++++++------------------------------------
drivers/usb/image/microtek.h | 3
4 files changed, 118 insertions(+), 421 deletions(-)
diff -Nru a/drivers/usb/image/hpusbscsi.c b/drivers/usb/image/hpusbscsi.c
--- a/drivers/usb/image/hpusbscsi.c Fri May 30 11:35:21 2003
+++ b/drivers/usb/image/hpusbscsi.c Fri May 30 11:35:21 2003
@@ -22,65 +22,56 @@
#define TRACE_STATE printk(KERN_DEBUG"hpusbscsi->state = %s at line %d\n", states[hpusbscsi->state], __LINE__)
-/* global variables */
-
-struct list_head hpusbscsi_devices;
-//LIST_HEAD(hpusbscsi_devices);
-
-/* USB related parts */
+static Scsi_Host_Template hpusbscsi_scsi_host_template = {
+ .module = THIS_MODULE,
+ .name = "hpusbscsi",
+ .proc_name = "hpusbscsi",
+ .queuecommand = hpusbscsi_scsi_queuecommand,
+ .eh_abort_handler = hpusbscsi_scsi_abort,
+ .eh_host_reset_handler = hpusbscsi_scsi_host_reset,
+ .sg_tablesize = SG_ALL,
+ .can_queue = 1,
+ .this_id = -1,
+ .cmd_per_lun = 1,
+ .use_clustering = 1,
+ .emulated = 1,
+};
static int
-hpusbscsi_usb_probe (struct usb_interface *intf,
- const struct usb_device_id *id)
+hpusbscsi_usb_probe(struct usb_interface *intf,
+ const struct usb_device_id *id)
{
+ struct usb_device *dev = interface_to_usbdev(intf);
+ struct usb_host_interface *altsetting = intf->altsetting;
struct hpusbscsi *new;
- struct usb_device *dev = interface_to_usbdev (intf);
- struct usb_host_interface *altsetting =
- &(intf->altsetting[0]);
-
+ int error = -ENOMEM;
int i, result;
- /* basic check */
-
if (altsetting->desc.bNumEndpoints != 3) {
printk (KERN_ERR "Wrong number of endpoints\n");
return -ENODEV;
}
- /* descriptor allocation */
-
- new =
- (struct hpusbscsi *) kmalloc (sizeof (struct hpusbscsi),
- GFP_KERNEL);
- if (new == NULL)
+ new = kmalloc(sizeof(struct hpusbscsi), GFP_KERNEL);
+ if (!new)
return -ENOMEM;
- DEBUG ("Allocated memory\n");
- memset (new, 0, sizeof (struct hpusbscsi));
+ memset(new, 0, sizeof(struct hpusbscsi));
new->dataurb = usb_alloc_urb(0, GFP_KERNEL);
- if (!new->dataurb) {
- kfree (new);
- return -ENOMEM;
- }
+ if (!new->dataurb)
+ goto out_kfree;
new->controlurb = usb_alloc_urb(0, GFP_KERNEL);
- if (!new->controlurb) {
- usb_free_urb (new->dataurb);
- kfree (new);
- return -ENOMEM;
- }
- new->dev = dev;
- init_waitqueue_head (&new->pending);
- init_waitqueue_head (&new->deathrow);
- INIT_LIST_HEAD (&new->lh);
-
+ if (!new->controlurb)
+ goto out_free_dataurb;
+ new->dev = dev;
+ init_waitqueue_head(&new->pending);
+ init_waitqueue_head(&new->deathrow);
- /* finding endpoints */
-
+ error = -ENODEV;
for (i = 0; i < altsetting->desc.bNumEndpoints; i++) {
- if (
- (altsetting->endpoint[i].desc.
+ if ((altsetting->endpoint[i].desc.
bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
- USB_ENDPOINT_XFER_BULK) {
+ USB_ENDPOINT_XFER_BULK) {
if (altsetting->endpoint[i].desc.
bEndpointAddress & USB_DIR_IN) {
new->ep_in =
@@ -97,57 +88,71 @@
new->ep_int =
altsetting->endpoint[i].desc.
bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
- new->interrupt_interval= altsetting->endpoint[i].desc.bInterval;
+ new->interrupt_interval= altsetting->endpoint[i].desc.
+ bInterval;
}
}
/* USB initialisation magic for the simple case */
-
- result = usb_set_interface (dev, altsetting->desc.bInterfaceNumber, 0);
-
+ result = usb_set_interface(dev, altsetting->desc.bInterfaceNumber, 0);
switch (result) {
case 0: /* no error */
break;
default:
- printk (KERN_ERR "unknown error %d from usb_set_interface\n",
+ printk(KERN_ERR "unknown error %d from usb_set_interface\n",
result);
- goto err_out;
+ goto out_free_controlurb;
}
- /* making a template for the scsi layer to fake detection of a scsi device */
+ /* build and submit an interrupt URB for status byte handling */
+ usb_fill_int_urb(new->controlurb, new->dev,
+ usb_rcvintpipe(new->dev, new->ep_int),
+ &new->scsi_state_byte, 1,
+ control_interrupt_callback,new,
+ new->interrupt_interval);
+
+ if (usb_submit_urb(new->controlurb, GFP_KERNEL) < 0)
+ goto out_free_controlurb;
- memcpy (&(new->ctempl), &hpusbscsi_scsi_host_template,
- sizeof (hpusbscsi_scsi_host_template));
- (struct hpusbscsi *) new->ctempl.proc_dir = new;
- new->ctempl.module = THIS_MODULE;
+ /* In host->hostdata we store a pointer to desc */
+ new->host = scsi_register(&hpusbscsi_scsi_host_template, sizeof(new));
+ if (!new->host)
+ goto out_unlink_controlurb;
- if (scsi_register_host(&new->ctempl))
- goto err_out;
+ new->host->hostdata[0] = (unsigned long)new;
+ scsi_add_host(new->host, &intf->dev);
new->sense_command[0] = REQUEST_SENSE;
new->sense_command[4] = HPUSBSCSI_SENSE_LENGTH;
- /* adding to list for module unload */
- list_add (&hpusbscsi_devices, &new->lh);
-
usb_set_intfdata(intf, new);
return 0;
- err_out:
- usb_free_urb (new->controlurb);
- usb_free_urb (new->dataurb);
- kfree (new);
- return -ENODEV;
+ out_unlink_controlurb:
+ usb_unlink_urb(new->controlurb);
+ out_free_controlurb:
+ usb_free_urb(new->controlurb);
+ out_free_dataurb:
+ usb_free_urb(new->dataurb);
+ out_kfree:
+ kfree(new);
+ return error;
}
static void
-hpusbscsi_usb_disconnect (struct usb_interface *intf)
+hpusbscsi_usb_disconnect(struct usb_interface *intf)
{
struct hpusbscsi *desc = usb_get_intfdata(intf);
usb_set_intfdata(intf, NULL);
- if (desc)
- usb_unlink_urb(desc->controlurb);
+
+ scsi_remove_host(desc->host);
+ usb_unlink_urb(desc->controlurb);
+ scsi_unregister(desc->host);
+
+ usb_free_urb(desc->controlurb);
+ usb_free_urb(desc->dataurb);
+ kfree(desc);
}
static struct usb_device_id hpusbscsi_usb_ids[] = {
@@ -177,100 +182,20 @@
/* module initialisation */
-int __init
+static int __init
hpusbscsi_init (void)
{
- int result;
-
- INIT_LIST_HEAD (&hpusbscsi_devices);
- DEBUG ("Driver loaded\n");
-
- if ((result = usb_register (&hpusbscsi_usb_driver)) < 0) {
- printk (KERN_ERR "hpusbscsi: driver registration failed\n");
- return -1;
- } else {
- return 0;
- }
+ return usb_register(&hpusbscsi_usb_driver);
}
-void __exit
+static void __exit
hpusbscsi_exit (void)
{
- struct list_head *tmp;
- struct list_head *old;
- struct hpusbscsi * o;
-
- for (tmp = hpusbscsi_devices.next; tmp != &hpusbscsi_devices;/*nothing */) {
- old = tmp;
- tmp = tmp->next;
- o = (struct hpusbscsi *)old;
- usb_unlink_urb(o->controlurb);
- scsi_unregister_host(&o->ctempl);
- usb_free_urb(o->controlurb);
- usb_free_urb(o->dataurb);
- kfree(old);
- }
-
- usb_deregister (&hpusbscsi_usb_driver);
+ usb_deregister(&hpusbscsi_usb_driver);
}
module_init (hpusbscsi_init);
module_exit (hpusbscsi_exit);
-
-/* interface to the scsi layer */
-
-static int
-hpusbscsi_scsi_detect (struct SHT *sht)
-{
- /* Whole function stolen from usb-storage */
-
- struct hpusbscsi *desc = (struct hpusbscsi *) sht->proc_dir;
- /* What a hideous hack! */
-
- char local_name[48];
-
-
- /* set up the name of our subdirectory under /proc/scsi/ */
- sprintf (local_name, "hpusbscsi-%d", desc->number);
- sht->proc_name = kmalloc (strlen (local_name) + 1, GFP_KERNEL);
- /* FIXME: where is this freed ? */
-
- if (!sht->proc_name) {
- return 0;
- }
-
- strcpy (sht->proc_name, local_name);
-
- sht->proc_dir = NULL;
-
- /* build and submit an interrupt URB for status byte handling */
- usb_fill_int_urb(desc->controlurb,
- desc->dev,
- usb_rcvintpipe(desc->dev,desc->ep_int),
- &desc->scsi_state_byte,
- 1,
- control_interrupt_callback,
- desc,
- desc->interrupt_interval
- );
-
- if ( 0 > usb_submit_urb(desc->controlurb, GFP_KERNEL)) {
- kfree(sht->proc_name);
- return 0;
- }
-
- /* In host->hostdata we store a pointer to desc */
- desc->host = scsi_register (sht, sizeof (desc));
- if (desc->host == NULL) {
- kfree (sht->proc_name);
- usb_unlink_urb(desc->controlurb);
- return 0;
- }
- desc->host->hostdata[0] = (unsigned long) desc;
-
-
- return 1;
-}
static int hpusbscsi_scsi_queuecommand (Scsi_Cmnd *srb, scsi_callback callback)
{
diff -Nru a/drivers/usb/image/hpusbscsi.h b/drivers/usb/image/hpusbscsi.h
--- a/drivers/usb/image/hpusbscsi.h Fri May 30 11:35:21 2003
+++ b/drivers/usb/image/hpusbscsi.h Fri May 30 11:35:21 2003
@@ -13,7 +13,6 @@
struct hpusbscsi
{
- struct list_head lh;
struct usb_device *dev; /* NULL indicates unplugged device */
int ep_out;
int ep_in;
@@ -36,7 +35,6 @@
int state;
int current_data_pipe;
- Scsi_Host_Template ctempl;
u8 sense_command[SENSE_COMMAND_SIZE];
u8 scsi_state_byte;
};
@@ -52,7 +50,6 @@
#define DIRECTION_IS_IN(x) ((scsi_command_direction[x>>3] >> (x & 7)) & 1)
-static int hpusbscsi_scsi_detect (struct SHT * sht);
static void simple_command_callback(struct urb *u, struct pt_regs *regs);
static void scatter_gather_callback(struct urb *u, struct pt_regs *regs);
static void simple_payload_callback (struct urb *u, struct pt_regs *regs);
@@ -63,25 +60,6 @@
static int hpusbscsi_scsi_host_reset (Scsi_Cmnd *srb);
static int hpusbscsi_scsi_abort (Scsi_Cmnd *srb);
static void issue_request_sense (struct hpusbscsi *hpusbscsi);
-
-static Scsi_Host_Template hpusbscsi_scsi_host_template = {
- .name = "hpusbscsi",
- .detect = hpusbscsi_scsi_detect,
-// .release = hpusbscsi_scsi_release,
- .queuecommand = hpusbscsi_scsi_queuecommand,
-
- .eh_abort_handler = hpusbscsi_scsi_abort,
- .eh_host_reset_handler = hpusbscsi_scsi_host_reset,
-
- .sg_tablesize = SG_ALL,
- .can_queue = 1,
- .this_id = -1,
- .cmd_per_lun = 1,
- .present = 0,
- .unchecked_isa_dma = FALSE,
- .use_clustering = TRUE,
- .emulated = TRUE
-};
/* defines for internal driver state */
#define HP_STATE_FREE 0 /*ready for next request */
diff -Nru a/drivers/usb/image/microtek.c b/drivers/usb/image/microtek.c
--- a/drivers/usb/image/microtek.c Fri May 30 11:35:21 2003
+++ b/drivers/usb/image/microtek.c Fri May 30 11:35:21 2003
@@ -326,76 +326,6 @@
usb_unlink_urb( desc->urb );
}
-static struct mts_desc * mts_list; /* list of active scanners */
-struct semaphore mts_list_semaphore;
-
-/* Internal list operations */
-
-static
-void mts_remove_nolock( struct mts_desc* to_remove )
-{
- MTS_DEBUG( "removing 0x%x from list\n",
- (int)to_remove );
-
- lock_kernel();
- mts_urb_abort(to_remove);
-
- MTS_DEBUG_GOT_HERE();
-
- if ( to_remove != mts_list ) {
- MTS_DEBUG_GOT_HERE();
- if (to_remove->prev && to_remove->next)
- to_remove->prev->next = to_remove->next;
- } else {
- MTS_DEBUG_GOT_HERE();
- mts_list = to_remove->next;
- if (mts_list) {
- MTS_DEBUG_GOT_HERE();
- mts_list->prev = 0;
- }
- }
-
- if ( to_remove->next ) {
- MTS_DEBUG_GOT_HERE();
- to_remove->next->prev = to_remove->prev;
- }
-
- MTS_DEBUG_GOT_HERE();
- scsi_unregister_host(&to_remove->ctempl);
- unlock_kernel();
-
- usb_free_urb(to_remove->urb);
- kfree( to_remove );
-}
-
-static
-void mts_add_nolock( struct mts_desc* to_add )
-{
- MTS_DEBUG( "adding 0x%x to list\n", (int)to_add );
-
- to_add->prev = 0;
- to_add->next = mts_list;
- if ( mts_list ) {
- mts_list->prev = to_add;
- }
-
- mts_list = to_add;
-}
-
-
-
-
-/* SCSI driver interface */
-
-/* scsi related functions - dummies for now mostly */
-
-static int mts_scsi_release(struct Scsi_Host *psh)
-{
- MTS_DEBUG_GOT_HERE();
-
- return 0;
-}
-
static int mts_scsi_abort (Scsi_Cmnd *srb)
{
struct mts_desc* desc = (struct mts_desc*)(srb->device->host->hostdata[0]);
@@ -418,54 +348,6 @@
return 0; /* RANT why here 0 and not SUCCESS */
}
-/* the core of the scsi part */
-
-/* faking a detection - which can't fail :-) */
-
-static int mts_scsi_detect (struct SHT * sht)
-{
- /* Whole function stolen from usb-storage */
-
- struct mts_desc * desc = (struct mts_desc *)sht->proc_dir;
- /* What a hideous hack! */
-
- char local_name[48];
-
- MTS_DEBUG_GOT_HERE();
-
- /* set up the name of our subdirectory under /proc/scsi/ */
- sprintf(local_name, "microtek-%d", desc->host_number);
- sht->proc_name = kmalloc (strlen(local_name) + 1, GFP_KERNEL);
- /* FIXME: where is this freed ? */
-
- if (!sht->proc_name) {
- MTS_ERROR( "unable to allocate memory for proc interface!!\n" );
- return 0;
- }
-
- strcpy(sht->proc_name, local_name);
-
- sht->proc_dir = NULL;
-
- /* In host->hostdata we store a pointer to desc */
- desc->host = scsi_register(sht, sizeof(desc));
- if (desc->host == NULL) {
- MTS_ERROR("Cannot register due to low memory");
- kfree(sht->proc_name);
- return 0;
- }
- desc->host->hostdata[0] = (unsigned long)desc;
-/* FIXME: what if sizeof(void*) != sizeof(unsigned long)? */
-
- return 1;
-}
-
-
-
-/* Main entrypoint: SCSI commands are dispatched to here */
-
-
-
static
int mts_scsi_queuecommand (Scsi_Cmnd *srb, mts_scsi_cmnd_callback callback );
@@ -743,52 +625,22 @@
out:
return err;
}
-/*
- * this defines our 'host'
- */
-
-/* NOTE: This is taken from usb-storage, should be right. */
-
static Scsi_Host_Template mts_scsi_host_template = {
- .name = "microtekX6",
- .detect = mts_scsi_detect,
- .release = mts_scsi_release,
- .queuecommand = mts_scsi_queuecommand,
-
- .eh_abort_handler = mts_scsi_abort,
- .eh_host_reset_handler =mts_scsi_host_reset,
-
+ .module = THIS_MODULE,
+ .name = "microtekX6",
+ .proc_name = "microtekX6",
+ .queuecommand = mts_scsi_queuecommand,
+ .eh_abort_handler = mts_scsi_abort,
+ .eh_host_reset_handler = mts_scsi_host_reset,
.sg_tablesize = SG_ALL,
.can_queue = 1,
.this_id = -1,
.cmd_per_lun = 1,
- .present = 0,
- .unchecked_isa_dma = FALSE,
- .use_clustering = TRUE,
- .emulated = TRUE
+ .use_clustering = 1,
+ .emulated = 1,
};
-
-/* USB layer driver interface implementation */
-
-static void mts_usb_disconnect (struct usb_interface *intf)
-{
- struct mts_desc* to_remove = usb_get_intfdata(intf);
-
- MTS_DEBUG_GOT_HERE();
-
- usb_set_intfdata(intf, NULL);
- if (to_remove) {
- /* leave the list - lock it */
- down(&mts_list_semaphore);
-
- mts_remove_nolock(to_remove);
-
- up(&mts_list_semaphore);
- }
-}
-
struct vendor_product
{
char* name;
@@ -836,8 +688,8 @@
MODULE_DEVICE_TABLE (usb, mts_usb_ids);
-static int mts_usb_probe (struct usb_interface *intf,
- const struct usb_device_id *id)
+static int mts_usb_probe(struct usb_interface *intf,
+ const struct usb_device_id *id)
{
int i;
int result;
@@ -929,39 +781,23 @@
}
- /* allocating a new descriptor */
- new_desc = (struct mts_desc *)kmalloc(sizeof(struct mts_desc), GFP_KERNEL);
- if (new_desc == NULL)
- {
- MTS_ERROR("couldn't allocate scanner desc, bailing out!\n");
- return -ENOMEM;
- }
+ new_desc = kmalloc(sizeof(struct mts_desc), GFP_KERNEL);
+ if (!new_desc)
+ goto out;
- memset( new_desc, 0, sizeof(*new_desc) );
+ memset(new_desc, 0, sizeof(*new_desc));
new_desc->urb = usb_alloc_urb(0, GFP_KERNEL);
- if (!new_desc->urb) {
- kfree(new_desc);
- return -ENOMEM;
- }
-
- /* initialising that descriptor */
- new_desc->usb_dev = dev;
+ if (!new_desc->urb)
+ goto out_kfree;
+ new_desc->usb_dev = dev;
init_MUTEX(&new_desc->lock);
- if(mts_list){
- new_desc->host_number = mts_list->host_number+1;
- } else {
- new_desc->host_number = 0;
- }
-
/* endpoints */
-
new_desc->ep_out = ep_out;
new_desc->ep_response = ep_in_set[0];
new_desc->ep_image = ep_in_set[1];
-
if ( new_desc->ep_out != MTS_EP_OUT )
MTS_WARNING( "will this work? Command EP is not usually %d\n",
(int)new_desc->ep_out );
@@ -974,87 +810,48 @@
MTS_WARNING( "will this work? Image data EP is not usually %d\n",
(int)new_desc->ep_image );
+ new_desc->host = scsi_register(&mts_scsi_host_template,
+ sizeof(new_desc));
+ if (!new_desc->host)
+ goto out_free_urb;
- /* Initialize the host template based on the default one */
- memcpy(&(new_desc->ctempl), &mts_scsi_host_template, sizeof(mts_scsi_host_template));
- /* HACK from usb-storage - this is needed for scsi detection */
- (struct mts_desc *)new_desc->ctempl.proc_dir = new_desc; /* FIXME */
-
- MTS_DEBUG("registering SCSI module\n");
-
- new_desc->ctempl.module = THIS_MODULE;
- result = scsi_register_host(&new_desc->ctempl);
- /* Will get hit back in microtek_detect by this func */
- if ( result )
- {
- MTS_ERROR( "error %d from scsi_register_host! Help!\n",
- (int)result );
-
- /* FIXME: need more cleanup? */
- kfree( new_desc );
- return -ENOMEM;
- }
- MTS_DEBUG_GOT_HERE();
-
- /* FIXME: the bomb is armed, must the host be registered under lock ? */
- /* join the list - lock it */
- down(&mts_list_semaphore);
-
- mts_add_nolock( new_desc );
-
- up(&mts_list_semaphore);
-
-
- MTS_DEBUG("completed probe and exiting happily\n");
+ new_desc->host->hostdata[0] = (unsigned long)new_desc;
+ scsi_add_host(new_desc->host, NULL);
usb_set_intfdata(intf, new_desc);
return 0;
-}
-
+ out_free_urb:
+ usb_free_urb(new_desc->urb);
+ out_kfree:
+ kfree(new_desc);
+ out:
+ return -ENOMEM;
+}
-/* get us noticed by the rest of the kernel */
-
-int __init microtek_drv_init(void)
+static void mts_usb_disconnect (struct usb_interface *intf)
{
- int result;
+ struct mts_desc *desc = usb_get_intfdata(intf);
- MTS_DEBUG_GOT_HERE();
- init_MUTEX(&mts_list_semaphore);
-
- if ((result = usb_register(&mts_usb_driver)) < 0) {
- MTS_DEBUG("usb_register returned %d\n", result );
- return -1;
- } else {
- MTS_DEBUG("driver registered.\n");
- }
+ usb_set_intfdata(intf, NULL);
- info(DRIVER_VERSION ":" DRIVER_DESC);
+ scsi_remove_host(desc->host);
+ usb_unlink_urb(desc->urb);
+ scsi_unregister(desc->host);
- return 0;
+ usb_free_urb(desc->urb);
+ kfree(desc);
}
-void __exit microtek_drv_exit(void)
-{
- struct mts_desc* next;
- MTS_DEBUG_GOT_HERE();
+static int __init microtek_drv_init(void)
+{
+ return usb_register(&mts_usb_driver);
+}
+static void __exit microtek_drv_exit(void)
+{
usb_deregister(&mts_usb_driver);
-
- down(&mts_list_semaphore);
-
- while (mts_list) {
- /* keep track of where the next one is */
- next = mts_list->next;
-
- mts_remove_nolock( mts_list );
-
- /* advance the list pointer */
- mts_list = next;
- }
-
- up(&mts_list_semaphore);
}
module_init(microtek_drv_init);
diff -Nru a/drivers/usb/image/microtek.h b/drivers/usb/image/microtek.h
--- a/drivers/usb/image/microtek.h Fri May 30 11:35:21 2003
+++ b/drivers/usb/image/microtek.h Fri May 30 11:35:21 2003
@@ -38,9 +38,6 @@
u8 ep_image;
struct Scsi_Host * host;
- Scsi_Host_Template ctempl;
- int host_number;
-
struct semaphore lock;
struct urb *urb;