| Message ID | 20200523101129.GB98132@mwanda (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 9d7464b18892332e35ff37f0b024429a1a9835e6 |
| Headers | show |
| Series | scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd() | expand |
On Sat, 23 May 2020 13:11:29 +0300, Dan Carpenter wrote: > The pr_debug() dereferences "cmd" after we already freed it by calling > tcmu_free_cmd(cmd). The debug printk needs to be done earlier. > > Fixes: 61fb24822166 ("scsi: target: tcmu: Userspace must not complete queued commands") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by: David Disseldorp <ddiss@suse.de> Looks like this could be squashed in with the change that it fixes, given that 5.8/scsi-queue hasn't gone out yet. Cheers, David
On 5/23/20 5:11 AM, Dan Carpenter wrote: > The pr_debug() dereferences "cmd" after we already freed it by calling > tcmu_free_cmd(cmd). The debug printk needs to be done earlier. > > Fixes: 61fb24822166 ("scsi: target: tcmu: Userspace must not complete queued commands") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/target/target_core_user.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c > index 904d8a8373f2..28fb9441de7a 100644 > --- a/drivers/target/target_core_user.c > +++ b/drivers/target/target_core_user.c > @@ -1292,13 +1292,13 @@ static void tcmu_check_expired_queue_cmd(struct tcmu_cmd *cmd) > if (!time_after(jiffies, cmd->deadline)) > return; > > + pr_debug("Timing out queued cmd %p on dev %s.\n", > + cmd, cmd->tcmu_dev->name); > + > list_del_init(&cmd->queue_entry); > se_cmd = cmd->se_cmd; > tcmu_free_cmd(cmd); > > - pr_debug("Timing out queued cmd %p on dev %s.\n", > - cmd, cmd->tcmu_dev->name); > - > target_complete_cmd(se_cmd, SAM_STAT_TASK_SET_FULL); > } > Thanks. Reviewed-by: Mike Christie <mchristi@redhat.com>
On 05/23/20 12:11, Dan Carpenter wrote: > The pr_debug() dereferences "cmd" after we already freed it by calling > tcmu_free_cmd(cmd). The debug printk needs to be done earlier. > > Fixes: 61fb24822166 ("scsi: target: tcmu: Userspace must not complete queued commands") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/target/target_core_user.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Thank you. I'm very sorry for this stupid bug. BR, Bodo
On Mon, May 25, 2020 at 11:11:38AM +0200, Bodo Stroesser wrote: > On 05/23/20 12:11, Dan Carpenter wrote: > > The pr_debug() dereferences "cmd" after we already freed it by calling > > tcmu_free_cmd(cmd). The debug printk needs to be done earlier. > > > > Fixes: 61fb24822166 ("scsi: target: tcmu: Userspace must not complete queued commands") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > drivers/target/target_core_user.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > Thank you. > > I'm very sorry for this stupid bug. Bugs like this are super common. Part of being human. It would have been hard to discover via testing because you have disable memory poisoning on free and debugging at the same time. regards, dan carpenter
On Sat, 23 May 2020 13:11:29 +0300, Dan Carpenter wrote: > The pr_debug() dereferences "cmd" after we already freed it by calling > tcmu_free_cmd(cmd). The debug printk needs to be done earlier. Applied to 5.8/scsi-queue, thanks! [1/1] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd() https://git.kernel.org/mkp/scsi/c/9d7464b18892
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index 904d8a8373f2..28fb9441de7a 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -1292,13 +1292,13 @@ static void tcmu_check_expired_queue_cmd(struct tcmu_cmd *cmd) if (!time_after(jiffies, cmd->deadline)) return; + pr_debug("Timing out queued cmd %p on dev %s.\n", + cmd, cmd->tcmu_dev->name); + list_del_init(&cmd->queue_entry); se_cmd = cmd->se_cmd; tcmu_free_cmd(cmd); - pr_debug("Timing out queued cmd %p on dev %s.\n", - cmd, cmd->tcmu_dev->name); - target_complete_cmd(se_cmd, SAM_STAT_TASK_SET_FULL); }
The pr_debug() dereferences "cmd" after we already freed it by calling tcmu_free_cmd(cmd). The debug printk needs to be done earlier. Fixes: 61fb24822166 ("scsi: target: tcmu: Userspace must not complete queued commands") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/target/target_core_user.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)