linux-kernel - Re: [PATCH 1/12] crypto: don't pollute the global namespace with 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <4643FD1C.1040401@panasas.com>
Date:	Fri, 11 May 2007 08:20:28 +0300
From:	Benny Halevy <bhalevy@...asas.com>
To:	Jens Axboe <jens.axboe@...cle.com>
CC:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/12] crypto: don't pollute the global namespace with
  sg_next()

Jens Axboe wrote:
> On Thu, May 10 2007, Benny Halevy wrote:
>> Jens Axboe wrote:
>>> It's a subsystem function, prefix it as such.
>> Jens, Boaz and I talked about this over lunch.
>> I wonder whether the crypto code must use your implementation
>> instead of its own as it needs to over the sglist, e.g. for
>> calculating iscsi (data) digest.
> 
> The thought did cross my mind, and yes I think that would be a good
> idea. The whole thing should probably just migrate to
> lib/scattersomething.c
> 
>> The crypto implementation of chained sglists in crypto/scatterwalk.h
>> determines the chain link by !sg->length which will sorta work
>> with your implementation, however the marker bit on page pointer must
>> be cleared to use it.
> 
> I don't like using sg->length, as that may be modified for legitimate
> reason. That's why I chose to use the lsb bit of the page pointer.
> 
>> Also, is it possible that after the original sglist has gone through
>> dma_map_sg and entries were merged, some entries will have zero
>> length?  I'm not sure... If so, if the crypto implementation scans
>> the sg list after it was dma mapped (maybe in a retry path) it
>> may hit an entry that looks to it like a chaining link.  This
>> might be an existing bug and another reason for the crypto code
>> to use your implementation.
> 
> It's hard to say, depends heavily on the sub system or arch. Even if
> using the pointer tagging mechanism seems a bit nasty, I think it's the
> more resilient approach.
> 

We're in agreement then :)
I was trying to say that the methods should be compatible, otherwise
bugs can happen, and that your scheme is better since it can
handle sglists with zero length entries that aren't the last.
A case that might be valid after dma mapping and merging.
If indeed this case is possible, this seems to be the right time
to converge to your scheme.

Benny
-
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/