linux-kernel - Re: [PATCH 0/5] sg

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20071227110927Q.fujita.tomonori@lab.ntt.co.jp>
Date:	Thu, 27 Dec 2007 11:09:27 +0900
From:	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
To:	rusty@...tcorp.com.au
Cc:	fujita.tomonori@....ntt.co.jp, davem@...emloft.net,
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
	jens.axboe@...cle.com, dougg@...que.net
Subject: Re: [PATCH 0/5] sg_ring for scsi

On Wed, 26 Dec 2007 11:27:40 +1100
Rusty Russell <rusty@...tcorp.com.au> wrote:

> On Friday 21 December 2007 15:37:46 FUJITA Tomonori wrote:
> > Some scsi drivers like ips access to sglist in a tricky way.
> 
> Indeed.  I fail to see how this code works, in fact:
> 
> drivers/scsi/ips.c:ips_queue() line 1101
> 
> 	if (ips_is_passthru(SC)) {
> 
> 		ips_copp_wait_item_t *scratch;
> 
> 		/* A Reset IOCTL is only sent by the boot CD in extreme cases.           */
> 		/* There can never be any system activity ( network or disk ), but check */
> 		/* anyway just as a good practice.                                       */
> 		pt = (ips_passthru_t *) scsi_sglist(SC);
> 		if ((pt->CoppCP.cmd.reset.op_code == IPS_CMD_RESET_CHANNEL) &&
> 		    (pt->CoppCP.cmd.reset.adapter_flag == 1)) {
> 
> Casting the scsi_sglist(SC) (which ips_is_passthrough treated as a
> scatterlist) to an ips_passthrough_t seems completely bogus.  It looks like
> it wants to access the region mapped by the scatterlist.

Yeah, it seems to be broken.

> There are many signs through the code that it needs a great deal of
> work: what is the purpose of sg_break? Why does the code check if kmap_atomic
> fails?

sg_break is a workaround for the hardware restrictions, I think.


> ===
> Convert the ips SCSI driver to sg_ring.
> 
> Slightly non-trivial conversion, will need testing.

As I said, I don't think that converting SCSI drivers to sg_ring (with
lots of non-trivial work) provides any benefits. Surely, sg_ring
enables us to modify sg lists but SCSI drivers don't need the
feature. What SCSI drivers needs is just a efficient way to get the
next sg entry (they use 'sg++' in the past and sg_next now).
--
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/