Code inspection indicates these are possible. At the very least we
should catch them and call a bug a bug.
While at it, this also changes iscsit_map_iovec() to take an index into
cmd->iov_data instead of a pointer. Also fixes an off-by-one leading to
leaked kmap()s. iscsit_send_datain() called iscsit_map_iovec() with and
index of 1, resulting in cmd->kmapped_nents being one less than required
and not unmapping the last element in the vector.
The fact that noone seems to have noticed this bug yet implies that
target code doesn't get a lot of testing on 32bit platforms.
Signed-off-by: Joern Engel <joern@xxxxxxxxx>
---
drivers/target/iscsi/iscsi_target.c | 173 +++++++++++++-----------------------
1 file changed, 64 insertions(+), 109 deletions(-)
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 416824a168c2..edb32e955628 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -729,17 +729,29 @@ int iscsit_reject_cmd(struct iscsi_cmd *cmd, u8 reason, unsigned char *buf)
return iscsit_add_reject_from_cmd(cmd, reason, false, buf);
}
+static void iscsit_map_data_iovec(struct iscsi_cmd *cmd, int i, void *iov_base,
+ size_t iov_len)
+{
+ WARN_ON_ONCE(i > cmd->orig_iov_data_count);
+ cmd->iov_data[i].iov_base = iov_base;
+ cmd->iov_data[i].iov_len = iov_len;
+}
+
+static void iscsit_map_misc_iovec(struct iscsi_cmd *cmd, int i, void *iov_base,
+ size_t iov_len)
+{
+ WARN_ON_ONCE(i > ISCSI_MISC_IOVECS);
+ cmd->iov_misc[i].iov_base = iov_base;
+ cmd->iov_misc[i].iov_len = iov_len;
+}
+
/*
* Map some portion of the allocated scatterlist to an iovec, suitable for
* kernel sockets to copy data in/out.
*/
-static int iscsit_map_iovec(
- struct iscsi_cmd *cmd,
- struct kvec *iov,
- u32 data_offset,
- u32 data_length)
+static int iscsit_map_iovec(struct iscsi_cmd *cmd, int i, u32 data_offset,
+ u32 data_length)
{
- u32 i = 0;
struct scatterlist *sg;
unsigned int page_off;
@@ -755,13 +767,11 @@ static int iscsit_map_iovec(
while (data_length) {
u32 cur_len = min_t(u32, data_length, sg->length - page_off);
- iov[i].iov_base = kmap(sg_page(sg)) + sg->offset + page_off;
- iov[i].iov_len = cur_len;
+ iscsit_map_data_iovec(cmd, i++, kmap(sg_page(sg)) + sg->offset + page_off, cur_len);
data_length -= cur_len;
page_off = 0;
sg = sg_next(sg);
- i++;
}
cmd->kmapped_nents = i;
@@ -1396,32 +1406,24 @@ static int
iscsit_get_dataout(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
struct iscsi_data *hdr)
{
- struct kvec *iov;
- u32 checksum, iov_count = 0, padding = 0, rx_got = 0, rx_size = 0;
+ u32 checksum, iov_count, padding = 0, rx_got = 0, rx_size = 0;
u32 payload_length = ntoh24(hdr->dlength);
- int iov_ret, data_crc_failed = 0;
+ int data_crc_failed = 0;
rx_size += payload_length;
- iov = &cmd->iov_data[0];
- iov_ret = iscsit_map_iovec(cmd, iov, be32_to_cpu(hdr->offset),
+ iov_count = iscsit_map_iovec(cmd, 0, be32_to_cpu(hdr->offset),
payload_length);
- if (iov_ret < 0)
- return -1;
-
- iov_count += iov_ret;
padding = ((-payload_length) & 3);
if (padding != 0) {
- iov[iov_count].iov_base = cmd->pad_bytes;
- iov[iov_count++].iov_len = padding;
+ iscsit_map_data_iovec(cmd, iov_count++, cmd->pad_bytes, padding);
rx_size += padding;
pr_debug("Receiving %u padding bytes.\n", padding);
}
if (conn->conn_ops->DataDigest) {
- iov[iov_count].iov_base = &checksum;
- iov[iov_count++].iov_len = ISCSI_CRC_LEN;
+ iscsit_map_data_iovec(cmd, iov_count++, &checksum, ISCSI_CRC_LEN);
rx_size += ISCSI_CRC_LEN;
}
@@ -1648,7 +1650,6 @@ static int iscsit_handle_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
{
unsigned char *ping_data = NULL;
struct iscsi_nopout *hdr = (struct iscsi_nopout *)buf;
- struct kvec *iov = NULL;
u32 payload_length = ntoh24(hdr->dlength);
int ret;
@@ -1670,21 +1671,17 @@ static int iscsit_handle_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
goto out;
}
- iov = &cmd->iov_misc[0];
- iov[niov].iov_base = ping_data;
- iov[niov++].iov_len = payload_length;
+ iscsit_map_misc_iovec(cmd, niov++, ping_data, payload_length);
padding = ((-payload_length) & 3);
if (padding != 0) {
pr_debug("Receiving %u additional bytes"
" for padding.\n", padding);
- iov[niov].iov_base = &cmd->pad_bytes;
- iov[niov++].iov_len = padding;
+ iscsit_map_misc_iovec(cmd, niov++, &cmd->pad_bytes, padding);
rx_size += padding;
}
if (conn->conn_ops->DataDigest) {
- iov[niov].iov_base = &checksum;
- iov[niov++].iov_len = ISCSI_CRC_LEN;
+ iscsit_map_misc_iovec(cmd, niov++, &checksum, ISCSI_CRC_LEN);
rx_size += ISCSI_CRC_LEN;
}
@@ -2411,29 +2408,21 @@ static int iscsit_handle_immediate_data(
struct iscsi_scsi_req *hdr,
u32 length)
{
- int iov_ret, rx_got = 0, rx_size = 0;
- u32 checksum, iov_count = 0, padding = 0;
+ int rx_got = 0, rx_size = 0;
+ u32 checksum, iov_count, padding = 0;
struct iscsi_conn *conn = cmd->conn;
- struct kvec *iov;
- iov_ret = iscsit_map_iovec(cmd, cmd->iov_data, cmd->write_data_done, length);
- if (iov_ret < 0)
- return IMMEDIATE_DATA_CANNOT_RECOVER;
+ iov_count = iscsit_map_iovec(cmd, 0, cmd->write_data_done, length);
rx_size = length;
- iov_count = iov_ret;
- iov = &cmd->iov_data[0];
-
padding = ((-length) & 3);
if (padding != 0) {
- iov[iov_count].iov_base = cmd->pad_bytes;
- iov[iov_count++].iov_len = padding;
+ iscsit_map_data_iovec(cmd, iov_count++, cmd->pad_bytes, padding);
rx_size += padding;
}
if (conn->conn_ops->DataDigest) {
- iov[iov_count].iov_base = &checksum;
- iov[iov_count++].iov_len = ISCSI_CRC_LEN;
+ iscsit_map_data_iovec(cmd, iov_count++, &checksum, ISCSI_CRC_LEN);
rx_size += ISCSI_CRC_LEN;
}
@@ -2640,9 +2629,8 @@ static int iscsit_send_datain(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
struct iscsi_data_rsp *hdr = (struct iscsi_data_rsp *)&cmd->pdu[0];
struct iscsi_datain datain;
struct iscsi_datain_req *dr;
- struct kvec *iov;
u32 iov_count = 0, tx_size = 0;
- int eodr = 0, ret, iov_ret;
+ int eodr = 0, ret;
bool set_statsn = false;
memset(&datain, 0, sizeof(struct iscsi_datain));
@@ -2684,9 +2672,7 @@ static int iscsit_send_datain(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
iscsit_build_datain_pdu(cmd, conn, &datain, hdr, set_statsn);
- iov = &cmd->iov_data[0];
- iov[iov_count].iov_base = cmd->pdu;
- iov[iov_count++].iov_len = ISCSI_HDR_LEN;
+ iscsit_map_data_iovec(cmd, iov_count++, cmd->pdu, ISCSI_HDR_LEN);
tx_size += ISCSI_HDR_LEN;
if (conn->conn_ops->HeaderDigest) {
@@ -2695,25 +2681,19 @@ static int iscsit_send_datain(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
iscsit_do_crypto_hash_buf(&conn->conn_tx_hash, cmd->pdu,
ISCSI_HDR_LEN, 0, NULL, (u8 *)header_digest);
- iov[0].iov_len += ISCSI_CRC_LEN;
+ cmd->iov_data[0].iov_len += ISCSI_CRC_LEN;
tx_size += ISCSI_CRC_LEN;
pr_debug("Attaching CRC32 HeaderDigest"
" for DataIN PDU 0x%08x\n", *header_digest);
}
- iov_ret = iscsit_map_iovec(cmd, &cmd->iov_data[1],
- datain.offset, datain.length);
- if (iov_ret < 0)
- return -1;
-
- iov_count += iov_ret;
+ iov_count += iscsit_map_iovec(cmd, 1, datain.offset, datain.length);
tx_size += datain.length;
cmd->padding = ((-datain.length) & 3);
if (cmd->padding) {
- iov[iov_count].iov_base = cmd->pad_bytes;
- iov[iov_count++].iov_len = cmd->padding;
+ iscsit_map_data_iovec(cmd, iov_count++, cmd->pad_bytes, cmd->padding);
tx_size += cmd->padding;
pr_debug("Attaching %u padding bytes\n",
@@ -2723,8 +2703,7 @@ static int iscsit_send_datain(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
cmd->data_crc = iscsit_do_crypto_hash_sg(&conn->conn_tx_hash, cmd,
datain.offset, datain.length, cmd->padding, cmd->pad_bytes);
- iov[iov_count].iov_base = &cmd->data_crc;
- iov[iov_count++].iov_len = ISCSI_CRC_LEN;
+ iscsit_map_data_iovec(cmd, iov_count++, &cmd->data_crc, ISCSI_CRC_LEN);
tx_size += ISCSI_CRC_LEN;
pr_debug("Attached CRC32C DataDigest %d bytes, crc"
@@ -2854,7 +2833,6 @@ EXPORT_SYMBOL(iscsit_build_logout_rsp);
static int
iscsit_send_logout(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
{
- struct kvec *iov;
int niov = 0, tx_size, rc;
rc = iscsit_build_logout_rsp(cmd, conn,
@@ -2863,9 +2841,7 @@ iscsit_send_logout(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
return rc;
tx_size = ISCSI_HDR_LEN;
- iov = &cmd->iov_misc[0];
- iov[niov].iov_base = cmd->pdu;
- iov[niov++].iov_len = ISCSI_HDR_LEN;
+ iscsit_map_misc_iovec(cmd, niov++, cmd->pdu, ISCSI_HDR_LEN);
if (conn->conn_ops->HeaderDigest) {
u32 *header_digest = (u32 *)&cmd->pdu[ISCSI_HDR_LEN];
@@ -2873,7 +2849,7 @@ iscsit_send_logout(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
iscsit_do_crypto_hash_buf(&conn->conn_tx_hash, &cmd->pdu[0],
ISCSI_HDR_LEN, 0, NULL, (u8 *)header_digest);
- iov[0].iov_len += ISCSI_CRC_LEN;
+ cmd->iov_misc[0].iov_len += ISCSI_CRC_LEN;
tx_size += ISCSI_CRC_LEN;
pr_debug("Attaching CRC32C HeaderDigest to"
" Logout Response 0x%08x\n", *header_digest);
@@ -2962,16 +2938,13 @@ static int
iscsit_send_nopin(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
{
struct iscsi_nopin *hdr = (struct iscsi_nopin *)&cmd->pdu[0];
- struct kvec *iov;
u32 padding = 0;
int niov = 0, tx_size;
iscsit_build_nopin_rsp(cmd, conn, hdr, true);
tx_size = ISCSI_HDR_LEN;
- iov = &cmd->iov_misc[0];
- iov[niov].iov_base = cmd->pdu;
- iov[niov++].iov_len = ISCSI_HDR_LEN;
+ iscsit_map_misc_iovec(cmd, niov++, cmd->pdu, ISCSI_HDR_LEN);
if (conn->conn_ops->HeaderDigest) {
u32 *header_digest = (u32 *)&cmd->pdu[ISCSI_HDR_LEN];
@@ -2979,7 +2952,7 @@ iscsit_send_nopin(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
iscsit_do_crypto_hash_buf(&conn->conn_tx_hash, hdr,
ISCSI_HDR_LEN, 0, NULL, (u8 *)header_digest);
- iov[0].iov_len += ISCSI_CRC_LEN;
+ cmd->iov_misc[0].iov_len += ISCSI_CRC_LEN;
tx_size += ISCSI_CRC_LEN;
pr_debug("Attaching CRC32C HeaderDigest"
" to NopIn 0x%08x\n", *header_digest);
@@ -2990,8 +2963,7 @@ iscsit_send_nopin(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
* NOPOUT DataSegmentLength is at struct iscsi_cmd->buf_ptr_size.
*/
if (cmd->buf_ptr_size) {
- iov[niov].iov_base = cmd->buf_ptr;
- iov[niov++].iov_len = cmd->buf_ptr_size;
+ iscsit_map_misc_iovec(cmd, niov++, cmd->buf_ptr, cmd->buf_ptr_size);
tx_size += cmd->buf_ptr_size;
pr_debug("Echoing back %u bytes of ping"
@@ -2999,8 +2971,7 @@ iscsit_send_nopin(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
padding = ((-cmd->buf_ptr_size) & 3);
if (padding != 0) {
- iov[niov].iov_base = &cmd->pad_bytes;
- iov[niov++].iov_len = padding;
+ iscsit_map_misc_iovec(cmd, niov++, &cmd->pad_bytes, padding);
tx_size += padding;
pr_debug("Attaching %u additional"
" padding bytes.\n", padding);
@@ -3011,8 +2982,7 @@ iscsit_send_nopin(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
padding, (u8 *)&cmd->pad_bytes,
(u8 *)&cmd->data_crc);
- iov[niov].iov_base = &cmd->data_crc;
- iov[niov++].iov_len = ISCSI_CRC_LEN;
+ iscsit_map_misc_iovec(cmd, niov++, &cmd->data_crc, ISCSI_CRC_LEN);
tx_size += ISCSI_CRC_LEN;
pr_debug("Attached DataDigest for %u"
" bytes of ping data, CRC 0x%08x\n",
@@ -3219,16 +3189,13 @@ EXPORT_SYMBOL(iscsit_build_rsp_pdu);
static int iscsit_send_response(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
{
struct iscsi_scsi_rsp *hdr = (struct iscsi_scsi_rsp *)&cmd->pdu[0];
- struct kvec *iov;
u32 padding = 0, tx_size = 0;
- int iov_count = 0;
+ int niov = 0;
bool inc_stat_sn = (cmd->i_state == ISTATE_SEND_STATUS);
iscsit_build_rsp_pdu(cmd, conn, inc_stat_sn, hdr);
- iov = &cmd->iov_misc[0];
- iov[iov_count].iov_base = cmd->pdu;
- iov[iov_count++].iov_len = ISCSI_HDR_LEN;
+ iscsit_map_misc_iovec(cmd, niov++, cmd->pdu, ISCSI_HDR_LEN);
tx_size += ISCSI_HDR_LEN;
/*
@@ -3242,9 +3209,8 @@ static int iscsit_send_response(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
padding = -(cmd->se_cmd.scsi_sense_length) & 3;
hton24(hdr->dlength, (u32)cmd->se_cmd.scsi_sense_length);
- iov[iov_count].iov_base = cmd->sense_buffer;
- iov[iov_count++].iov_len =
- (cmd->se_cmd.scsi_sense_length + padding);
+ iscsit_map_misc_iovec(cmd, niov++, cmd->sense_buffer,
+ cmd->se_cmd.scsi_sense_length + padding);
tx_size += cmd->se_cmd.scsi_sense_length;
if (padding) {
@@ -3261,8 +3227,7 @@ static int iscsit_send_response(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
(cmd->se_cmd.scsi_sense_length + padding),
0, NULL, (u8 *)&cmd->data_crc);
- iov[iov_count].iov_base = &cmd->data_crc;
- iov[iov_count++].iov_len = ISCSI_CRC_LEN;
+ iscsit_map_misc_iovec(cmd, niov++, &cmd->data_crc, ISCSI_CRC_LEN);
tx_size += ISCSI_CRC_LEN;
pr_debug("Attaching CRC32 DataDigest for"
@@ -3282,13 +3247,13 @@ static int iscsit_send_response(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
iscsit_do_crypto_hash_buf(&conn->conn_tx_hash, cmd->pdu,
ISCSI_HDR_LEN, 0, NULL, (u8 *)header_digest);
- iov[0].iov_len += ISCSI_CRC_LEN;
+ cmd->iov_misc[0].iov_len += ISCSI_CRC_LEN;
tx_size += ISCSI_CRC_LEN;
pr_debug("Attaching CRC32 HeaderDigest for Response"
" PDU 0x%08x\n", *header_digest);
}
- cmd->iov_misc_count = iov_count;
+ cmd->iov_misc_count = niov;
cmd->tx_size = tx_size;
return 0;
@@ -3562,20 +3527,16 @@ static int iscsit_send_text_rsp(
struct iscsi_conn *conn)
{
struct iscsi_text_rsp *hdr = (struct iscsi_text_rsp *)cmd->pdu;
- struct kvec *iov;
u32 tx_size = 0;
- int text_length, iov_count = 0, rc;
+ int text_length, niov = 0, rc;
rc = iscsit_build_text_rsp(cmd, conn, hdr, ISCSI_TCP);
if (rc < 0)
return rc;
text_length = rc;
- iov = &cmd->iov_misc[0];
- iov[iov_count].iov_base = cmd->pdu;
- iov[iov_count++].iov_len = ISCSI_HDR_LEN;
- iov[iov_count].iov_base = cmd->buf_ptr;
- iov[iov_count++].iov_len = text_length;
+ iscsit_map_misc_iovec(cmd, niov++, cmd->pdu, ISCSI_HDR_LEN);
+ iscsit_map_misc_iovec(cmd, niov++, cmd->buf_ptr, text_length);
tx_size += (ISCSI_HDR_LEN + text_length);
@@ -3585,7 +3546,7 @@ static int iscsit_send_text_rsp(
iscsit_do_crypto_hash_buf(&conn->conn_tx_hash, hdr,
ISCSI_HDR_LEN, 0, NULL, (u8 *)header_digest);
- iov[0].iov_len += ISCSI_CRC_LEN;
+ cmd->iov_misc[0].iov_len += ISCSI_CRC_LEN;
tx_size += ISCSI_CRC_LEN;
pr_debug("Attaching CRC32 HeaderDigest for"
" Text Response PDU 0x%08x\n", *header_digest);
@@ -3596,8 +3557,7 @@ static int iscsit_send_text_rsp(
cmd->buf_ptr, text_length,
0, NULL, (u8 *)&cmd->data_crc);
- iov[iov_count].iov_base = &cmd->data_crc;
- iov[iov_count++].iov_len = ISCSI_CRC_LEN;
+ iscsit_map_misc_iovec(cmd, niov++, &cmd->data_crc, ISCSI_CRC_LEN);
tx_size += ISCSI_CRC_LEN;
pr_debug("Attaching DataDigest for %u bytes of text"
@@ -3605,7 +3565,7 @@ static int iscsit_send_text_rsp(
cmd->data_crc);
}
- cmd->iov_misc_count = iov_count;
+ cmd->iov_misc_count = niov;
cmd->tx_size = tx_size;
return 0;
@@ -3633,16 +3593,12 @@ static int iscsit_send_reject(
struct iscsi_conn *conn)
{
struct iscsi_reject *hdr = (struct iscsi_reject *)&cmd->pdu[0];
- struct kvec *iov;
- u32 iov_count = 0, tx_size;
+ u32 niov = 0, tx_size;
iscsit_build_reject(cmd, conn, hdr);
- iov = &cmd->iov_misc[0];
- iov[iov_count].iov_base = cmd->pdu;
- iov[iov_count++].iov_len = ISCSI_HDR_LEN;
- iov[iov_count].iov_base = cmd->buf_ptr;
- iov[iov_count++].iov_len = ISCSI_HDR_LEN;
+ iscsit_map_misc_iovec(cmd, niov++, cmd->pdu, ISCSI_HDR_LEN);
+ iscsit_map_misc_iovec(cmd, niov++, cmd->buf_ptr, ISCSI_HDR_LEN);
tx_size = (ISCSI_HDR_LEN + ISCSI_HDR_LEN);
@@ -3652,7 +3608,7 @@ static int iscsit_send_reject(
iscsit_do_crypto_hash_buf(&conn->conn_tx_hash, hdr,
ISCSI_HDR_LEN, 0, NULL, (u8 *)header_digest);
- iov[0].iov_len += ISCSI_CRC_LEN;
+ cmd->iov_misc[0].iov_len += ISCSI_CRC_LEN;
tx_size += ISCSI_CRC_LEN;
pr_debug("Attaching CRC32 HeaderDigest for"
" REJECT PDU 0x%08x\n", *header_digest);
@@ -3662,14 +3618,13 @@ static int iscsit_send_reject(
iscsit_do_crypto_hash_buf(&conn->conn_tx_hash, cmd->buf_ptr,
ISCSI_HDR_LEN, 0, NULL, (u8 *)&cmd->data_crc);
- iov[iov_count].iov_base = &cmd->data_crc;
- iov[iov_count++].iov_len = ISCSI_CRC_LEN;
+ iscsit_map_misc_iovec(cmd, niov++, &cmd->data_crc, ISCSI_CRC_LEN);
tx_size += ISCSI_CRC_LEN;
pr_debug("Attaching CRC32 DataDigest for REJECT"
" PDU 0x%08x\n", cmd->data_crc);
}
- cmd->iov_misc_count = iov_count;
+ cmd->iov_misc_count = niov;
cmd->tx_size = tx_size;
pr_debug("Built Reject PDU StatSN: 0x%08x, Reason: 0x%02x,"
--
2.0.0.rc0.1.g7b2ba98
--
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
