Re: SNACK-R2T - RunLength > 0 Crashing Target

Arshad Hussain <arshad.hussain@xxxxxxxxxxxxxx> · Thu, 23 Jan 2014 15:14:47 +0530

Hi Nicholas,

On 1/23/2014 12:37 AM, Nicholas A. Bellinger wrote:
Hi Arshad,

On Wed, 2014-01-22 at 11:10 +0530, Arshad Hussain wrote:
On 1/16/2014 3:43 PM, Arshad Hussain wrote:
Hi,

I want to report an error in target. I am trying to confirm SNACK-R2T
that the target properly responds to a SNACK request.  Whenever I
am putting runLength>0 it is crashing the target (machine reboot).

Sequence of PDU is like below.

I->T SCSI WRITE Command for 512B // Send out write
T->I R2T // Got R2T.
I->T SNACK BegRun=0 RunLength=1  // This crashes the Target.
/*
Send out SNACK
BegRun = First PDU whose retransmission is requested. (r2tsn)
RunLength = Number of PDU whose retransmission is requested.
*/

However, if the begrun and runlength is modified to 0 (Send all) it
works.

I->T SCSI WRITE Command for 512B // Send out write
T->I R2T // Got R2T.
I->T SNACK BegRun=0 RunLength=0 // Works correctly.

My Machine Details.

[root@localhost test_suite_iscsi]# cat /sys/kernel/config/target/version
Target Engine Core ConfigFS Infrastructure v4.1.0-rc2-ml on
Linux/x86_64 on 3.10.13-101.fc18.x86_64
[root@localhost test_suite_iscsi]#

[root@localhost test_suite_iscsi]# uname -a
Linux localhost.localdomain 3.10.13-101.fc18.x86_64 #1 SMP Fri Sep 27
20:22:12 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
[root@localhost test_suite_iscsi]#

Thanks,
Arshad

Hi,

I Regret the slow turnaround.

What I got is, if I do not convert from host to network byte order
and assign BegRun and RunLength values greater than 0. ( of course 0 is
same for both endian ) it crashes the machine.

This was of course error on my side. However should this have crashed
the target.?  Shoud not this have returned an error and stopped.
Your comments please.

Looking at the code in iscsi_target_erl1.c:iscsit_handle_r2t_snack(), it
is checking the validity of received BegRun + RunLength values, in that
they don't exceed what R2TSN has already been put on the wire:

        if (runlength) {
                 if ((begrun + runlength) > cmd->r2t_sn) {
                         pr_err("Command ITT: 0x%08x received R2T SNACK"
                         " with BegRun: 0x%08x, RunLength: 0x%08x, exceeds"
                         " current R2TSN: 0x%08x, protocol error.\n",
                         cmd->init_task_tag, begrun, runlength, cmd->r2t_sn);
                         return iscsit_reject_cmd(cmd,
                                         ISCSI_REASON_BOOKMARK_INVALID, buf);
                 }
                 last_r2tsn = (begrun + runlength);
         } else
                 last_r2tsn = cmd->r2t_sn;

Can you confirm your bug is with R2T SNACKs, and not a different type of
SNACK..?
Yes I confirm it is with R2T SNACK.  My two cents. Without htonl() 
conversion the
value 0x1 (intel) is getting converted into large value 0x10000000 on 
assignment.
Which in turn is getting assigned to last_r2tsn. And further down your 
code...

while (begrun < last_r2tsn) {

... // it may be looping in uncontrolled manner and failing.
begrun++
}

Also, a wireshark dump and/or access to the test that is generating this
particular scenario would be extremely helpful for tracking down the
bug.

Thanks!

--nab

If I populate SNACK with these below values. This passes. (pcap file = 
R2T_snack_error1)

//iscsi_snack_request
snack->opcode = 0x10;
snack->flags = 0x80;
snack->beg_run = 0;
snack->run_length = htonl(1); // Note the conversion is being done from 
host to network order.

If I populate SNACK with these below values, this crashes the 
target/machine (pcap file = R2T_snack_error2)
//iscsi_snack_request
snack->opcode = 0x10;
snack->flags = 0x80;
snack->beg_run = 0;
snack->run_length = 1; //Note, no conversion from host to network here. 
(Machine Crash.)

Thanks,
Arshad

PS: Attached pcap for both runs.
Attachment:
R2T_snack_error1

Description: Binary data
Attachment:
R2T_snack_error2

Description: Binary data