* [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
@ 2020-05-23 10:11 Dan Carpenter
2020-05-23 13:03 ` David Disseldorp
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Dan Carpenter @ 2020-05-23 10:11 UTC (permalink / raw)
To: Martin K. Petersen, Bodo Stroesser
Cc: Mike Christie, linux-scsi, target-devel, kernel-janitors
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);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
2020-05-23 10:11 [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd() Dan Carpenter
@ 2020-05-23 13:03 ` David Disseldorp
2020-05-23 19:04 ` Mike Christie
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: David Disseldorp @ 2020-05-23 13:03 UTC (permalink / raw)
To: Dan Carpenter
Cc: Martin K. Petersen, Bodo Stroesser, Mike Christie, linux-scsi,
target-devel, kernel-janitors
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
2020-05-23 10:11 [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd() Dan Carpenter
2020-05-23 13:03 ` David Disseldorp
@ 2020-05-23 19:04 ` Mike Christie
2020-05-25 9:11 ` Bodo Stroesser
2020-05-27 2:12 ` Martin K. Petersen
3 siblings, 0 replies; 6+ messages in thread
From: Mike Christie @ 2020-05-23 19:04 UTC (permalink / raw)
To: Dan Carpenter, Martin K. Petersen, Bodo Stroesser
Cc: linux-scsi, target-devel, kernel-janitors
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>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
2020-05-23 10:11 [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd() Dan Carpenter
2020-05-23 13:03 ` David Disseldorp
2020-05-23 19:04 ` Mike Christie
@ 2020-05-25 9:11 ` Bodo Stroesser
2020-05-26 12:26 ` Dan Carpenter
2020-05-27 2:12 ` Martin K. Petersen
3 siblings, 1 reply; 6+ messages in thread
From: Bodo Stroesser @ 2020-05-25 9:11 UTC (permalink / raw)
To: Dan Carpenter, Martin K. Petersen
Cc: Mike Christie, linux-scsi, target-devel, kernel-janitors
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
2020-05-25 9:11 ` Bodo Stroesser
@ 2020-05-26 12:26 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2020-05-26 12:26 UTC (permalink / raw)
To: Bodo Stroesser
Cc: Martin K. Petersen, Mike Christie, linux-scsi, target-devel,
kernel-janitors
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
2020-05-23 10:11 [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd() Dan Carpenter
` (2 preceding siblings ...)
2020-05-25 9:11 ` Bodo Stroesser
@ 2020-05-27 2:12 ` Martin K. Petersen
3 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2020-05-27 2:12 UTC (permalink / raw)
To: Dan Carpenter, Bodo Stroesser
Cc: Martin K . Petersen, Mike Christie, linux-scsi, target-devel,
kernel-janitors
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
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-27 2:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-23 10:11 [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd() Dan Carpenter
2020-05-23 13:03 ` David Disseldorp
2020-05-23 19:04 ` Mike Christie
2020-05-25 9:11 ` Bodo Stroesser
2020-05-26 12:26 ` Dan Carpenter
2020-05-27 2:12 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).