firewire: fw-sbp2: fix races
1: There is a small race between queue_delayed_work() and its corresponding kref_get(). Do the kref_get first, and _put it again if the queue_delayed_work() failed, so there is no chance of the kref going to zero while the work is scheduled. 2: An SBP2_LOGOUT_REQUEST could be sent out with a login_id full of garbage. Initialize it to an invalid value so we can tell if we ever got a valid login_id. 3: The node ID and generation may have changed but the new values may not yet have been recorded in lu and tgt when the final logout is attempted. Use the latest values from the device in sbp2_release_target(). Signed-off-by: Jay Fenlason <fenlason@redhat.com> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This commit is contained in:
parent
0dcfeb7e3c
commit
cd1f70fdb4
|
@ -173,6 +173,9 @@ struct sbp2_target {
|
||||||
int blocked; /* ditto */
|
int blocked; /* ditto */
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/* Impossible login_id, to detect logout attempt before successful login */
|
||||||
|
#define INVALID_LOGIN_ID 0x10000
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Per section 7.4.8 of the SBP-2 spec, a mgt_ORB_timeout value can be
|
* Per section 7.4.8 of the SBP-2 spec, a mgt_ORB_timeout value can be
|
||||||
* provided in the config rom. Most devices do provide a value, which
|
* provided in the config rom. Most devices do provide a value, which
|
||||||
|
@ -788,9 +791,20 @@ static void sbp2_release_target(struct kref *kref)
|
||||||
scsi_remove_device(sdev);
|
scsi_remove_device(sdev);
|
||||||
scsi_device_put(sdev);
|
scsi_device_put(sdev);
|
||||||
}
|
}
|
||||||
sbp2_send_management_orb(lu, tgt->node_id, lu->generation,
|
if (lu->login_id != INVALID_LOGIN_ID) {
|
||||||
SBP2_LOGOUT_REQUEST, lu->login_id, NULL);
|
int generation, node_id;
|
||||||
|
/*
|
||||||
|
* tgt->node_id may be obsolete here if we failed
|
||||||
|
* during initial login or after a bus reset where
|
||||||
|
* the topology changed.
|
||||||
|
*/
|
||||||
|
generation = device->generation;
|
||||||
|
smp_rmb(); /* node_id vs. generation */
|
||||||
|
node_id = device->node_id;
|
||||||
|
sbp2_send_management_orb(lu, node_id, generation,
|
||||||
|
SBP2_LOGOUT_REQUEST,
|
||||||
|
lu->login_id, NULL);
|
||||||
|
}
|
||||||
fw_core_remove_address_handler(&lu->address_handler);
|
fw_core_remove_address_handler(&lu->address_handler);
|
||||||
list_del(&lu->link);
|
list_del(&lu->link);
|
||||||
kfree(lu);
|
kfree(lu);
|
||||||
|
@ -805,19 +819,20 @@ static void sbp2_release_target(struct kref *kref)
|
||||||
|
|
||||||
static struct workqueue_struct *sbp2_wq;
|
static struct workqueue_struct *sbp2_wq;
|
||||||
|
|
||||||
|
static void sbp2_target_put(struct sbp2_target *tgt)
|
||||||
|
{
|
||||||
|
kref_put(&tgt->kref, sbp2_release_target);
|
||||||
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Always get the target's kref when scheduling work on one its units.
|
* Always get the target's kref when scheduling work on one its units.
|
||||||
* Each workqueue job is responsible to call sbp2_target_put() upon return.
|
* Each workqueue job is responsible to call sbp2_target_put() upon return.
|
||||||
*/
|
*/
|
||||||
static void sbp2_queue_work(struct sbp2_logical_unit *lu, unsigned long delay)
|
static void sbp2_queue_work(struct sbp2_logical_unit *lu, unsigned long delay)
|
||||||
{
|
{
|
||||||
if (queue_delayed_work(sbp2_wq, &lu->work, delay))
|
|
||||||
kref_get(&lu->tgt->kref);
|
kref_get(&lu->tgt->kref);
|
||||||
}
|
if (!queue_delayed_work(sbp2_wq, &lu->work, delay))
|
||||||
|
sbp2_target_put(lu->tgt);
|
||||||
static void sbp2_target_put(struct sbp2_target *tgt)
|
|
||||||
{
|
|
||||||
kref_put(&tgt->kref, sbp2_release_target);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -978,6 +993,7 @@ static int sbp2_add_logical_unit(struct sbp2_target *tgt, int lun_entry)
|
||||||
|
|
||||||
lu->tgt = tgt;
|
lu->tgt = tgt;
|
||||||
lu->lun = lun_entry & 0xffff;
|
lu->lun = lun_entry & 0xffff;
|
||||||
|
lu->login_id = INVALID_LOGIN_ID;
|
||||||
lu->retries = 0;
|
lu->retries = 0;
|
||||||
lu->has_sdev = false;
|
lu->has_sdev = false;
|
||||||
lu->blocked = false;
|
lu->blocked = false;
|
||||||
|
|
Loading…
Reference in a new issue