why store the search type in the ieee name field?

why store the search type in the ieee name field?

[Steven Dake]

Folks,

I was wondering the reasoning behind the code

scsi_scan:734
        /*
         * All OK - store ID
         */
      name[0] = hex_str[id_search->id_type];

This corrupts a perfectly good IEEE unique identifier (WWN for FC 
drives).  Is there some usage for this in user space?

Thanks
-steve

Re: why store the search type in the ieee name field?

[James Bottomley]

On Mon, 2003-02-10 at 16:45, Steven Dake wrote:
> Folks,
> 
> I was wondering the reasoning behind the code
> 
> scsi_scan:734
>         /*
>          * All OK - store ID
>          */
>       name[0] = hex_str[id_search->id_type];
> 
> This corrupts a perfectly good IEEE unique identifier (WWN for FC 
> drives).  Is there some usage for this in user space?

Erm, it's appending the type to the beginning of the name string
(although it does seem to be relying on the sysfs name field being zero
filled).  I don't see any corruption since the inquiry fields and unique
name name follows this byte.

You get the identifier it found from the sysfs name file for the
device.  The first byte tells you what type of unique name you're
dealing with.

James

Re: why store the search type in the ieee name field?

[Patrick Mansfield]

On Mon, Feb 10, 2003 at 05:12:54PM -0600, James Bottomley wrote:
> On Mon, 2003-02-10 at 16:45, Steven Dake wrote:
> > Folks,
> > 
> > I was wondering the reasoning behind the code
> > 
> > scsi_scan:734
> >         /*
> >          * All OK - store ID
> >          */
> >       name[0] = hex_str[id_search->id_type];
> > 
> > This corrupts a perfectly good IEEE unique identifier (WWN for FC 
> > drives).  Is there some usage for this in user space?
> 
> Erm, it's appending the type to the beginning of the name string
> (although it does seem to be relying on the sysfs name field being zero
> filled).  I don't see any corruption since the inquiry fields and unique
> name name follows this byte.

It should fill in the rest of the 'name' and append a '\0', or return a
failure.

> You get the identifier it found from the sysfs name file for the
> device.  The first byte tells you what type of unique name you're
> dealing with.
> 
> James

And the identifiers are not unique across different identifier types, so
it has to store the id_type in order to have unique identifiers (at least
for scsi) across all types: for example, a vendor specific id could be the
same as a NAA (IEEE) id.

-- Patrick Mansfield

Re: why store the search type in the ieee name field?

[James Bottomley]

On Tue, 2003-02-11 at 10:20, Patrick Mansfield wrote:
> On Mon, Feb 10, 2003 at 05:12:54PM -0600, James Bottomley wrote:
> > Erm, it's appending the type to the beginning of the name string
> > (although it does seem to be relying on the sysfs name field being zero
> > filled).  I don't see any corruption since the inquiry fields and unique
> > name name follows this byte.
> 
> It should fill in the rest of the 'name' and append a '\0', or return a
> failure.

No, look at the code.  After doing name[0] = hex_str[...
it then does a strcat.  If name is full of junk, strcat will append
after the first \0 it finds (which may be off the end of the name
field).  It only works because name[1] is zero but nothing seems to
clear it.  I'm hoping it's being cleared in a piece of code I haven't
found yet...

James

Re: why store the search type in the ieee name field?

[Patrick Mansfield]

On Tue, Feb 11, 2003 at 10:43:57AM -0600, James Bottomley wrote:

> No, look at the code.  After doing name[0] = hex_str[...
> it then does a strcat.  If name is full of junk, strcat will append
> after the first \0 it finds (which may be off the end of the name
> field).  It only works because name[1] is zero but nothing seems to
> clear it.  I'm hoping it's being cleared in a piece of code I haven't
> found yet...
> 
> James

Yes - there's a memset() in scsi_load_identifier, plus on error some of
the fields are re-zeroed (a bit ugly).

-- Patrick Mansfield

Re: why store the search type in the ieee name field?

[Steven Dake]

When I looked at this code originally, it looked correct, I was just 
wondering why have an identifier field. I understand the reasoning now.

Thanks
-steve

James Bottomley wrote:

>On Tue, 2003-02-11 at 10:20, Patrick Mansfield wrote:
>  
>
>>On Mon, Feb 10, 2003 at 05:12:54PM -0600, James Bottomley wrote:
>>    
>>
>>>Erm, it's appending the type to the beginning of the name string
>>>(although it does seem to be relying on the sysfs name field being zero
>>>filled).  I don't see any corruption since the inquiry fields and unique
>>>name name follows this byte.
>>>      
>>>
>>It should fill in the rest of the 'name' and append a '\0', or return a
>>failure.
>>    
>>
>
>No, look at the code.  After doing name[0] = hex_str[...
>it then does a strcat.  If name is full of junk, strcat will append
>after the first \0 it finds (which may be off the end of the name
>field).  It only works because name[1] is zero but nothing seems to
>clear it.  I'm hoping it's being cleared in a piece of code I haven't
>found yet...
>
>James
>
>
>
>
>
>  
>