linux-kernel - Re: [PATCH 1/4] [SCSI]stex: fix id mapping issue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Sat, 31 Mar 2007 09:22:08 -0500
From:	James Bottomley <James.Bottomley@...elEye.com>
To:	Ed Lin <ed.lin@...mise.com>
Cc:	linux-scsi <linux-scsi@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	jeff <jeff@...zik.org>, promise_linux <promise_linux@...mise.com>
Subject: Re: [PATCH 1/4] [SCSI]stex: fix id mapping issue

On Fri, 2007-03-30 at 15:21 -0700, Ed Lin wrote:
> The internal id/lun mapping of st_vsc and st_vsc1 controllers is different
> from st_shasta. The original driver code can only  map first 16 'entities'
> for st_vsc and st_vsc1 while there are actually 128 available.
> 
> Also the  ST_MAX_LUN_PER_TARGET should be 8, although this can do
> no harm because inquiries beyond boundary are discarded by firmware.
> 
> The correct internal mapping should be:
> id:0~15, lun:0~7 (st_shasta)
> id:0, lun:0~127 (st_yosemite)
> id:0~127, lun:0 (st_vsc and st_vsc1)
> To scsi mid layer they are all channel:0~7, id:0~15, lun:0, with a maximun
> 'entity' number of 128. The RAID console only interfaces to scsi mid layer
> and is always mapped at channel:0, id:16, lun:0.

I'm with Christoph here ... if we're going to break the backwards
compatibility of the mappings (which your code does) then we could just
dump channel and use the SCSI id and lun directly.

Understanding this code is predicated on this quirky definition in
stex_queuecommand:

	id = cmd->device->id;
	lun = cmd->device->channel; /* firmware lun issue work around */
                           ^^^^^^^

> @ -645,12 +645,16 @@ stex_queuecommand(struct scsi_cmnd *cmd,
>  
>  	req = stex_alloc_req(hba);
>  
> -	if (hba->cardtype == st_yosemite) {
> -		req->lun = lun * (ST_MAX_TARGET_NUM - 1) + id;

This looks to be correct, it goes up id 0 to ST_MAX_TARGET_NUM -1 then
takes the next channel.

> -		req->target = 0;
> -	} else {
> +	if (hba->cardtype == st_shasta) {
>  		req->lun = lun;
>  		req->target = id;
> +	} else if (hba->cardtype == st_yosemite){
> +		req->lun = id * ST_MAX_LUN_PER_TARGET + lun;
> +		req->target = 0;
> +	} else {
> +		/* st_vsc and st_vsc1 */
> +		req->lun = 0;
> +		req->target = id * ST_MAX_LUN_PER_TARGET + lun;

These both look to be wrong.  You're taking the channel as the lowest
common denominator, so your first target is on channel 1 id 0, your next
on channel 2, id 0 and so on.  That's really going to mess with the
ordering (which will be user visible) is that really what you want?

James


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/