isd200.c Has fatal bugs and is probably not used any more

isd200.c Has fatal bugs and is probably not used any more

[Boaz Harrosh]

Hi!

the driver drivers/usb/storage/isd200.c has code like:

static int isd200_action( struct us_data *us, int action,
              void* pointer, int value )

...

    case ACTION_READ_STATUS:
        US_DEBUGP("   isd200_action(READ_STATUS)\n");
        ata.generic.ActionSelect = ACTION_SELECT_0|ACTION_SELECT_2;
        ata.generic.RegisterSelect =
          REG_CYLINDER_LOW | REG_CYLINDER_HIGH |
          REG_STATUS | REG_ERROR;
        srb->sc_data_direction = DMA_FROM_DEVICE;
        srb->request_buffer = pointer;
        srb->request_bufflen = value;
        break;
...

Now what the hell is that hijacked scsi_cmnd and who said
that the srb->use_sg is Zero. If this is a regular queued
scsi_cmnd, this can easily crash. And at best a memory leak.

The last git recorded real change to this driver was it's insertion
into git by Linus.

Is this an indication that this driver is no longer used and
can be removed?

Boaz Harrosh

Re: isd200.c Has fatal bugs and is probably not used any more

[Doug Maxey]

On Tue, 21 Aug 2007 22:06:26 +0300, Boaz Harrosh wrote:
> Hi!
> 
> the driver drivers/usb/storage/isd200.c has code like:
> 
> static int isd200_action( struct us_data *us, int action,
>               void* pointer, int value )
> 
> ...
> 
>     case ACTION_READ_STATUS:
>         US_DEBUGP("   isd200_action(READ_STATUS)\n");
>         ata.generic.ActionSelect = ACTION_SELECT_0|ACTION_SELECT_2;
>         ata.generic.RegisterSelect =
>           REG_CYLINDER_LOW | REG_CYLINDER_HIGH |
>           REG_STATUS | REG_ERROR;
>         srb->sc_data_direction = DMA_FROM_DEVICE;
>         srb->request_buffer = pointer;
>         srb->request_bufflen = value;
>         break;
> ...
> 
> Now what the hell is that hijacked scsi_cmnd and who said
> that the srb->use_sg is Zero. If this is a regular queued
> scsi_cmnd, this can easily crash. And at best a memory leak.
> 
> The last git recorded real change to this driver was it's insertion
> into git by Linus.
> 
> Is this an indication that this driver is no longer used and
> can be removed?

All IBM bladecenters have the HW this driver supports.  And it does
mostly work.  This particular frag requires USB_DEBUG enabled, correct?

Re: isd200.c Has fatal bugs and is probably not used any more

[Boaz Harrosh]

Doug Maxey wrote:
> On Tue, 21 Aug 2007 22:06:26 +0300, Boaz Harrosh wrote:
>> Hi!
>>
>> the driver drivers/usb/storage/isd200.c has code like:
>>
>> static int isd200_action( struct us_data *us, int action,
>>               void* pointer, int value )
>>
>> ...
>>
>>     case ACTION_READ_STATUS:
>>         US_DEBUGP("   isd200_action(READ_STATUS)\n");
>>         ata.generic.ActionSelect = ACTION_SELECT_0|ACTION_SELECT_2;
>>         ata.generic.RegisterSelect =
>>           REG_CYLINDER_LOW | REG_CYLINDER_HIGH |
>>           REG_STATUS | REG_ERROR;
>>         srb->sc_data_direction = DMA_FROM_DEVICE;
>>         srb->request_buffer = pointer;
>>         srb->request_bufflen = value;
>>         break;
>> ...
>>
>> Now what the hell is that hijacked scsi_cmnd and who said
>> that the srb->use_sg is Zero. If this is a regular queued
>> scsi_cmnd, this can easily crash. And at best a memory leak.
>>
>> The last git recorded real change to this driver was it's insertion
>> into git by Linus.
>>
>> Is this an indication that this driver is no longer used and
>> can be removed?
> 
> All IBM bladecenters have the HW this driver supports.  And it does
> mostly work.  This particular frag requires USB_DEBUG enabled, correct?
> 

OK Thanks, Do you know if there is an active maintainer for this driver?
Else I will need someone that can test patches for me.

Boaz

Re: isd200.c Has fatal bugs and is probably not used any more

[Björn Stenberg]

Boaz Harrosh wrote:
> The last git recorded real change to this driver was it's insertion
> into git by Linus.
> 
> Is this an indication that this driver is no longer used and
> can be removed?

It is definitely used. The driver was introduced for a line of Archos MP3 players which are still in active use. The Rockbox projects gets around 500 firmware downloads for these players every month.

(Please cc me on replies, I am not on the list.)
-- 
Björn
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html