On Fri, 2017-01-13 at 20:53 +0530, Varun Prakash wrote:
> Split iscsit_check_dataout_hdr() into two functions
> 1. __iscsit_check_dataout_hdr() - This function
> validates data out hdr.
> 2. iscsit_check_dataout_hdr() - This function finds
> iSCSI cmd using iscsit_find_cmd_from_itt_or_dump(),
> then it calls __iscsit_check_dataout_hdr() to
> validate iSCSI hdr.
>
> This split is required to support Chelsio T6 iSCSI
> DDP completion feature. T6 adapters reduce number of
> completions to host by generating single completion
> for all directly placed(DDP) iSCSI pdus in a sequence,
> DDP completion contains iSCSI hdr of the last pdu in a
> sequence.
>
> On receiving DDP completion cxgbit driver will first
> find iSCSI cmd using iscsit_find_cmd_from_itt_or_dump()
> then updates cmd->write_data_done, cmd->next_burst_len,
> cmd->data_sn and calls __iscsit_check_dataout_hdr()
> to validate iSCSI hdr.
>
> Signed-off-by: Varun Prakash <varun@xxxxxxxxxxx>
> ---
> drivers/target/iscsi/iscsi_target.c | 38 +++++++++++++++++++++++---------
> drivers/target/iscsi/iscsi_target_util.c | 1 +
> include/target/iscsi/iscsi_transport.h | 11 +++++++--
> 3 files changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index da2c73a..3120068 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -1431,11 +1431,10 @@ static void iscsit_do_crypto_hash_buf(
> }
>
> int
> -iscsit_check_dataout_hdr(struct iscsi_conn *conn, unsigned char *buf,
> - struct iscsi_cmd **out_cmd)
> +__iscsit_check_dataout_hdr(struct iscsi_conn *conn, void *buf,
> + struct iscsi_cmd *cmd, bool *success)
> {
> - struct iscsi_data *hdr = (struct iscsi_data *)buf;
> - struct iscsi_cmd *cmd = NULL;
> + struct iscsi_data *hdr = buf;
> struct se_cmd *se_cmd;
> u32 payload_length = ntoh24(hdr->dlength);
> int rc;
> @@ -1456,11 +1455,6 @@ iscsit_check_dataout_hdr(struct iscsi_conn *conn, unsigned char *buf,
> buf);
> }
>
> - cmd = iscsit_find_cmd_from_itt_or_dump(conn, hdr->itt,
> - payload_length);
> - if (!cmd)
> - return 0;
> -
Proceeding this ITT lookup in existing iscsit_check_dataout_hdr() code,
there are checks for zero payload_length and MXDSL vs. received
payload_length.
As-is this change moves the ITT lookup and possible dump payload before
these two checks, which leaves the potential for a bogus payload length
to make it into iscsit_find_cmd_from_itt_or_dump(), which for a payload
dump involves a memory allocation.
That said, here is the updated version I've applied to avoid this case.
Note patch #5 has also been updated to use the new
__iscsit_check_dataout_hdr() parameter.
Thank you,
--nab
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index b4f1d1c..2285988 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1431,36 +1431,17 @@ static void iscsit_do_crypto_hash_buf(
}
int
-iscsit_check_dataout_hdr(struct iscsi_conn *conn, unsigned char *buf,
- struct iscsi_cmd **out_cmd)
+__iscsit_check_dataout_hdr(struct iscsi_conn *conn, void *buf,
+ struct iscsi_cmd *cmd, u32 payload_length,
+ bool *success)
{
- struct iscsi_data *hdr = (struct iscsi_data *)buf;
- struct iscsi_cmd *cmd = NULL;
+ struct iscsi_data *hdr = buf;
struct se_cmd *se_cmd;
- u32 payload_length = ntoh24(hdr->dlength);
int rc;
- if (!payload_length) {
- pr_warn("DataOUT payload is ZERO, ignoring.\n");
- return 0;
- }
-
/* iSCSI write */
atomic_long_add(payload_length, &conn->sess->rx_data_octets);
- if (payload_length > conn->conn_ops->MaxXmitDataSegmentLength) {
- pr_err("DataSegmentLength: %u is greater than"
- " MaxXmitDataSegmentLength: %u\n", payload_length,
- conn->conn_ops->MaxXmitDataSegmentLength);
- return iscsit_add_reject(conn, ISCSI_REASON_PROTOCOL_ERROR,
- buf);
- }
-
- cmd = iscsit_find_cmd_from_itt_or_dump(conn, hdr->itt,
- payload_length);
- if (!cmd)
- return 0;
-
pr_debug("Got DataOut ITT: 0x%08x, TTT: 0x%08x,"
" DataSN: 0x%08x, Offset: %u, Length: %u, CID: %hu\n",
hdr->itt, hdr->ttt, hdr->datasn, ntohl(hdr->offset),
@@ -1553,10 +1534,44 @@ static void iscsit_do_crypto_hash_buf(
return 0;
else if (rc == DATAOUT_CANNOT_RECOVER)
return -1;
-
- *out_cmd = cmd;
+ *success = true;
return 0;
}
+EXPORT_SYMBOL(__iscsit_check_dataout_hdr);
+
+int
+iscsit_check_dataout_hdr(struct iscsi_conn *conn, void *buf,
+ struct iscsi_cmd **out_cmd)
+{
+ struct iscsi_data *hdr = buf;
+ struct iscsi_cmd *cmd;
+ u32 payload_length = ntoh24(hdr->dlength);
+ int rc;
+ bool success = false;
+
+ if (!payload_length) {
+ pr_warn_ratelimited("DataOUT payload is ZERO, ignoring.\n");
+ return 0;
+ }
+
+ if (payload_length > conn->conn_ops->MaxXmitDataSegmentLength) {
+ pr_err_ratelimited("DataSegmentLength: %u is greater than"
+ " MaxXmitDataSegmentLength: %u\n", payload_length,
+ conn->conn_ops->MaxXmitDataSegmentLength);
+ return iscsit_add_reject(conn, ISCSI_REASON_PROTOCOL_ERROR, buf);
+ }
+
+ cmd = iscsit_find_cmd_from_itt_or_dump(conn, hdr->itt, payload_length);
+ if (!cmd)
+ return 0;
+
+ rc = __iscsit_check_dataout_hdr(conn, buf, cmd, payload_length, &success);
+
+ if (success)
+ *out_cmd = cmd;
+
+ return rc;
+}
EXPORT_SYMBOL(iscsit_check_dataout_hdr);
static int
diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index b5a1b4c..f46eadf 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -417,6 +417,7 @@ struct iscsi_cmd *iscsit_find_cmd_from_itt_or_dump(
return NULL;
}
+EXPORT_SYMBOL(iscsit_find_cmd_from_itt_or_dump);
struct iscsi_cmd *iscsit_find_cmd_from_ttt(
struct iscsi_conn *conn,
diff --git a/include/target/iscsi/iscsi_transport.h b/include/target/iscsi/iscsi_transport.h
index 1277e9b..ff1a4f4 100644
--- a/include/target/iscsi/iscsi_transport.h
+++ b/include/target/iscsi/iscsi_transport.h
@@ -55,8 +55,12 @@ extern int iscsit_setup_scsi_cmd(struct iscsi_conn *, struct iscsi_cmd *,
extern void iscsit_set_unsoliticed_dataout(struct iscsi_cmd *);
extern int iscsit_process_scsi_cmd(struct iscsi_conn *, struct iscsi_cmd *,
struct iscsi_scsi_req *);
-extern int iscsit_check_dataout_hdr(struct iscsi_conn *, unsigned char *,
- struct iscsi_cmd **);
+extern int
+__iscsit_check_dataout_hdr(struct iscsi_conn *, void *,
+ struct iscsi_cmd *, u32, bool *);
+extern int
+iscsit_check_dataout_hdr(struct iscsi_conn *conn, void *buf,
+ struct iscsi_cmd **out_cmd);
extern int iscsit_check_dataout_payload(struct iscsi_cmd *, struct iscsi_data *,
bool);
extern int iscsit_setup_nop_out(struct iscsi_conn *, struct iscsi_cmd *,
@@ -125,6 +129,9 @@ extern int iscsit_sequence_cmd(struct iscsi_conn *, struct iscsi_cmd *,
extern void iscsit_free_cmd(struct iscsi_cmd *, bool);
extern void iscsit_add_cmd_to_immediate_queue(struct iscsi_cmd *,
struct iscsi_conn *, u8);
+extern struct iscsi_cmd *
+iscsit_find_cmd_from_itt_or_dump(struct iscsi_conn *conn,
+ itt_t init_task_tag, u32 length);
/*
* From iscsi_target_nego.c
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
