On 01/02/2016 12:52 PM, Milan Broz wrote:

On 12/25/2015 08:40 AM, Herbert Xu wrote:

quoted

Dmitry Vyukov [off-list ref] wrote:

quoted

I am testing with your two patches:
crypto: algif_skcipher - Use new skcipher interface
crypto: algif_skcipher - Require setkey before accept(2)
on top of a88164345b81292b55a8d4829fdd35c8d611cd7d (Dec 23).

You sent the email to everyone on the original CC list except me.
Please don't do that.

quoted

Now the following program causes a bunch of use-after-frees and them
kills kernel:

Yes there is an obvious bug in the patch that Julia Lawall has
responded to in another thread.  Here is a fixed version.

---8<--
Some cipher implementations will crash if you try to use them
without calling setkey first.  This patch adds a check so that
the accept(2) call will fail with -ENOKEY if setkey hasn't been
done on the socket yet.

Hi Herbert,

this patch breaks userspace in cryptsetup...

We use algif_skcipher in cryptsetup (for years, even before
there was Stephan's library) and with this patch applied
I see fail in ALG_SET_IV call (patch from your git).

(Obviously this was because of failing accept() call here, not set_iv.)

I can fix it upstream, but for thousands of installations it will
be broken (for LUKS there is a fallback, cor TrueCrypt compatible devices
it will be unusable. Also people who configured kernel crypto API as default
backend will have non-working cryptsetup).

Is it really thing for stable branch?

Also how it is supposed to work for cipher_null, where there is no key?
Why it should call set_key if it is noop? (and set key length 0 is not possible).

(We are using cipher_null for testing and for offline re-encryption tool
to create temporary "fake" header for not-yet encrypted device...)

Milan