Skip to content

Commit 9e95fb8

Browse files
ssudhakarpmartinkpetersen
authored andcommitted
scsi: target: Fix NULL pointer dereference
NULL pointer dereference happens when the following conditions are met: 1) A SCSI command is received for a non-existing LU or cdb initialization fails in target_setup_cmd_from_cdb(). 2) Tracing is enabled. The following call sequences lead to NULL pointer dereference: 1) iscsit_setup_scsi_cmd transport_lookup_cmd_lun <-- lookup fails. or target_setup_cmd_from_cdb() <-- cdb initialization fails iscsit_process_scsi_cmd iscsit_sequence_cmd transport_send_check_condition_and_sense trace_target_cmd_complete <-- NULL dereference 2) target_submit_cmd_map_sgls transport_lookup_cmd_lun <-- lookup fails or target_setup_cmd_from_cdb() <-- cdb initialization fails transport_send_check_condition_and_sense trace_target_cmd_complete <-- NULL dereference In the above sequence, cmd->t_task_cdb is uninitialized which when referenced in trace_target_cmd_complete() causes NULL pointer dereference. The fix is to use the helper, target_cmd_init_cdb() and call it after transport_init_se_cmd() is called, so that cmd->t_task_cdb can be initialized and hence can be referenced in trace_target_cmd_complete(). Link: https://lore.kernel.org/r/1591559913-8388-4-git-send-email-sudhakar.panneerselvam@oracle.com Reviewed-by: Mike Christie <[email protected]> Signed-off-by: Sudhakar Panneerselvam <[email protected]> Signed-off-by: Martin K. Petersen <[email protected]>
1 parent a36840d commit 9e95fb8

File tree

3 files changed

+39
-13
lines changed

3 files changed

+39
-13
lines changed

drivers/target/iscsi/iscsi_target.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,21 +1167,25 @@ int iscsit_setup_scsi_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
11671167

11681168
target_get_sess_cmd(&cmd->se_cmd, true);
11691169

1170+
cmd->sense_reason = target_cmd_init_cdb(&cmd->se_cmd, hdr->cdb);
1171+
if (cmd->sense_reason) {
1172+
if (cmd->sense_reason == TCM_OUT_OF_RESOURCES) {
1173+
return iscsit_add_reject_cmd(cmd,
1174+
ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
1175+
}
1176+
1177+
goto attach_cmd;
1178+
}
1179+
11701180
cmd->sense_reason = transport_lookup_cmd_lun(&cmd->se_cmd);
11711181
if (cmd->sense_reason)
11721182
goto attach_cmd;
11731183

11741184
/* only used for printks or comparing with ->ref_task_tag */
11751185
cmd->se_cmd.tag = (__force u32)cmd->init_task_tag;
11761186
cmd->sense_reason = target_setup_cmd_from_cdb(&cmd->se_cmd, hdr->cdb);
1177-
if (cmd->sense_reason) {
1178-
if (cmd->sense_reason == TCM_OUT_OF_RESOURCES) {
1179-
return iscsit_add_reject_cmd(cmd,
1180-
ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
1181-
}
1182-
1187+
if (cmd->sense_reason)
11831188
goto attach_cmd;
1184-
}
11851189

11861190
if (iscsit_build_pdu_and_seq_lists(cmd, payload_length) < 0) {
11871191
return iscsit_add_reject_cmd(cmd,

drivers/target/target_core_transport.c

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,6 +1413,9 @@ transport_check_alloc_task_attr(struct se_cmd *cmd)
14131413
sense_reason_t
14141414
target_cmd_init_cdb(struct se_cmd *cmd, unsigned char *cdb)
14151415
{
1416+
sense_reason_t ret;
1417+
1418+
cmd->t_task_cdb = &cmd->__t_task_cdb[0];
14161419
/*
14171420
* Ensure that the received CDB is less than the max (252 + 8) bytes
14181421
* for VARIABLE_LENGTH_CMD
@@ -1421,7 +1424,8 @@ target_cmd_init_cdb(struct se_cmd *cmd, unsigned char *cdb)
14211424
pr_err("Received SCSI CDB with command_size: %d that"
14221425
" exceeds SCSI_MAX_VARLEN_CDB_SIZE: %d\n",
14231426
scsi_command_size(cdb), SCSI_MAX_VARLEN_CDB_SIZE);
1424-
return TCM_INVALID_CDB_FIELD;
1427+
ret = TCM_INVALID_CDB_FIELD;
1428+
goto err;
14251429
}
14261430
/*
14271431
* If the received CDB is larger than TCM_MAX_COMMAND_SIZE,
@@ -1436,17 +1440,26 @@ target_cmd_init_cdb(struct se_cmd *cmd, unsigned char *cdb)
14361440
" %u > sizeof(cmd->__t_task_cdb): %lu ops\n",
14371441
scsi_command_size(cdb),
14381442
(unsigned long)sizeof(cmd->__t_task_cdb));
1439-
return TCM_OUT_OF_RESOURCES;
1443+
ret = TCM_OUT_OF_RESOURCES;
1444+
goto err;
14401445
}
1441-
} else
1442-
cmd->t_task_cdb = &cmd->__t_task_cdb[0];
1446+
}
14431447
/*
14441448
* Copy the original CDB into cmd->
14451449
*/
14461450
memcpy(cmd->t_task_cdb, cdb, scsi_command_size(cdb));
14471451

14481452
trace_target_sequencer_start(cmd);
14491453
return 0;
1454+
1455+
err:
1456+
/*
1457+
* Copy the CDB here to allow trace_target_cmd_complete() to
1458+
* print the cdb to the trace buffers.
1459+
*/
1460+
memcpy(cmd->t_task_cdb, cdb, min(scsi_command_size(cdb),
1461+
(unsigned int)TCM_MAX_COMMAND_SIZE));
1462+
return ret;
14501463
}
14511464
EXPORT_SYMBOL(target_cmd_init_cdb);
14521465

@@ -1456,8 +1469,6 @@ target_setup_cmd_from_cdb(struct se_cmd *cmd, unsigned char *cdb)
14561469
struct se_device *dev = cmd->se_dev;
14571470
sense_reason_t ret;
14581471

1459-
target_cmd_init_cdb(cmd, cdb);
1460-
14611472
ret = dev->transport->parse_cdb(cmd);
14621473
if (ret == TCM_UNSUPPORTED_SCSI_OPCODE)
14631474
pr_warn_ratelimited("%s/%s: Unsupported SCSI Opcode 0x%02x, sending CHECK_CONDITION.\n",
@@ -1621,6 +1632,14 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess
16211632
*/
16221633
if (flags & TARGET_SCF_BIDI_OP)
16231634
se_cmd->se_cmd_flags |= SCF_BIDI;
1635+
1636+
rc = target_cmd_init_cdb(se_cmd, cdb);
1637+
if (rc) {
1638+
transport_send_check_condition_and_sense(se_cmd, rc, 0);
1639+
target_put_sess_cmd(se_cmd);
1640+
return 0;
1641+
}
1642+
16241643
/*
16251644
* Locate se_lun pointer and attach it to struct se_cmd
16261645
*/

drivers/target/target_core_xcopy.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,9 @@ static int target_xcopy_setup_pt_cmd(
526526
}
527527
cmd->se_cmd_flags |= SCF_SE_LUN_CMD;
528528

529+
if (target_cmd_init_cdb(cmd, cdb))
530+
return -EINVAL;
531+
529532
cmd->tag = 0;
530533
if (target_setup_cmd_from_cdb(cmd, cdb))
531534
return -EINVAL;

0 commit comments

Comments
 (0)