On Thu, 2014-01-30 at 13:25 -0800, Andy Grover wrote: > Don't use pr_reg after free_registration() is called. > > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > Signed-off-by: Andy Grover <agrover@xxxxxxxxxx> > --- > > Hi Nab, > > I'll take a half-Grr but I think the second bit was pre-existing. > Look again, there was no pre-existing bug in <= v3.10.y. > Here's an alternative patch that may be preferable, assuming it's ok > to move the free_registration() after the ua_allocate loop. > This patch would introduce yet another free-after-use when core_scsi3_ua_allocate() is called on the *pr_reg that is about to be released.. --nab > Regards -- Andy > > drivers/target/target_core_pr.c | 14 ++++++++------ > 1 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c > index 2f5d779..361eae1 100644 > --- a/drivers/target/target_core_pr.c > +++ b/drivers/target/target_core_pr.c > @@ -2158,11 +2158,6 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key, > } > > /* > - * Release the calling I_T Nexus registration now.. > - */ > - __core_scsi3_free_registration(cmd->se_dev, pr_reg, NULL, 1); > - > - /* > * From spc4r17, section 5.7.11.3 Unregistering > * > * If the persistent reservation is a registrants only > @@ -2188,13 +2183,20 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key, > } > } > > + /* > + * Release the calling I_T Nexus registration now.. > + */ > + __core_scsi3_free_registration(cmd->se_dev, pr_reg, NULL, 1); > + pr_reg = NULL; > + > spin_unlock(&pr_tmpl->registration_lock); > } > > 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
