sg_set_buf

sg_set_buf

[J. Bruce Fields]

The NFS code is using sg_set_buf to turn a bit of stack menory into a
scatterlist it can pass to the crypto code.  That started BUG()ing as of
ac4e97abce9b "scatterlist: sg_set_buf() argument must be in linear
mapping".

Is that BUG() being overly strict, or is this something we should never
have been doing in the first place?

--b.

Re: sg_set_buf

[Christoph Hellwig]

On Tue, Oct 18, 2016 at 05:37:55PM -0400, J. Bruce Fields wrote:
> The NFS code is using sg_set_buf to turn a bit of stack menory into a
> scatterlist it can pass to the crypto code.  That started BUG()ing as of
> ac4e97abce9b "scatterlist: sg_set_buf() argument must be in linear
> mapping".
> 
> Is that BUG() being overly strict, or is this something we should never
> have been doing in the first place?

In general we use scatterlists to pass dma addresses to hardware, so
yes, they should not point to the stack. 

Re: sg_set_buf

[J. Bruce Fields]

On Thu, Oct 20, 2016 at 03:22:00AM -0700, Christoph Hellwig wrote:
> On Tue, Oct 18, 2016 at 05:37:55PM -0400, J. Bruce Fields wrote:
> > The NFS code is using sg_set_buf to turn a bit of stack menory into a
> > scatterlist it can pass to the crypto code.  That started BUG()ing as of
> > ac4e97abce9b "scatterlist: sg_set_buf() argument must be in linear
> > mapping".
> > 
> > Is that BUG() being overly strict, or is this something we should never
> > have been doing in the first place?
> 
> In general we use scatterlists to pass dma addresses to hardware, so
> yes, they should not point to the stack. 

The crypto code isn't actually doing that though, is it?

(Or is there a chance it could be passing the data to separate crypto
hardware?  Do people do that these days?)

--b.

Re: sg_set_buf

[Christoph Hellwig]

On Thu, Oct 20, 2016 at 09:20:30AM -0400, J. Bruce Fields wrote:
> The crypto code isn't actually doing that though, is it?
> 
> (Or is there a chance it could be passing the data to separate crypto
> hardware?  Do people do that these days?)

Yes, there are tons of crypto accelarators, and they are built around
using struct scatterlist.

Re: sg_set_buf

[J. Bruce Fields]

On Thu, Oct 20, 2016 at 06:31:07AM -0700, Christoph Hellwig wrote:
> On Thu, Oct 20, 2016 at 09:20:30AM -0400, J. Bruce Fields wrote:
> > The crypto code isn't actually doing that though, is it?
> > 
> > (Or is there a chance it could be passing the data to separate crypto
> > hardware?  Do people do that these days?)
> 
> Yes, there are tons of crypto accelarators, and they are built around
> using struct scatterlist.

OK, thanks.

Turns out there are several places in the kerberos code where it just
needs to encrypt one small checksum or sequence number, and it's been
doing that on the stack.

For now I'll just sprinkle kmalloc()'s all over.  Eventually we'll need
to find something better.

--b.

Re: sg_set_buf

[Christoph Hellwig]

On Thu, Oct 20, 2016 at 05:42:19PM -0400, J. Bruce Fields wrote:
> Turns out there are several places in the kerberos code where it just
> needs to encrypt one small checksum or sequence number, and it's been
> doing that on the stack.
> 
> For now I'll just sprinkle kmalloc()'s all over.  Eventually we'll need
> to find something better.

I agree that it would be nice to be able to hash small objects on the
stack.  But unless I've missed something there is no way to do that
without using struct scatterlist.  I've added linux-crypto to the cc
list to confirm that I really didn't miss anything.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: sg_set_buf

[Christoph Hellwig]

On Thu, Oct 20, 2016 at 05:42:19PM -0400, J. Bruce Fields wrote:
> Turns out there are several places in the kerberos code where it just
> needs to encrypt one small checksum or sequence number, and it's been
> doing that on the stack.
> 
> For now I'll just sprinkle kmalloc()'s all over.  Eventually we'll need
> to find something better.

I agree that it would be nice to be able to hash small objects on the
stack.  But unless I've missed something there is no way to do that
without using struct scatterlist.  I've added linux-crypto to the cc
list to confirm that I really didn't miss anything.