WRITE with large data buffer

[Dirk Jagdmann]

In the iscsi_scsi_command_async() function, if a write wants to send a data buffer that is larger than the length allowed by iscsi->first_burst_length there are two problems in the code flow. See the following diffs (and allow for likely messed up characters, I don't trust this Google Groups web editor to preserve the diff format correctly)

--- a/lib/iscsi-command.c
+++ b/lib/iscsi-command.c
@@ -286,8 +296,7 @@ iscsi_scsi_command_async(struct iscsi_context *iscsi, int lun,
                 * of data-out further below.
                 */
                if (iscsi->use_initial_r2t == ISCSI_INITIAL_R2T_NO
-                   && pdu->payload_len < (uint32_t)task->expxferlen
-                   && pdu->payload_len < iscsi->first_burst_length) {
+                   && pdu->payload_len < (uint32_t)task->expxferlen) {
                        /* We have more data to send, and we are allowed to send
                         * unsolicited data, so don't flag this PDU as final.
                         */

pdu->payload_len was previously clamped to a maximum of iscsi->first_burst_length. Thus the expression "pdu->payload_len < iscsi->first_burst_length" can never be true, as payload_len will always be equal to first_burst_length. Thus the if block is never executed and the libiscsi code will never add any Data-In PDUs. With that comparison removed from the if expression the code will clear that ISCSI_PDU_SCSI_FINAL correctly.

But then the code that follows has another problem, see the following diff:

--- a/lib/iscsi-command.c
+++ b/lib/iscsi-command.c
@@ -192,7 +196,13 @@ iscsi_timeout_scan(struct iscsi_context *iscsi)
 static int
 iscsi_send_unsolicited_data_out(struct iscsi_context *iscsi, struct iscsi_pdu *pdu)
 {
-       uint32_t len = MIN(pdu->expxferlen, iscsi->first_burst_length) - pdu->payload_len;
+       uint32_t len;
+
+       if (pdu->payload_len >= pdu->expxferlen) {
+               return 0;
+       }
+
+       len = pdu->expxferlen - pdu->payload_len;

        return iscsi_send_data_out(iscsi, pdu, 0xffffffff,
                                   pdu->payload_len, len);

The original code will always calculate len as 0, since the result of the "MIN(pdu->expxferlen, iscsi->first_burst_length)" will always be first_burst_length and pdu->payload_len will also be calculated as first_burst_length by the code that called the  iscsi_send_unsolicited_data_out() function. What the function really wants to calculate is the remaining length of data, that is not already included in the CDB PDU. My changes calculate this correctly and also check if the PDU lengths make sense. Maybe this function should return -1 if payload_len is greater than expxferlen though?

Do I understand this situation correctly and do my changes make sense?