On 01/02/2016 12:52 PM, Milan Broz wrote: > On 12/25/2015 08:40 AM, Herbert Xu wrote: >> Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote: >>> >>> 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. >> >>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
