On Thu, 2014-01-30 at 14:05 +0300, Dan Carpenter wrote:
> Hello Andy Grover,
>
> The patch bc118fe4c4a8: "target: Further refactoring of
> core_scsi3_emulate_pro_register()" from May 16, 2013, leads to the
> following static checker warning:
>
> drivers/target/target_core_pr.c:2177 core_scsi3_emulate_pro_register()
> warn: 'pr_reg' was already freed.
>
> drivers/target/target_core_pr.c
> 2161 * Release the calling I_T Nexus registration now..
> 2162 */
> 2163 __core_scsi3_free_registration(cmd->se_dev, pr_reg, NULL, 1);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Frees "pr_reg".
>
> 2164
> 2165 /*
> 2166 * From spc4r17, section 5.7.11.3 Unregistering
> 2167 *
> 2168 * If the persistent reservation is a registrants only
> 2169 * type, the device server shall establish a unit
> 2170 * attention condition for the initiator port associated
> 2171 * with every registered I_T nexus except for the I_T
> 2172 * nexus on which the PERSISTENT RESERVE OUT command was
> 2173 * received, with the additional sense code set to
> 2174 * RESERVATIONS RELEASED.
> 2175 */
> 2176 if (pr_holder &&
> 2177 (pr_reg->pr_res_type == PR_TYPE_WRITE_EXCLUSIVE_REGONLY ||
> ^^^^^^^^^^^^^^^^^^^
> 2178 pr_reg->pr_res_type == PR_TYPE_EXCLUSIVE_ACCESS_REGONLY)) {
> ^^^^^^^^^^^^^^^^^^^
> Dereferences.
>
> 2179 list_for_each_entry(pr_reg_p,
> 2180 &pr_tmpl->registration_list,
> 2181 pr_reg_list) {
>
Grrrrrr, Andy.
This is a free-after-use regression in >= v3.11 introduced under the
guise of 'refactoring' and 'removing unneeded variables'.
I'm applying the following fix to the target-pending queue for
v3.14-rc2.
--nab
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 2f5d779..3013287 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -2009,7 +2009,7 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa
struct t10_reservation *pr_tmpl = &dev->t10_pr;
unsigned char isid_buf[PR_REG_ISID_LEN], *isid_ptr = NULL;
sense_reason_t ret = TCM_NO_SENSE;
- int pr_holder = 0;
+ int pr_holder = 0, type;
if (!se_sess || !se_lun) {
pr_err("SPC-3 PR: se_sess || struct se_lun is NULL!\n");
@@ -2131,6 +2131,7 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa
ret = TCM_RESERVATION_CONFLICT;
goto out;
}
+ type = pr_reg->pr_res_type;
spin_lock(&pr_tmpl->registration_lock);
/*
@@ -2161,6 +2162,7 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa
* Release the calling I_T Nexus registration now..
*/
__core_scsi3_free_registration(cmd->se_dev, pr_reg, NULL, 1);
+ pr_reg = NULL;
/*
* From spc4r17, section 5.7.11.3 Unregistering
@@ -2174,8 +2176,8 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa
* RESERVATIONS RELEASED.
*/
if (pr_holder &&
- (pr_reg->pr_res_type == PR_TYPE_WRITE_EXCLUSIVE_REGONLY ||
- pr_reg->pr_res_type == PR_TYPE_EXCLUSIVE_ACCESS_REGONLY)) {
+ (type == PR_TYPE_WRITE_EXCLUSIVE_REGONLY ||
+ type == PR_TYPE_EXCLUSIVE_ACCESS_REGONLY)) {
list_for_each_entry(pr_reg_p,
&pr_tmpl->registration_list,
pr_reg_list) {
@@ -2194,7 +2196,8 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa
ret = core_scsi3_update_and_write_aptpl(dev, aptpl);
out:
- core_scsi3_put_pr_reg(pr_reg);
+ if (pr_reg)
+ core_scsi3_put_pr_reg(pr_reg);
return ret;
}
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
