[stgt] [PATCH] Another patch to improve the LBA out of range even further

[ronnie sahlberg]

On Sun, Jun 17, 2012 at 8:53 AM, FUJITA Tomonori
<fujita.tomonori at lab.ntt.co.jp> wrote:
> On Mon, 4 Jun 2012 19:10:12 +1000
> ronnie sahlberg <ronniesahlberg at gmail.com> wrote:
>
>> From efc1f1720ad9a00e69f7935a001df906e609afe5 Mon Sep 17 00:00:00 2001
>> From: Ronnie Sahlberg <ronniesahlberg at gmail.com>
>> Date: Mon, 4 Jun 2012 18:56:18 +1000
>> Subject: [PATCH] SBC: LBA range check, fix some bugs in the LBA out of range check
>>
>> We can not shift the LBA << 9   and compare to the file size since this means
>> that for a HUGE LBA, like LBA==2^63  this will cause the 64 bit integer
>> to overflow and we suddenly pass all the tests and LBA sudddenly becomes LBA 0 instead.
>> Several targets have this bug as far as I can tell in testing.
>>
>> Instead, use LBA as is and instead shift the filesize >> 9  before the check
>> to avoid this overflow.
>>
>> Also, if both LBA and tranfser length are huge,  LBA + TL can wrap too
>> so we need to check for that too and return check condition if
>> lba+tl < lba
>>
>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg at gmail.com>
>> ---
>>  usr/sbc.c |   13 +++++++------
>>  1 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/usr/sbc.c b/usr/sbc.c
>> index 248a547..bc9e3d6 100644
>> --- a/usr/sbc.c
>> +++ b/usr/sbc.c
>> @@ -297,27 +297,28 @@ static int sbc_rw(int host_no, struct scsi_cmd *cmd)
>>               }
>>       }
>>
>> -     lba = scsi_rw_offset(cmd->scb) << cmd->dev->blk_shift;
>> -     tl  = scsi_rw_count(cmd->scb) << cmd->dev->blk_shift;
>> +     lba = scsi_rw_offset(cmd->scb);
>> +     tl  = scsi_rw_count(cmd->scb);
>>
>>       /* Verify that we are not doing i/o beyond
>>          the end-of-lun */
>>       if (tl) {
>> -             if (lba + tl > lu->size) {
>> +             if ((lba + tl < lba)
>
> How this could happen?

For example if LBA == 0xffffffffffffffff and TL == 2,   then LBA+TL == 1


My iscsi test tool has this as the third subtest in :
0204_read16_beyond_eol

                printf("Test that READ16 fails if reading beyond
end-of-lun.\n");
                printf("1, Read 1-256 blocks one block beyond end-of-lun.\n");
                printf("2, Read 1-256 blocks at LBA 2^63\n");
                printf("3, Read 1-256 blocks at LBA -1\n");
                printf("\n");
                return 0;




>
>> +             || (lba + tl > (lu->size >> cmd->dev->blk_shift))) {
>>                       key = ILLEGAL_REQUEST;
>>                       asc = ASC_LBA_OUT_OF_RANGE;
>>                       goto sense;
>>               }
>>       } else {
>> -             if (lba >= lu->size) {
>> +             if (lba >= (lu->size >> cmd->dev->blk_shift)) {
>>                       key = ILLEGAL_REQUEST;
>>                       asc = ASC_LBA_OUT_OF_RANGE;
>>                       goto sense;
>>               }
>>       }
>>
>> -     cmd->offset = lba;
>> -     cmd->tl     = tl;
>> +     cmd->offset = lba << cmd->dev->blk_shift;
>> +     cmd->tl     = tl  << cmd->dev->blk_shift;
>>
>>       ret = cmd->dev->bst->bs_cmd_submit(cmd);
>>       if (ret) {
>> --
>> 1.7.3.1
>>
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html