On Wed, Nov 02, 2016 at 01:54:20PM -0700, Eric Biggers wrote:
>
> I think the case where skcipher_copy_iv() fails may be handled incorrectly.
> Wouldn't it need to set walk.nbytes to 0 so as to not confuse callers which
> expect that behavior?  Or maybe it should be calling skcipher_walk_done().

Good catch.  I'll fix and repost.

> Setting walk->in.sg and walk->out.sg is redundant with the scatterwalk_start()
> calls.

Will remove.

> This gets called with uninitialized 'walk.flags'.  This was somewhat of a
> theoretical problem with the old blkcipher_walk code but it looks like now it
> will interact badly with the new SKCIPHER_WALK_SLEEP flag.  As far as I can 
> see,
> whether the flag will end up set or not can depend on the uninitialized value.
> It would be nice if this problem could be avoided entirely be setting flags=0.

Right.  I'll fix this as well.

> I'm also wondering about the choice to not look at 'atomic' until after the 
> call
> to skcipher_walk_skcipher().  Wouldn't this mean that the choice of 'atomic'
> would not be respected in e.g. the kmalloc() in skcipher_copy_iv()?

The atomic flag is meant to be used in cases such as aesni where
you need to do kernel_fpu_begin after the call to start the walk.
IOW sleeping is fine at the start but not on subsequent walk calls.

> I don't see any users of the "async" walking being introduced; are some 
> planned?

skcipher_walk is meant to unite blkcipher_walk and ablkcipher_walk.
The latter will use the async case.

Cheers,
-- 
Email: Herbert Xu <herb...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to