| Message ID | 20210720225522.26291-1-ddiss@suse.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a47fa41381a09e5997afd762664db4f5f6657e03 |
| Headers | show |
| Series | scsi: target: fix NULL dereference on XCOPY completion | expand |
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>
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
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;
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(-)