Commit f8436158 authored by Stefan Richter's avatar Stefan Richter

firewire: fw-sbp2: better fix for NULL pointer dereference in scsi_remove_device

Patch "firewire: fw-sbp2: fix NULL pointer deref. in scsi_remove_device"
had the unintended effect that firewire-sbp2 could not be unloaded
anymore until all SBP-2 devices were unplugged.

We now fix the NULL pointer bug by reacquiring a reference to the sdev
instead of holding a reference to the sdev (and to the module) all the
time.
Signed-off-by: default avatarStefan Richter <stefanr@s5r6.in-berlin.de>
Tested-by: default avatarJarod Wilson <jwilson@redhat.com>
parent d395991c
...@@ -122,7 +122,6 @@ static const char sbp2_driver_name[] = "sbp2"; ...@@ -122,7 +122,6 @@ static const char sbp2_driver_name[] = "sbp2";
struct sbp2_logical_unit { struct sbp2_logical_unit {
struct sbp2_target *tgt; struct sbp2_target *tgt;
struct list_head link; struct list_head link;
struct scsi_device *sdev;
struct fw_address_handler address_handler; struct fw_address_handler address_handler;
struct list_head orb_list; struct list_head orb_list;
...@@ -139,6 +138,7 @@ struct sbp2_logical_unit { ...@@ -139,6 +138,7 @@ struct sbp2_logical_unit {
int generation; int generation;
int retries; int retries;
struct delayed_work work; struct delayed_work work;
bool has_sdev;
bool blocked; bool blocked;
}; };
...@@ -751,20 +751,33 @@ static void sbp2_unblock(struct sbp2_target *tgt) ...@@ -751,20 +751,33 @@ static void sbp2_unblock(struct sbp2_target *tgt)
scsi_unblock_requests(shost); scsi_unblock_requests(shost);
} }
static int sbp2_lun2int(u16 lun)
{
struct scsi_lun eight_bytes_lun;
memset(&eight_bytes_lun, 0, sizeof(eight_bytes_lun));
eight_bytes_lun.scsi_lun[0] = (lun >> 8) & 0xff;
eight_bytes_lun.scsi_lun[1] = lun & 0xff;
return scsilun_to_int(&eight_bytes_lun);
}
static void sbp2_release_target(struct kref *kref) static void sbp2_release_target(struct kref *kref)
{ {
struct sbp2_target *tgt = container_of(kref, struct sbp2_target, kref); struct sbp2_target *tgt = container_of(kref, struct sbp2_target, kref);
struct sbp2_logical_unit *lu, *next; struct sbp2_logical_unit *lu, *next;
struct Scsi_Host *shost = struct Scsi_Host *shost =
container_of((void *)tgt, struct Scsi_Host, hostdata[0]); container_of((void *)tgt, struct Scsi_Host, hostdata[0]);
struct scsi_device *sdev;
/* prevent deadlocks */ /* prevent deadlocks */
sbp2_unblock(tgt); sbp2_unblock(tgt);
list_for_each_entry_safe(lu, next, &tgt->lu_list, link) { list_for_each_entry_safe(lu, next, &tgt->lu_list, link) {
if (lu->sdev) { sdev = scsi_device_lookup(shost, 0, 0, sbp2_lun2int(lu->lun));
scsi_remove_device(lu->sdev); if (sdev) {
scsi_device_put(lu->sdev); scsi_remove_device(sdev);
scsi_device_put(sdev);
} }
sbp2_send_management_orb(lu, tgt->node_id, lu->generation, sbp2_send_management_orb(lu, tgt->node_id, lu->generation,
SBP2_LOGOUT_REQUEST, lu->login_id, NULL); SBP2_LOGOUT_REQUEST, lu->login_id, NULL);
...@@ -807,7 +820,6 @@ static void sbp2_login(struct work_struct *work) ...@@ -807,7 +820,6 @@ static void sbp2_login(struct work_struct *work)
struct fw_device *device = fw_device(tgt->unit->device.parent); struct fw_device *device = fw_device(tgt->unit->device.parent);
struct Scsi_Host *shost; struct Scsi_Host *shost;
struct scsi_device *sdev; struct scsi_device *sdev;
struct scsi_lun eight_bytes_lun;
struct sbp2_login_response response; struct sbp2_login_response response;
int generation, node_id, local_node_id; int generation, node_id, local_node_id;
...@@ -820,7 +832,7 @@ static void sbp2_login(struct work_struct *work) ...@@ -820,7 +832,7 @@ static void sbp2_login(struct work_struct *work)
local_node_id = device->card->node_id; local_node_id = device->card->node_id;
/* If this is a re-login attempt, log out, or we might be rejected. */ /* If this is a re-login attempt, log out, or we might be rejected. */
if (lu->sdev) if (lu->has_sdev)
sbp2_send_management_orb(lu, device->node_id, generation, sbp2_send_management_orb(lu, device->node_id, generation,
SBP2_LOGOUT_REQUEST, lu->login_id, NULL); SBP2_LOGOUT_REQUEST, lu->login_id, NULL);
...@@ -859,7 +871,7 @@ static void sbp2_login(struct work_struct *work) ...@@ -859,7 +871,7 @@ static void sbp2_login(struct work_struct *work)
sbp2_agent_reset(lu); sbp2_agent_reset(lu);
/* This was a re-login. */ /* This was a re-login. */
if (lu->sdev) { if (lu->has_sdev) {
sbp2_cancel_orbs(lu); sbp2_cancel_orbs(lu);
sbp2_conditionally_unblock(lu); sbp2_conditionally_unblock(lu);
goto out; goto out;
...@@ -868,13 +880,8 @@ static void sbp2_login(struct work_struct *work) ...@@ -868,13 +880,8 @@ static void sbp2_login(struct work_struct *work)
if (lu->tgt->workarounds & SBP2_WORKAROUND_DELAY_INQUIRY) if (lu->tgt->workarounds & SBP2_WORKAROUND_DELAY_INQUIRY)
ssleep(SBP2_INQUIRY_DELAY); ssleep(SBP2_INQUIRY_DELAY);
memset(&eight_bytes_lun, 0, sizeof(eight_bytes_lun));
eight_bytes_lun.scsi_lun[0] = (lu->lun >> 8) & 0xff;
eight_bytes_lun.scsi_lun[1] = lu->lun & 0xff;
shost = container_of((void *)tgt, struct Scsi_Host, hostdata[0]); shost = container_of((void *)tgt, struct Scsi_Host, hostdata[0]);
sdev = __scsi_add_device(shost, 0, 0, sbp2_lun2int(lu->lun), lu);
sdev = __scsi_add_device(shost, 0, 0,
scsilun_to_int(&eight_bytes_lun), lu);
/* /*
* FIXME: We are unable to perform reconnects while in sbp2_login(). * FIXME: We are unable to perform reconnects while in sbp2_login().
* Therefore __scsi_add_device() will get into trouble if a bus reset * Therefore __scsi_add_device() will get into trouble if a bus reset
...@@ -896,7 +903,8 @@ static void sbp2_login(struct work_struct *work) ...@@ -896,7 +903,8 @@ static void sbp2_login(struct work_struct *work)
} }
/* No error during __scsi_add_device() */ /* No error during __scsi_add_device() */
lu->sdev = sdev; lu->has_sdev = true;
scsi_device_put(sdev);
sbp2_allow_block(lu); sbp2_allow_block(lu);
goto out; goto out;
...@@ -934,11 +942,11 @@ static int sbp2_add_logical_unit(struct sbp2_target *tgt, int lun_entry) ...@@ -934,11 +942,11 @@ static int sbp2_add_logical_unit(struct sbp2_target *tgt, int lun_entry)
return -ENOMEM; return -ENOMEM;
} }
lu->tgt = tgt; lu->tgt = tgt;
lu->sdev = NULL; lu->lun = lun_entry & 0xffff;
lu->lun = lun_entry & 0xffff; lu->retries = 0;
lu->retries = 0; lu->has_sdev = false;
lu->blocked = false; lu->blocked = false;
++tgt->dont_block; ++tgt->dont_block;
INIT_LIST_HEAD(&lu->orb_list); INIT_LIST_HEAD(&lu->orb_list);
INIT_DELAYED_WORK(&lu->work, sbp2_login); INIT_DELAYED_WORK(&lu->work, sbp2_login);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment