target-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] scsi: target: fix NULL dereference on XCOPY completion
@ 2021-07-20 22:55 David Disseldorp
  2021-07-21  0:22 ` michael.christie
  2021-07-21  3:22 ` Martin K. Petersen
  0 siblings, 2 replies; 3+ messages in thread
From: David Disseldorp @ 2021-07-20 22:55 UTC (permalink / raw)
  To: target-devel; +Cc: martin.petersen, David Disseldorp, Lee Duncan, Mike Christie

CPU affinity control added with 39ae3edda325 makes target_complete_cmd()
queue work on a CPU based on se_tpg->se_tpg_wwn->cmd_compl_affinity
state.

LIO's EXTENDED COPY worker is a special case in that read/write cmds
are dispatched using the global xcopy_pt_tpg, which carries a NULL
se_tpg_wwn pointer following initialization in target_xcopy_setup_pt().

The NULL xcopy_pt_tpg->se_tpg_wwn pointer is dereferenced on completion
of any EXTENDED COPY initiated read/write cmds. E.g using the libiscsi
SCSI.ExtendedCopy.Simple test:
  BUG: kernel NULL pointer dereference, address: 00000000000001a8
  RIP: 0010:target_complete_cmd+0x9d/0x130 [target_core_mod]
  Call Trace:
   fd_execute_rw+0x148/0x42a [target_core_file]
   ? __dynamic_pr_debug+0xa7/0xe0
   ? target_check_reservation+0x5b/0x940 [target_core_mod]
   __target_execute_cmd+0x1e/0x90 [target_core_mod]
   transport_generic_new_cmd+0x17c/0x330 [target_core_mod]
   target_xcopy_issue_pt_cmd+0x9/0x60 [target_core_mod]
   target_xcopy_read_source.isra.7+0x10b/0x1b0 [target_core_mod]
   ? target_check_fua+0x40/0x40 [target_core_mod]
   ? transport_complete_task_attr+0x130/0x130 [target_core_mod]
   target_xcopy_do_work+0x61f/0xc00 [target_core_mod]

This fix makes target_complete_cmd() queue work on se_cmd->cpuid if
se_tpg_wwn is NULL.

Fixes: 39ae3edda325 ("scsi: target: core: Make completion affinity configurable")
Signed-off-by: David Disseldorp <ddiss@suse.de>
Cc: Lee Duncan <lduncan@suse.com>
Cc: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/target_core_transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 7e35eddd9eb7..26ceabe34de5 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -886,7 +886,7 @@ void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
 	INIT_WORK(&cmd->work, success ? target_complete_ok_work :
 		  target_complete_failure_work);
 
-	if (wwn->cmd_compl_affinity == SE_COMPL_AFFINITY_CPUID)
+	if (!wwn || wwn->cmd_compl_affinity == SE_COMPL_AFFINITY_CPUID)
 		cpu = cmd->cpuid;
 	else
 		cpu = wwn->cmd_compl_affinity;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] scsi: target: fix NULL dereference on XCOPY completion
  2021-07-20 22:55 [PATCH] scsi: target: fix NULL dereference on XCOPY completion David Disseldorp
@ 2021-07-21  0:22 ` michael.christie
  2021-07-21  3:22 ` Martin K. Petersen
  1 sibling, 0 replies; 3+ messages in thread
From: michael.christie @ 2021-07-21  0:22 UTC (permalink / raw)
  To: David Disseldorp, target-devel; +Cc: martin.petersen, Lee Duncan

On 7/20/21 5:55 PM, David Disseldorp wrote:
> CPU affinity control added with 39ae3edda325 makes target_complete_cmd()
> queue work on a CPU based on se_tpg->se_tpg_wwn->cmd_compl_affinity
> state.
> 
> LIO's EXTENDED COPY worker is a special case in that read/write cmds
> are dispatched using the global xcopy_pt_tpg, which carries a NULL
> se_tpg_wwn pointer following initialization in target_xcopy_setup_pt().
> 
> The NULL xcopy_pt_tpg->se_tpg_wwn pointer is dereferenced on completion
> of any EXTENDED COPY initiated read/write cmds. E.g using the libiscsi
> SCSI.ExtendedCopy.Simple test:
>   BUG: kernel NULL pointer dereference, address: 00000000000001a8
>   RIP: 0010:target_complete_cmd+0x9d/0x130 [target_core_mod]
>   Call Trace:
>    fd_execute_rw+0x148/0x42a [target_core_file]
>    ? __dynamic_pr_debug+0xa7/0xe0
>    ? target_check_reservation+0x5b/0x940 [target_core_mod]
>    __target_execute_cmd+0x1e/0x90 [target_core_mod]
>    transport_generic_new_cmd+0x17c/0x330 [target_core_mod]
>    target_xcopy_issue_pt_cmd+0x9/0x60 [target_core_mod]
>    target_xcopy_read_source.isra.7+0x10b/0x1b0 [target_core_mod]
>    ? target_check_fua+0x40/0x40 [target_core_mod]
>    ? transport_complete_task_attr+0x130/0x130 [target_core_mod]
>    target_xcopy_do_work+0x61f/0xc00 [target_core_mod]
> 
> This fix makes target_complete_cmd() queue work on se_cmd->cpuid if
> se_tpg_wwn is NULL.
> 
> Fixes: 39ae3edda325 ("scsi: target: core: Make completion affinity configurable")
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> Cc: Lee Duncan <lduncan@suse.com>
> Cc: Mike Christie <michael.christie@oracle.com>
> ---
>  drivers/target/target_core_transport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index 7e35eddd9eb7..26ceabe34de5 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -886,7 +886,7 @@ void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
>  	INIT_WORK(&cmd->work, success ? target_complete_ok_work :
>  		  target_complete_failure_work);
>  
> -	if (wwn->cmd_compl_affinity == SE_COMPL_AFFINITY_CPUID)
> +	if (!wwn || wwn->cmd_compl_affinity == SE_COMPL_AFFINITY_CPUID)
>  		cpu = cmd->cpuid;
>  	else
>  		cpu = wwn->cmd_compl_affinity;
> 

Reviewed-by: Mike Christie <michael.christie@oracle.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] scsi: target: fix NULL dereference on XCOPY completion
  2021-07-20 22:55 [PATCH] scsi: target: fix NULL dereference on XCOPY completion David Disseldorp
  2021-07-21  0:22 ` michael.christie
@ 2021-07-21  3:22 ` Martin K. Petersen
  1 sibling, 0 replies; 3+ messages in thread
From: Martin K. Petersen @ 2021-07-21  3:22 UTC (permalink / raw)
  To: David Disseldorp, target-devel
  Cc: Martin K . Petersen, Mike Christie, Lee Duncan

On Wed, 21 Jul 2021 00:55:22 +0200, David Disseldorp wrote:

> CPU affinity control added with 39ae3edda325 makes target_complete_cmd()
> queue work on a CPU based on se_tpg->se_tpg_wwn->cmd_compl_affinity
> state.
> 
> LIO's EXTENDED COPY worker is a special case in that read/write cmds
> are dispatched using the global xcopy_pt_tpg, which carries a NULL
> se_tpg_wwn pointer following initialization in target_xcopy_setup_pt().
> 
> [...]

Applied to 5.14/scsi-fixes, thanks!

[1/1] scsi: target: fix NULL dereference on XCOPY completion
      https://git.kernel.org/mkp/scsi/c/a47fa41381a0

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-07-21  3:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-20 22:55 [PATCH] scsi: target: fix NULL dereference on XCOPY completion David Disseldorp
2021-07-21  0:22 ` michael.christie
2021-07-21  3:22 ` 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).