[PATCH 4.9 02/69] tcmu: Fix possible overwrite of t_data_sgs last iov[]

[Greg Kroah-Hartman]

4.9-stable review patch.  If anyone has any objections, please let me know.

--

From: Xiubo Li 

commit ab22d2604c86ceb01bb2725c9860b88a7dd383bb upstream.

If there has BIDI data, its first iov[] will overwrite the last
iov[] for se_cmd->t_data_sg.

To fix this, we can just increase the iov pointer, but this may
introuduce a new memory leakage bug: If the se_cmd->data_length
and se_cmd->t_bidi_data_sg->length are all not aligned up to the
DATA_BLOCK_SIZE, the actual length needed maybe larger than just
sum of them.

So, this could be avoided by rounding all the data lengthes up
to DATA_BLOCK_SIZE.

Reviewed-by: Mike Christie 
Tested-by: Ilias Tsitsimpis 
Reviewed-by: Bryant G. Ly 
Signed-off-by: Xiubo Li 
Signed-off-by: Nicholas Bellinger 
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/target/target_core_user.c |   34 +++---
 1 file changed, 23 insertions(+), 11 deletions(-)

--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -389,6 +389,20 @@ static bool is_ring_space_avail(struct t
return true;
 }
 
+static inline size_t tcmu_cmd_get_data_length(struct tcmu_cmd *tcmu_cmd)
+{
+   struct se_cmd *se_cmd = tcmu_cmd->se_cmd;
+   size_t data_length = round_up(se_cmd->data_length, DATA_BLOCK_SIZE);
+
+   if (se_cmd->se_cmd_flags & SCF_BIDI) {
+   BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
+   data_length += round_up(se_cmd->t_bidi_data_sg->length,
+   DATA_BLOCK_SIZE);
+   }
+
+   return data_length;
+}
+
 static sense_reason_t
 tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd)
 {
@@ -402,7 +416,7 @@ tcmu_queue_cmd_ring(struct tcmu_cmd *tcm
uint32_t cmd_head;
uint64_t cdb_off;
bool copy_to_data_area;
-   size_t data_length;
+   size_t data_length = tcmu_cmd_get_data_length(tcmu_cmd);
DECLARE_BITMAP(old_bitmap, DATA_BLOCK_BITS);
 
if (test_bit(TCMU_DEV_BIT_BROKEN, >flags))
@@ -428,11 +442,6 @@ tcmu_queue_cmd_ring(struct tcmu_cmd *tcm
 
mb = udev->mb_addr;
cmd_head = mb->cmd_head % udev->cmdr_size; /* UAM */
-   data_length = se_cmd->data_length;
-   if (se_cmd->se_cmd_flags & SCF_BIDI) {
-   BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
-   data_length += se_cmd->t_bidi_data_sg->length;
-   }
if ((command_size > (udev->cmdr_size / 2)) ||
data_length > udev->data_size) {
pr_warn("TCMU: Request of size %zu/%zu is too big for %u/%zu "
@@ -502,11 +511,14 @@ tcmu_queue_cmd_ring(struct tcmu_cmd *tcm
entry->req.iov_dif_cnt = 0;
 
/* Handle BIDI commands */
-   iov_cnt = 0;
-   alloc_and_scatter_data_area(udev, se_cmd->t_bidi_data_sg,
-   se_cmd->t_bidi_data_nents, , _cnt, false);
-   entry->req.iov_bidi_cnt = iov_cnt;
-
+   if (se_cmd->se_cmd_flags & SCF_BIDI) {
+   iov_cnt = 0;
+   iov++;
+   alloc_and_scatter_data_area(udev, se_cmd->t_bidi_data_sg,
+   se_cmd->t_bidi_data_nents, , _cnt,
+   false);
+   entry->req.iov_bidi_cnt = iov_cnt;
+   }
/* cmd's data_bitmap is what changed in process */
bitmap_xor(tcmu_cmd->data_bitmap, old_bitmap, udev->data_bitmap,
DATA_BLOCK_BITS);

[PATCH 4.9 02/69] tcmu: Fix possible overwrite of t_data_sgs last iov[]

[Greg Kroah-Hartman]

4.9-stable review patch.  If anyone has any objections, please let me know.

--

From: Xiubo Li 

commit ab22d2604c86ceb01bb2725c9860b88a7dd383bb upstream.

If there has BIDI data, its first iov[] will overwrite the last
iov[] for se_cmd->t_data_sg.

To fix this, we can just increase the iov pointer, but this may
introuduce a new memory leakage bug: If the se_cmd->data_length
and se_cmd->t_bidi_data_sg->length are all not aligned up to the
DATA_BLOCK_SIZE, the actual length needed maybe larger than just
sum of them.

So, this could be avoided by rounding all the data lengthes up
to DATA_BLOCK_SIZE.

Reviewed-by: Mike Christie 
Tested-by: Ilias Tsitsimpis 
Reviewed-by: Bryant G. Ly 
Signed-off-by: Xiubo Li 
Signed-off-by: Nicholas Bellinger 
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/target/target_core_user.c |   34 +++---
 1 file changed, 23 insertions(+), 11 deletions(-)

--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -389,6 +389,20 @@ static bool is_ring_space_avail(struct t
return true;
 }
 
+static inline size_t tcmu_cmd_get_data_length(struct tcmu_cmd *tcmu_cmd)
+{
+   struct se_cmd *se_cmd = tcmu_cmd->se_cmd;
+   size_t data_length = round_up(se_cmd->data_length, DATA_BLOCK_SIZE);
+
+   if (se_cmd->se_cmd_flags & SCF_BIDI) {
+   BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
+   data_length += round_up(se_cmd->t_bidi_data_sg->length,
+   DATA_BLOCK_SIZE);
+   }
+
+   return data_length;
+}
+
 static sense_reason_t
 tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd)
 {
@@ -402,7 +416,7 @@ tcmu_queue_cmd_ring(struct tcmu_cmd *tcm
uint32_t cmd_head;
uint64_t cdb_off;
bool copy_to_data_area;
-   size_t data_length;
+   size_t data_length = tcmu_cmd_get_data_length(tcmu_cmd);
DECLARE_BITMAP(old_bitmap, DATA_BLOCK_BITS);
 
if (test_bit(TCMU_DEV_BIT_BROKEN, >flags))
@@ -428,11 +442,6 @@ tcmu_queue_cmd_ring(struct tcmu_cmd *tcm
 
mb = udev->mb_addr;
cmd_head = mb->cmd_head % udev->cmdr_size; /* UAM */
-   data_length = se_cmd->data_length;
-   if (se_cmd->se_cmd_flags & SCF_BIDI) {
-   BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
-   data_length += se_cmd->t_bidi_data_sg->length;
-   }
if ((command_size > (udev->cmdr_size / 2)) ||
data_length > udev->data_size) {
pr_warn("TCMU: Request of size %zu/%zu is too big for %u/%zu "
@@ -502,11 +511,14 @@ tcmu_queue_cmd_ring(struct tcmu_cmd *tcm
entry->req.iov_dif_cnt = 0;
 
/* Handle BIDI commands */
-   iov_cnt = 0;
-   alloc_and_scatter_data_area(udev, se_cmd->t_bidi_data_sg,
-   se_cmd->t_bidi_data_nents, , _cnt, false);
-   entry->req.iov_bidi_cnt = iov_cnt;
-
+   if (se_cmd->se_cmd_flags & SCF_BIDI) {
+   iov_cnt = 0;
+   iov++;
+   alloc_and_scatter_data_area(udev, se_cmd->t_bidi_data_sg,
+   se_cmd->t_bidi_data_nents, , _cnt,
+   false);
+   entry->req.iov_bidi_cnt = iov_cnt;
+   }
/* cmd's data_bitmap is what changed in process */
bitmap_xor(tcmu_cmd->data_bitmap, old_bitmap, udev->data_bitmap,
DATA_BLOCK_BITS);