[Stgt-devel] Stability problem

[ronnie sahlberg]

Hi,

Im not Konrad but I tried your
> http://stgt.berlios.de/releases/tgt-20080526-debug.tar.bz2
and tgtd does not crash on my system any more when I send a manually
"probe how big the page actually is"
mode sense 10 to tgtd any more.


I have positively verified with a specially constructed tool that tgtd
no longer crashes for these mode sense commands.


please apply your patch.


ronnie sahlberg


On Mon, May 26, 2008 at 10:36 PM, FUJITA Tomonori
<fujita.tomonori at lab.ntt.co.jp> wrote:
> On Thu, 15 May 2008 15:37:02 +0200
> Konrad Bechler <kbechler at bakertillypoland.eu> wrote:
>
>> Hello,
>>
>> I've got small, testing environment. I use Fedora 7 with
>> 2.6.23.15-80.fc7 as a iSCSI target and Windows XP box with "Microsoft
>> iSCSI Initiator" as a initiator.
>> I configured stgt with only one target and one LUN:
>> Target 1: iqn.2008.05.eu.tgc.net:storage.disk1
>>      System information:
>>          Driver: iscsi
>>          State: ready
>>      I_T nexus information:
>>      LUN information:
>>          LUN: 0
>>              Type: controller
>>              SCSI ID: deadbeaf1:0
>>              SCSI SN: beaf10
>>              Size: 0 MB
>>              Online: Yes
>>              Removable media: No
>>              Backing store: No backing store
>>          LUN: 1
>>              Type: disk
>>              SCSI ID: deadbeaf1:1
>>              SCSI SN: beaf11
>>              Size: 10486 MB
>>              Online: Yes
>>              Removable media: No
>>              Backing store: /data/disk1
>>      Account information:
>>      ACL information:
>>          ALL
>>
>>
>> I can connect to this target. But when trying to format such drive, I've
>> got message, that formatting exited with error. When "quick format" box
>> is checked, tgtd aborts.
>
> Last week, Konrad helped me to figure out what's the problem. Seems
> that the MODE_SENSE data corruption leads to this, which Ronnie
> reported in another thread.
>
> Konrad, can you try the following version?
>
> http://stgt.berlios.de/releases/tgt-20080526-debug.tar.bz2
>
>
> I've also attached a patch against the lasest git head.
>
> Thanks,
>
> diff --git a/usr/spc.c b/usr/spc.c
> index 65a8e62..b3d7162 100644
> --- a/usr/spc.c
> +++ b/usr/spc.c
> @@ -335,21 +335,25 @@ int spc_test_unit(int host_no, struct scsi_cmd *cmd)
>  *
>  * Returns number of bytes copied.
>  */
> -static int build_mode_page(uint8_t *data, struct mode_pg *pg, int update)
> +static int build_mode_page(uint8_t *data, struct mode_pg *pg, uint16_t *alloc_len)
>  {
>        uint8_t *p;
>        int len;
>
>        len = pg->pcode_size;
> -       if (update) {
> +       if (*alloc_len >= 2) {
>                data[0] = pg->pcode;
>                data[1] = len;
>        }
> +       *alloc_len -= min_t(uint16_t, *alloc_len, 2);
> +
>        p = &data[2];
>        len += 2;
> -       if (update)
> +       if (*alloc_len >= pg->pcode_size)
>                memcpy(p, pg->mode_data, pg->pcode_size);
>
> +       *alloc_len -= min_t(uint16_t, *alloc_len, pg->pcode_size);
> +
>        return len;
>  }
>
> @@ -362,9 +366,8 @@ static int build_mode_page(uint8_t *data, struct mode_pg *pg, int update)
>  */
>  int spc_mode_sense(int host_no, struct scsi_cmd *cmd)
>  {
> -       int len = 0;
>        uint8_t *data = NULL, *scb, mode6, dbd, pcode, subpcode;
> -       uint16_t alloc_len;
> +       uint16_t alloc_len, len = 0;
>        unsigned char key = ILLEGAL_REQUEST;
>        uint16_t asc = ASC_INVALID_FIELD_IN_CDB;
>        struct mode_pg *pg;
> @@ -393,11 +396,14 @@ int spc_mode_sense(int host_no, struct scsi_cmd *cmd)
>                goto sense;
>        memset(data, 0, alloc_len);
>
> +       alloc_len -= min(alloc_len, len);
> +
>        if (!dbd) {
> -               if (alloc_len >= len)
> +               if (alloc_len >= BLOCK_DESCRIPTOR_LEN)
>                        memcpy(data + len, cmd->dev->mode_block_descriptor,
>                               BLOCK_DESCRIPTOR_LEN);
>                len += BLOCK_DESCRIPTOR_LEN;
> +               alloc_len -= min_t(uint16_t, alloc_len, BLOCK_DESCRIPTOR_LEN);
>        }
>
>        if (pcode == 0x3f) {
> @@ -405,14 +411,13 @@ int spc_mode_sense(int host_no, struct scsi_cmd *cmd)
>                for (i = 0; i < ARRAY_SIZE(cmd->dev->mode_pgs); i++) {
>                        pg = cmd->dev->mode_pgs[i];
>                        if (pg)
> -                               len += build_mode_page(data + len, pg,
> -                                                      alloc_len >= len);
> +                               len += build_mode_page(data + len, pg, &alloc_len);
>                }
>        } else {
>                pg = cmd->dev->mode_pgs[pcode];
>                if (!pg)
>                        goto sense;
> -               len += build_mode_page(data + len, pg, alloc_len >= len);
> +               len += build_mode_page(data + len, pg, &alloc_len);
>        }
>
>        if (mode6) {
> _______________________________________________
> Stgt-devel mailing list
> Stgt-devel at lists.berlios.de
> https://lists.berlios.de/mailman/listinfo/stgt-devel
>