[stgt] [PATCH] smc: fix memory overwite bug in smc_read_element_status

[Mark Harvey]

On Wed, Dec 2, 2009 at 5:15 AM, Albert Pauw <albert.pauw at gmail.com> wrote:
> Hi Tomo,
>
> tried your patch. Seems to work well.
>
> If I use a targets.conf file and log into the vtl target I now get an error
> in stead of a hang:
>
> [root at orange tgt]# mtx -f /dev/changer status
> Warning:Too Many Storage Elements Reported (expected 0, now have 1
> Too many Data Transfer Elements Reported
>
> Which is good. If I build the vtl with my (manual) script it works fully, as
> before.
>
> So what we need now is that the tgt-admin script will issue the commands in
> the proper sequence, I guess.
> So the mode pages need to be set up before we attach the drivce luns to the
> changer (using the params parameter).
>
> Albert
>
>
>
> On 12/01/2009 06:22 PM, FUJITA Tomonori wrote:
>>
>> Signed-off-by: FUJITA Tomonori<fujita.tomonori at lab.ntt.co.jp>
>> ---
>>  usr/smc.c |   14 +++++++++++++-
>>  1 files changed, 13 insertions(+), 1 deletions(-)
>>
>> diff --git a/usr/smc.c b/usr/smc.c
>> index 6430882..c0f25d6 100644
>> --- a/usr/smc.c
>> +++ b/usr/smc.c
>> @@ -259,6 +259,17 @@ static int smc_initialize_element_status(int host_no,
>> struct scsi_cmd *cmd)
>>                return SAM_STAT_GOOD;
>>  }
>>
>> +static int nr_slots(struct smc_info *smc)
>> +{
>> +       int count = 0;
>> +       struct slot *s;
>> +
>> +       list_for_each_entry(s,&smc->slots, slot_siblings)
>> +               count++;
>> +
>> +       return count;
>> +}
>> +
>>  /**
>>   * smc_read_element_status  -  READ ELEMENT STATUS op code
>>   *
>> @@ -304,7 +315,8 @@ static int smc_read_element_status(int host_no, struct
>> scsi_cmd *cmd)
>>                }
>>        }
>>
>> -       data = zalloc(alloc_len);
>> +       /* we allocate possible maximum data length */
>> +       data = zalloc(8 + elementSize * nr_slots(smc));
>>        if (!data) {
>>                dprintf("Can't allocate enough memory for cmd\n");
>>                key = HARDWARE_ERROR;
>>
>
> --
> 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
>

Looks like a good solution :) If nothing else, makes the smc much more robust.

You can add my ACK to this.


FWIW: I'm still trying to figure out a nice way to prevent the element
page from being over-written with invalid (NULL) data.

Thinking along the lines of adding a 'Write-Protect' and 'Read-only'
attribute(s) to each mode page which could be optionally set.

Just havn't had any great ideas on best way about it yet.

Cheers
Mark
--
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