All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
@ 2020-05-23 10:11 ` Dan Carpenter
  0 siblings, 0 replies; 12+ 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] 12+ messages in thread

* [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
@ 2020-05-23 10:11 ` Dan Carpenter
  0 siblings, 0 replies; 12+ 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] 12+ messages in thread

* Re: [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
  -1 siblings, 0 replies; 12+ 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] 12+ messages in thread

* Re: [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
@ 2020-05-23 13:03   ` David Disseldorp
  0 siblings, 0 replies; 12+ 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] 12+ messages in thread

* Re: [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 19:04   ` Mike Christie
  -1 siblings, 0 replies; 12+ 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] 12+ messages in thread

* Re: [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
@ 2020-05-23 19:04   ` Mike Christie
  0 siblings, 0 replies; 12+ 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] 12+ messages in thread

* Re: [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
  2020-05-23 10:11 ` Dan Carpenter
@ 2020-05-25  9:11   ` Bodo Stroesser
  -1 siblings, 0 replies; 12+ 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] 12+ 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
  0 siblings, 0 replies; 12+ 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] 12+ 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
  -1 siblings, 0 replies; 12+ 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] 12+ messages in thread

* Re: [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
@ 2020-05-26 12:26     ` Dan Carpenter
  0 siblings, 0 replies; 12+ 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] 12+ messages in thread

* Re: [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
  2020-05-23 10:11 ` Dan Carpenter
@ 2020-05-27  2:12   ` Martin K. Petersen
  -1 siblings, 0 replies; 12+ 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] 12+ messages in thread

* Re: [PATCH] scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd()
@ 2020-05-27  2:12   ` Martin K. Petersen
  0 siblings, 0 replies; 12+ 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] 12+ messages in thread

end of thread, other threads:[~2020-05-27  2:13 UTC | newest]

Thread overview: 12+ 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 10:11 ` Dan Carpenter
2020-05-23 13:03 ` David Disseldorp
2020-05-23 13:03   ` David Disseldorp
2020-05-23 19:04 ` Mike Christie
2020-05-23 19:04   ` Mike Christie
2020-05-25  9:11 ` Bodo Stroesser
2020-05-25  9:11   ` Bodo Stroesser
2020-05-26 12:26   ` Dan Carpenter
2020-05-26 12:26     ` Dan Carpenter
2020-05-27  2:12 ` Martin K. Petersen
2020-05-27  2:12   ` Martin K. Petersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.