Hi,
something is broken with crypto in linux-4.1.18.
On my system I have two disks (sda and sdb), both encrypted with LUKS
(cipher=aes-xts-plain64).
My rootfs resides encrypted on sda2 (sda1 is an unencrypted boot
partition).
sdb has one full encrypted partition (sdb1) mounted in "/backup".
After I upgraded from linux-4.1.17 to linux-4.1.18 and rebooted I noticed
that my encrypted rootfs was opened successfully (must be my initramfs)
however opening sdb1 with key file failed:
> * Setting up dm-crypt mappings ...
> * backupVault using: open /dev/sdb1 backupVault ...
> Failed to setup dm-crypt key mapping for device /dev/sdb1.
> Check that kernel supports aes-xts-plain64 cipher (check syslog for more info).
> * failure running cryptsetup
> [ !! ]
> * Failed to setup dm-crypt devices
> [ !! ]
> * ERROR: dmcrypt failed to start
Calling cryptsetup from terminal with debug option showed
> Failed to setup dm-crypt key mapping for device /dev/sdb1.
> Check that kernel supports aes-xts-plain64 cipher (check syslog for more info).
> Command failed with code 22: Invalid argument
> # cryptsetup 1.7.0 processing "cryptsetup --verbose --debug --key-file /etc/backupVault.key luksOpen /dev/sdb1 backupVault"
> # Running command open.
> # Locking memory.
> # Installing SIGINT/SIGTERM handler.
> # Unblocking interruption on signal.
> # Allocating crypt device /dev/sdb1 context.
> # Trying to open and read device /dev/sdb1 with direct-io.
> # Initialising device-mapper backend library.
> # Trying to load LUKS1 crypt type from device /dev/sdb1.
> # Crypto backend (gcrypt 1.6.5) initialized in cryptsetup library version 1.7.0.
> # Detected kernel Linux 4.1.18 x86_64.
> # Reading LUKS header of size 1024 from device /dev/sdb1
> # Key length 64, device size 10483679 sectors, header size 4036 sectors.
> # Timeout set to 0 miliseconds.
> # Password retry count set to 3.
> # Password verification disabled.
> # Iteration time set to 2000 miliseconds.
> # Password retry count set to 1.
> # Activating volume backupVault [keyslot -1] using keyfile /etc/backupVault.key.
> # dm version OF [16384] (*1)
> # dm versions OF [16384] (*1)
> # Detected dm-crypt version 1.14.1, dm-ioctl version 4.31.0.
> # Device-mapper backend running with UDEV support disabled.
> # dm status backupVault OF [16384] (*1)
> # File descriptor passphrase entry requested.
> # Trying to open key slot 0 [ACTIVE].
> # Reading key slot 0 area.
> # Userspace crypto wrapper cannot use aes-xts-plain64 (-22).
> # Releasing crypt device /dev/sdb1 context.
> # Releasing device-mapper backend.
> # Unlocking memory.
dmesg/syslog never showed an error.
Calling `cryptsetup benchmark` shows (notice the "N/A"):
> # Tests are approximate using memory only (no storage IO).
> PBKDF2-sha1 647269 iterations per second for 256-bit key
> PBKDF2-sha256 832203 iterations per second for 256-bit key
> PBKDF2-sha512 576140 iterations per second for 256-bit key
> PBKDF2-ripemd160 379918 iterations per second for 256-bit key
> PBKDF2-whirlpool 264791 iterations per second for 256-bit key
> # Algorithm | Key | Encryption | Decryption
> aes-cbc 128b N/A N/A
> serpent-cbc 128b N/A N/A
> twofish-cbc 128b N/A N/A
> aes-cbc 256b N/A N/A
> serpent-cbc 256b N/A N/A
> twofish-cbc 256b N/A N/A
> aes-xts 256b N/A N/A
> serpent-xts 256b N/A N/A
> twofish-xts 256b N/A N/A
> aes-xts 512b N/A N/A
> serpent-xts 512b N/A N/A
> twofish-xts 512b N/A N/A
Comparing /proc/crypto between 4.1.17 and 4.1.18:
> --- proc_crypto_4.1.17 2016-02-17 01:40:02.028647072 +0100
> +++ proc_crypto_4.1.18 2016-02-17 01:20:26.049998773 +0100
> @@ -1,158 +1,8 @@
> -name : __xts-twofish-avx
> -driver : cryptd(__driver-xts-twofish-avx)
> -module : kernel
> -priority : 50
> -refcnt : 1
> -selftest : passed
> -internal : yes
> -type : ablkcipher
> -async : yes
> -blocksize : 16
> -min keysize : 32
> -max keysize : 64
> -ivsize : 16
> -geniv : <default>
> -
> -name : xts(twofish)
> -driver : xts-twofish-avx
> -module : kernel
> -priority : 400
> -refcnt : 1
> -selftest : passed
> -internal : no
> -type : givcipher
> -async : yes
> -blocksize : 16
> -min keysize : 32
> -max keysize : 64
> -ivsize : 16
> -geniv : eseqiv
> -
> -name : __xts-serpent-avx
> -driver : cryptd(__driver-xts-serpent-avx)
> -module : kernel
> -priority : 50
> -refcnt : 1
> -selftest : passed
> -internal : yes
> -type : ablkcipher
> -async : yes
> -blocksize : 16
> -min keysize : 0
> -max keysize : 64
> -ivsize : 16
> -geniv : <default>
> -
> -name : xts(serpent)
> -driver : xts-serpent-avx
> -module : kernel
> -priority : 500
> -refcnt : 1
> -selftest : passed
> -internal : no
> -type : givcipher
> -async : yes
> -blocksize : 16
> -min keysize : 0
> -max keysize : 64
> -ivsize : 16
> -geniv : eseqiv
> -
> -name : __cbc-twofish-avx
> -driver : cryptd(__driver-cbc-twofish-avx)
> -module : kernel
> -priority : 50
> -refcnt : 1
> -selftest : passed
> -internal : yes
> -type : ablkcipher
> -async : yes
> -blocksize : 16
> -min keysize : 16
> -max keysize : 32
> -ivsize : 0
> -geniv : <default>
> -
> -name : cbc(twofish)
> -driver : cbc-twofish-avx
> -module : kernel
> -priority : 400
> -refcnt : 1
> -selftest : passed
> -internal : no
> -type : givcipher
> -async : yes
> -blocksize : 16
> -min keysize : 16
> -max keysize : 32
> -ivsize : 16
> -geniv : eseqiv
> -
> -name : __cbc-serpent-avx
> -driver : cryptd(__driver-cbc-serpent-avx)
> -module : kernel
> -priority : 50
> -refcnt : 1
> -selftest : passed
> -internal : yes
> -type : ablkcipher
> -async : yes
> -blocksize : 16
> -min keysize : 0
> -max keysize : 32
> -ivsize : 0
> -geniv : <default>
> -
> -name : cbc(serpent)
> -driver : cbc-serpent-avx
> -module : kernel
> -priority : 500
> -refcnt : 1
> -selftest : passed
> -internal : no
> -type : givcipher
> -async : yes
> -blocksize : 16
> -min keysize : 0
> -max keysize : 32
> -ivsize : 16
> -geniv : eseqiv
> -
> -name : __cbc-aes-aesni
> -driver : cryptd(__driver-cbc-aes-aesni)
> -module : kernel
> -priority : 50
> -refcnt : 1
> -selftest : passed
> -internal : yes
> -type : ablkcipher
> -async : yes
> -blocksize : 16
> -min keysize : 16
> -max keysize : 32
> -ivsize : 0
> -geniv : <default>
> -
> -name : cbc(aes)
> -driver : cbc-aes-aesni
> -module : kernel
> -priority : 400
> -refcnt : 1
> -selftest : passed
> -internal : no
> -type : givcipher
> -async : yes
> -blocksize : 16
> -min keysize : 16
> -max keysize : 32
> -ivsize : 16
> -geniv : eseqiv
> -
> name : __xts-aes-aesni
> driver : cryptd(__driver-xts-aes-aesni)
> module : kernel
> priority : 50
> -refcnt : 3
> +refcnt : 2
> selftest : passed
> internal : yes
> type : ablkcipher
> @@ -167,7 +17,7 @@
> driver : xts-aes-aesni
> module : kernel
> priority : 400
> -refcnt : 3
> +refcnt : 2
> selftest : passed
> internal : no
> type : givcipher
> @@ -1524,7 +1374,7 @@
> driver : xts-aes-aesni
> module : kernel
> priority : 400
> -refcnt : 3
> +refcnt : 2
> selftest : passed
> internal : no
> type : ablkcipher
> @@ -1554,7 +1404,7 @@
> driver : __driver-xts-aes-aesni
> module : kernel
> priority : 0
> -refcnt : 3
> +refcnt : 2
> selftest : passed
> internal : yes
> type : blkcipher
However I am using the same kernel configuration :)
After I bisect the kernel I found the following bad commit:
> commit 0571ba52a19e18a1c20469454231eef681cb1310
> Author: Herbert Xu <[email protected]>
> Date: Wed Dec 30 11:47:53 2015 +0800
>
> crypto: af_alg - Disallow bind/setkey/... after accept(2)
>
> [ Upstream commit c840ac6af3f8713a71b4d2363419145760bd6044 ]
>
> Each af_alg parent socket obtained by socket(2) corresponds to a
> tfm object once bind(2) has succeeded. An accept(2) call on that
> parent socket creates a context which then uses the tfm object.
>
> Therefore as long as any child sockets created by accept(2) exist
> the parent socket must not be modified or freed.
>
> This patch guarantees this by using locks and a reference count
> on the parent socket. Any attempt to modify the parent socket will
> fail with EBUSY.
bisect log:
> Bisecting: 114 revisions left to test after this (roughly 7 steps)
> [3a1e81ad84e4d880b00ecf7ad8d03b9b772ddfa7] crypto: algif_hash - Fix race condition in hash_check_key
> Bisecting: 56 revisions left to test after this (roughly 6 steps)
> [d6341753c418d3699948290d8c0b9d9dc78bd209] udf: Prevent buffer overrun with multi-byte characters
> Bisecting: 28 revisions left to test after this (roughly 5 steps)
> [13aedd784b84cb7d8a3bb835941d80e99f5c796e] dmaengine: dw: fix cyclic transfer setup
> Bisecting: 14 revisions left to test after this (roughly 4 steps)
> [664ecf4f243bac17065cd9878790d40a592e2f3d] zram/zcomp: use GFP_NOIO to allocate streams
> Bisecting: 7 revisions left to test after this (roughly 3 steps)
> [0571ba52a19e18a1c20469454231eef681cb1310] crypto: af_alg - Disallow bind/setkey/... after accept(2)
> Bisecting: 3 revisions left to test after this (roughly 2 steps)
> [2c641f5b0c8e87d43235ce39890bcc4d0c7cd2fb] memcg: only free spare array when readers are done
> Bisecting: 1 revision left to test after this (roughly 1 step)
> [0e19e24c3fe0abde8e2c5f4543616a251ccea6bf] kernel/panic.c: turn off locks debug before releasing console lock
> Bisecting: 0 revisions left to test after this (roughly 0 steps)
> [bc24ac15b0746172a8f603171352aa54abcf7c78] printk: do cond_resched() between lines while outputting to consoles
> 0571ba52a19e18a1c20469454231eef681cb1310 is the first bad commit
-Thomas
On 02/17/2016 09:04 AM, Thomas D. wrote:
> Hi,
>
> something is broken with crypto in linux-4.1.18.
>
> On my system I have two disks (sda and sdb), both encrypted with LUKS
> (cipher=aes-xts-plain64).
>
> My rootfs resides encrypted on sda2 (sda1 is an unencrypted boot
> partition).
> sdb has one full encrypted partition (sdb1) mounted in "/backup".
>
> After I upgraded from linux-4.1.17 to linux-4.1.18 and rebooted I noticed
> that my encrypted rootfs was opened successfully (must be my initramfs)
> however opening sdb1 with key file failed:
Thanks for the report Thomas.
[...]
> After I bisect the kernel I found the following bad commit:
>
>> commit 0571ba52a19e18a1c20469454231eef681cb1310
>> Author: Herbert Xu <[email protected]>
>> Date: Wed Dec 30 11:47:53 2015 +0800
>>
>> crypto: af_alg - Disallow bind/setkey/... after accept(2)
>>
>> [ Upstream commit c840ac6af3f8713a71b4d2363419145760bd6044 ]
>>
>> Each af_alg parent socket obtained by socket(2) corresponds to a
>> tfm object once bind(2) has succeeded. An accept(2) call on that
>> parent socket creates a context which then uses the tfm object.
>>
>> Therefore as long as any child sockets created by accept(2) exist
>> the parent socket must not be modified or freed.
>>
>> This patch guarantees this by using locks and a reference count
>> on the parent socket. Any attempt to modify the parent socket will
>> fail with EBUSY.
So either the upstream patch is broken, or the 4.1 backport is wrong/missing
dependency/missing fix.
Any chance you could try 4.5-rc3 and see if that works for you? That'll narrow
it down a lot.
Thanks,
Sasha
Hi,
Sasha Levin wrote:
> So either the upstream patch is broken, or the 4.1 backport is wrong/missing
> dependency/missing fix.
>
> Any chance you could try 4.5-rc3 and see if that works for you? That'll narrow
> it down a lot.
4.5-rc3 works for me.
Linux-4.4.0 works, too.
-Thomas
On 02/17/2016 10:24 AM, Thomas D. wrote:
> Hi,
>
> Sasha Levin wrote:
>> > So either the upstream patch is broken, or the 4.1 backport is wrong/missing
>> > dependency/missing fix.
>> >
>> > Any chance you could try 4.5-rc3 and see if that works for you? That'll narrow
>> > it down a lot.
> 4.5-rc3 works for me.
>
> Linux-4.4.0 works, too.
Herbert,
Is there a dependency I missed in 4.1? I don't really see anything that
could have gone wrong there.
Thanks,
Sasha
On Wed, Feb 17, 2016 at 05:12:56PM -0500, Sasha Levin wrote:
> On 02/17/2016 10:24 AM, Thomas D. wrote:
> > Hi,
> >
> > Sasha Levin wrote:
> >> > So either the upstream patch is broken, or the 4.1 backport is wrong/missing
> >> > dependency/missing fix.
> >> >
> >> > Any chance you could try 4.5-rc3 and see if that works for you? That'll narrow
> >> > it down a lot.
> > 4.5-rc3 works for me.
> >
> > Linux-4.4.0 works, too.
>
> Herbert,
>
> Is there a dependency I missed in 4.1? I don't really see anything that
> could have gone wrong there.
Or maybe Thomas can run a bisect ? Thomas, do you feel comfortable with
doing this ? There are a few patches between 4.1.17 and 4.1.18, in 8
steps it should be done, and only 4 if you limit yourself to the crypto
directory. Stable versions are fast at bisecting because each patch
touches a very isolated area so only the first build takes a bit of
time but the other ones are often pretty fast. Finding the faulty
patch can help spotting if something is wrong in its backport.
Regards,
Willy
Hi
Willy Tarreau wrote:
>> Is there a dependency I missed in 4.1? I don't really see anything that
>> could have gone wrong there.
>
> Or maybe Thomas can run a bisect ?
I cannot follow. I did a bisect between 4.1.7 and 4.1.8 as I have written
in my first mail. The bad commit was:
> commit 0571ba52a19e18a1c20469454231eef681cb1310
> Author: Herbert Xu
> Date: Wed Dec 30 11:47:53 2015 +0800
>
> crypto: af_alg - Disallow bind/setkey/... after accept(2)
>
> [ Upstream commit c840ac6af3f8713a71b4d2363419145760bd6044 ]
>
> Each af_alg parent socket obtained by socket(2) corresponds to a
> tfm object once bind(2) has succeeded. An accept(2) call on that
> parent socket creates a context which then uses the tfm object.
>
> Therefore as long as any child sockets created by accept(2) exist
> the parent socket must not be modified or freed.
>
> This patch guarantees this by using locks and a reference count
> on the parent socket. Any attempt to modify the parent socket will
> fail with EBUSY.
bisect log:
> Bisecting: 114 revisions left to test after this (roughly 7 steps)
> [3a1e81ad84e4d880b00ecf7ad8d03b9b772ddfa7] crypto: algif_hash - Fix race condition in hash_check_key
> Bisecting: 56 revisions left to test after this (roughly 6 steps)
> [d6341753c418d3699948290d8c0b9d9dc78bd209] udf: Prevent buffer overrun with multi-byte characters
> Bisecting: 28 revisions left to test after this (roughly 5 steps)
> [13aedd784b84cb7d8a3bb835941d80e99f5c796e] dmaengine: dw: fix cyclic transfer setup
> Bisecting: 14 revisions left to test after this (roughly 4 steps)
> [664ecf4f243bac17065cd9878790d40a592e2f3d] zram/zcomp: use GFP_NOIO to allocate streams
> Bisecting: 7 revisions left to test after this (roughly 3 steps)
> [0571ba52a19e18a1c20469454231eef681cb1310] crypto: af_alg - Disallow bind/setkey/... after accept(2)
> Bisecting: 3 revisions left to test after this (roughly 2 steps)
> [2c641f5b0c8e87d43235ce39890bcc4d0c7cd2fb] memcg: only free spare array when readers are done
> Bisecting: 1 revision left to test after this (roughly 1 step)
> [0e19e24c3fe0abde8e2c5f4543616a251ccea6bf] kernel/panic.c: turn off locks debug before releasing console lock
> Bisecting: 0 revisions left to test after this (roughly 0 steps)
> [bc24ac15b0746172a8f603171352aa54abcf7c78] printk: do cond_resched() between lines while outputting to consoles
> 0571ba52a19e18a1c20469454231eef681cb1310 is the first bad commit
-Thomas
On Thu, Feb 18, 2016 at 12:49:57AM +0100, Thomas D. wrote:
> Hi
>
> Willy Tarreau wrote:
> >> Is there a dependency I missed in 4.1? I don't really see anything that
> >> could have gone wrong there.
> >
> > Or maybe Thomas can run a bisect ?
>
> I cannot follow. I did a bisect between 4.1.7 and 4.1.8 as I have written
> in my first mail. The bad commit was:
>
> > commit 0571ba52a19e18a1c20469454231eef681cb1310
Ah sorry, I missed this one, I was confused by the test results on the
more recent kernels. So indeed, only this patch's backport has to be
studied.
Thanks and sorry for the noise.
Willy
Am Donnerstag, 18. Februar 2016, 00:49:57 schrieb Thomas D.:
Hi Thomas,
> Hi
>
> Willy Tarreau wrote:
> >> Is there a dependency I missed in 4.1? I don't really see anything that
> >> could have gone wrong there.
> >
> > Or maybe Thomas can run a bisect ?
>
> I cannot follow. I did a bisect between 4.1.7 and 4.1.8 as I have written
>
> in my first mail. The bad commit was:
That breakage was expected and forcasted by Milan Broz a couple of days ago on
this mailing list. The referenced patch covered a bug that must have been
fixed but introduced a regression for backwards compatibility. Since then,
this regression was fixed.
> > commit 0571ba52a19e18a1c20469454231eef681cb1310
> > Author: Herbert Xu
> > Date: Wed Dec 30 11:47:53 2015 +0800
> >
> > crypto: af_alg - Disallow bind/setkey/... after accept(2)
> >
> > [ Upstream commit c840ac6af3f8713a71b4d2363419145760bd6044 ]
> >
> > Each af_alg parent socket obtained by socket(2) corresponds to a
> > tfm object once bind(2) has succeeded. An accept(2) call on that
> > parent socket creates a context which then uses the tfm object.
> >
> > Therefore as long as any child sockets created by accept(2) exist
> > the parent socket must not be modified or freed.
> >
> > This patch guarantees this by using locks and a reference count
> > on the parent socket. Any attempt to modify the parent socket will
> > fail with EBUSY.
>
> bisect log:
> > Bisecting: 114 revisions left to test after this (roughly 7 steps)
> > [3a1e81ad84e4d880b00ecf7ad8d03b9b772ddfa7] crypto: algif_hash - Fix race
> > condition in hash_check_key Bisecting: 56 revisions left to test after
> > this (roughly 6 steps)
> > [d6341753c418d3699948290d8c0b9d9dc78bd209] udf: Prevent buffer overrun
> > with multi-byte characters Bisecting: 28 revisions left to test after
> > this (roughly 5 steps)
> > [13aedd784b84cb7d8a3bb835941d80e99f5c796e] dmaengine: dw: fix cyclic
> > transfer setup Bisecting: 14 revisions left to test after this (roughly 4
> > steps)
> > [664ecf4f243bac17065cd9878790d40a592e2f3d] zram/zcomp: use GFP_NOIO to
> > allocate streams Bisecting: 7 revisions left to test after this (roughly
> > 3 steps)
> > [0571ba52a19e18a1c20469454231eef681cb1310] crypto: af_alg - Disallow
> > bind/setkey/... after accept(2) Bisecting: 3 revisions left to test after
> > this (roughly 2 steps)
> > [2c641f5b0c8e87d43235ce39890bcc4d0c7cd2fb] memcg: only free spare array
> > when readers are done Bisecting: 1 revision left to test after this
> > (roughly 1 step)
> > [0e19e24c3fe0abde8e2c5f4543616a251ccea6bf] kernel/panic.c: turn off locks
> > debug before releasing console lock Bisecting: 0 revisions left to test
> > after this (roughly 0 steps)
> > [bc24ac15b0746172a8f603171352aa54abcf7c78] printk: do cond_resched()
> > between lines while outputting to consoles
> > 0571ba52a19e18a1c20469454231eef681cb1310 is the first bad commit
>
> -Thomas
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Ciao
Stephan
Hi,
On 02/18/2016, 09:17 AM, Stephan Mueller wrote:
> Am Donnerstag, 18. Februar 2016, 00:49:57 schrieb Thomas D.:
>> Willy Tarreau wrote:
>>>> Is there a dependency I missed in 4.1? I don't really see anything that
>>>> could have gone wrong there.
>>>
>>> Or maybe Thomas can run a bisect ?
>>
>> I cannot follow. I did a bisect between 4.1.7 and 4.1.8 as I have written
>>
>> in my first mail. The bad commit was:
>
> That breakage was expected and forcasted by Milan Broz a couple of days ago on
> this mailing list.
Could you be more specific? What is "this mailing list"? stable or
crypto? I cannot see anything from Milan on this mailing list aka stable.
> The referenced patch covered a bug that must have been
> fixed but introduced a regression for backwards compatibility. Since then,
> this regression was fixed.
By what? Please use SHAs, links, whatever.
thanks,
--
js
suse labs
Hi,
Jiri Slaby wrote:
>> That breakage was expected and forcasted by Milan Broz a couple of days ago on
>> this mailing list.
>
> Could you be more specific? What is "this mailing list"? stable or
> crypto? I cannot see anything from Milan on this mailing list aka stable.
Cannot speak for Stephan but I guess he means the "[PATCH v2] crypto:
algif_skcipher - Require setkey before accept(2)" thread on the crypto ml:
http://www.spinics.net/lists/linux-crypto/msg17931.html
http://www.spinics.net/lists/linux-crypto/msg17936.html
PS: In 4.4.2 it looks like the same commit breaking 4.1.8 was added
however 4.4.2 works for me.
-Thomas
Hi,
FYI: v3.10.97, v3.14.61 and 3.18.27 are also affected.
v4.3.6 works. Looks like the patch set is only compatible with >=linux-4.3.
v3.12.54 works because it doesn't contain the patch in question.
-Thomas
On 02/20/2016 03:33 PM, Thomas D. wrote:
> Hi,
>
> FYI: v3.10.97, v3.14.61 and 3.18.27 are also affected.
>
> v4.3.6 works. Looks like the patch set is only compatible with >=linux-4.3.
>
> v3.12.54 works because it doesn't contain the patch in question.
Hi,
indeed, because whoever backported this patchset skipped two patches
from series (because of skcipher interface file was introduced later).
I tried to backport it (on 4.1.18 tree, see patch below) and it fixes the problem
for me.
Anyway, it is just quick test what is the problem, patch need proper review or
backport from someone who knows the code better.
I will release stable cryptsetup soon that will work with new kernel even without
this patch but I would strongly recommend that stable kernels get these compatibility
backports as well to avoid breaking existing userspace.
Thanks,
Milan
-
This patch backports missing parts of crypto API compatibility changes:
dd504589577d8e8e70f51f997ad487a4cb6c026f
crypto: algif_skcipher - Require setkey before accept(2)
a0fa2d037129a9849918a92d91b79ed6c7bd2818
crypto: algif_skcipher - Add nokey compatibility path
--- crypto/algif_skcipher.c.orig 2016-02-21 16:44:27.172312990 +0100
+++ crypto/algif_skcipher.c 2016-02-21 17:03:58.555785347 +0100
@@ -31,6 +31,11 @@
struct scatterlist sg[0];
};
+struct skcipher_tfm {
+ struct crypto_ablkcipher *skcipher;
+ bool has_key;
+};
+
struct skcipher_ctx {
struct list_head tsgl;
struct af_alg_sgl rsgl;
@@ -750,19 +755,136 @@
.poll = skcipher_poll,
};
+static int skcipher_check_key(struct socket *sock)
+{
+ int err;
+ struct sock *psk;
+ struct alg_sock *pask;
+ struct skcipher_tfm *tfm;
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+
+ if (ask->refcnt)
+ return 0;
+
+ psk = ask->parent;
+ pask = alg_sk(ask->parent);
+ tfm = pask->private;
+
+ err = -ENOKEY;
+ lock_sock(psk);
+ if (!tfm->has_key)
+ goto unlock;
+
+ if (!pask->refcnt++)
+ sock_hold(psk);
+
+ ask->refcnt = 1;
+ sock_put(psk);
+
+ err = 0;
+
+unlock:
+ release_sock(psk);
+
+ return err;
+}
+
+static int skcipher_sendmsg_nokey(struct socket *sock, struct msghdr *msg,
+ size_t size)
+{
+ int err;
+
+ err = skcipher_check_key(sock);
+ if (err)
+ return err;
+
+ return skcipher_sendmsg(sock, msg, size);
+}
+
+static ssize_t skcipher_sendpage_nokey(struct socket *sock, struct page *page,
+ int offset, size_t size, int flags)
+{
+ int err;
+
+ err = skcipher_check_key(sock);
+ if (err)
+ return err;
+
+ return skcipher_sendpage(sock, page, offset, size, flags);
+}
+
+static int skcipher_recvmsg_nokey(struct socket *sock, struct msghdr *msg,
+ size_t ignored, int flags)
+{
+ int err;
+
+ err = skcipher_check_key(sock);
+ if (err)
+ return err;
+
+ return skcipher_recvmsg(sock, msg, ignored, flags);
+}
+
+static struct proto_ops algif_skcipher_ops_nokey = {
+ .family = PF_ALG,
+
+ .connect = sock_no_connect,
+ .socketpair = sock_no_socketpair,
+ .getname = sock_no_getname,
+ .ioctl = sock_no_ioctl,
+ .listen = sock_no_listen,
+ .shutdown = sock_no_shutdown,
+ .getsockopt = sock_no_getsockopt,
+ .mmap = sock_no_mmap,
+ .bind = sock_no_bind,
+ .accept = sock_no_accept,
+ .setsockopt = sock_no_setsockopt,
+
+ .release = af_alg_release,
+ .sendmsg = skcipher_sendmsg_nokey,
+ .sendpage = skcipher_sendpage_nokey,
+ .recvmsg = skcipher_recvmsg_nokey,
+ .poll = skcipher_poll,
+};
+
static void *skcipher_bind(const char *name, u32 type, u32 mask)
{
- return crypto_alloc_ablkcipher(name, type, mask);
+ struct skcipher_tfm *tfm;
+ struct crypto_ablkcipher *skcipher;
+
+ tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
+ if (!tfm)
+ return ERR_PTR(-ENOMEM);
+
+ skcipher = crypto_alloc_ablkcipher(name, type, mask);
+ if (IS_ERR(skcipher)) {
+ kfree(tfm);
+ return ERR_CAST(skcipher);
+ }
+
+ tfm->skcipher = skcipher;
+
+ return tfm;
}
static void skcipher_release(void *private)
{
- crypto_free_ablkcipher(private);
+ struct skcipher_tfm *tfm = private;
+
+ crypto_free_ablkcipher(tfm->skcipher);
+ kfree(tfm);
}
static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
{
- return crypto_ablkcipher_setkey(private, key, keylen);
+ struct skcipher_tfm *tfm = private;
+ int err;
+
+ err = crypto_ablkcipher_setkey(tfm->skcipher, key, keylen);
+ tfm->has_key = !err;
+
+ return err;
}
static void skcipher_wait(struct sock *sk)
@@ -775,7 +897,7 @@
msleep(100);
}
-static void skcipher_sock_destruct(struct sock *sk)
+static void skcipher_sock_destruct_common(struct sock *sk)
{
struct alg_sock *ask = alg_sk(sk);
struct skcipher_ctx *ctx = ask->private;
@@ -790,24 +912,50 @@
af_alg_release_parent(sk);
}
-static int skcipher_accept_parent(void *private, struct sock *sk)
+static void skcipher_sock_destruct(struct sock *sk)
+{
+ skcipher_sock_destruct_common(sk);
+ af_alg_release_parent(sk);
+}
+
+static void skcipher_release_parent_nokey(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+
+ if (!ask->refcnt) {
+ sock_put(ask->parent);
+ return;
+ }
+
+ af_alg_release_parent(sk);
+}
+
+static void skcipher_sock_destruct_nokey(struct sock *sk)
+{
+ skcipher_sock_destruct_common(sk);
+ skcipher_release_parent_nokey(sk);
+}
+
+static int skcipher_accept_parent_common(void *private, struct sock *sk)
{
struct skcipher_ctx *ctx;
struct alg_sock *ask = alg_sk(sk);
- unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(private);
+ struct skcipher_tfm *tfm = private;
+ struct crypto_ablkcipher *skcipher = tfm->skcipher;
+ unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(skcipher);
ctx = sock_kmalloc(sk, len, GFP_KERNEL);
if (!ctx)
return -ENOMEM;
- ctx->iv = sock_kmalloc(sk, crypto_ablkcipher_ivsize(private),
+ ctx->iv = sock_kmalloc(sk, crypto_ablkcipher_ivsize(skcipher),
GFP_KERNEL);
if (!ctx->iv) {
sock_kfree_s(sk, ctx, len);
return -ENOMEM;
}
- memset(ctx->iv, 0, crypto_ablkcipher_ivsize(private));
+ memset(ctx->iv, 0, crypto_ablkcipher_ivsize(skcipher));
INIT_LIST_HEAD(&ctx->tsgl);
ctx->len = len;
@@ -820,7 +968,7 @@
ask->private = ctx;
- ablkcipher_request_set_tfm(&ctx->req, private);
+ ablkcipher_request_set_tfm(&ctx->req, skcipher);
ablkcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
af_alg_complete, &ctx->completion);
@@ -829,12 +977,38 @@
return 0;
}
+static int skcipher_accept_parent(void *private, struct sock *sk)
+{
+ struct skcipher_tfm *tfm = private;
+
+ if (!tfm->has_key)
+ return -ENOKEY;
+
+ return skcipher_accept_parent_common(private, sk);
+}
+
+static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
+{
+ int err;
+
+ err = skcipher_accept_parent_common(private, sk);
+ if (err)
+ goto out;
+
+ sk->sk_destruct = skcipher_sock_destruct_nokey;
+
+out:
+ return err;
+}
+
static const struct af_alg_type algif_type_skcipher = {
.bind = skcipher_bind,
.release = skcipher_release,
.setkey = skcipher_setkey,
.accept = skcipher_accept_parent,
+ .accept_nokey = skcipher_accept_parent_nokey,
.ops = &algif_skcipher_ops,
+ .ops_nokey = &algif_skcipher_ops_nokey,
.name = "skcipher",
.owner = THIS_MODULE
};
On 02/21/2016 05:40 PM, Milan Broz wrote:
> On 02/20/2016 03:33 PM, Thomas D. wrote:
>> Hi,
>>
>> FYI: v3.10.97, v3.14.61 and 3.18.27 are also affected.
>>
>> v4.3.6 works. Looks like the patch set is only compatible with >=linux-4.3.
>>
>> v3.12.54 works because it doesn't contain the patch in question.
>
> Hi,
>
> indeed, because whoever backported this patchset skipped two patches
> from series (because of skcipher interface file was introduced later).
Ping?
I always thought that breaking userspace is not the way mainline kernel
operates and here we break even stable tree...
Anyone planning to release new kernel version with properly backported patches?
There is already a lot of downstream distro bugs reported.
Thanks,
Milan
>
> I tried to backport it (on 4.1.18 tree, see patch below) and it fixes the problem
> for me.
>
> Anyway, it is just quick test what is the problem, patch need proper review or
> backport from someone who knows the code better.
>
> I will release stable cryptsetup soon that will work with new kernel even without
> this patch but I would strongly recommend that stable kernels get these compatibility
> backports as well to avoid breaking existing userspace.
>
> Thanks,
> Milan
> -
>
> This patch backports missing parts of crypto API compatibility changes:
>
> dd504589577d8e8e70f51f997ad487a4cb6c026f
> crypto: algif_skcipher - Require setkey before accept(2)
>
> a0fa2d037129a9849918a92d91b79ed6c7bd2818
> crypto: algif_skcipher - Add nokey compatibility path
>
> --- crypto/algif_skcipher.c.orig 2016-02-21 16:44:27.172312990 +0100
> +++ crypto/algif_skcipher.c 2016-02-21 17:03:58.555785347 +0100
> @@ -31,6 +31,11 @@
> struct scatterlist sg[0];
> };
>
> +struct skcipher_tfm {
> + struct crypto_ablkcipher *skcipher;
> + bool has_key;
> +};
> +
> struct skcipher_ctx {
> struct list_head tsgl;
> struct af_alg_sgl rsgl;
> @@ -750,19 +755,136 @@
> .poll = skcipher_poll,
> };
>
> +static int skcipher_check_key(struct socket *sock)
> +{
> + int err;
> + struct sock *psk;
> + struct alg_sock *pask;
> + struct skcipher_tfm *tfm;
> + struct sock *sk = sock->sk;
> + struct alg_sock *ask = alg_sk(sk);
> +
> + if (ask->refcnt)
> + return 0;
> +
> + psk = ask->parent;
> + pask = alg_sk(ask->parent);
> + tfm = pask->private;
> +
> + err = -ENOKEY;
> + lock_sock(psk);
> + if (!tfm->has_key)
> + goto unlock;
> +
> + if (!pask->refcnt++)
> + sock_hold(psk);
> +
> + ask->refcnt = 1;
> + sock_put(psk);
> +
> + err = 0;
> +
> +unlock:
> + release_sock(psk);
> +
> + return err;
> +}
> +
> +static int skcipher_sendmsg_nokey(struct socket *sock, struct msghdr *msg,
> + size_t size)
> +{
> + int err;
> +
> + err = skcipher_check_key(sock);
> + if (err)
> + return err;
> +
> + return skcipher_sendmsg(sock, msg, size);
> +}
> +
> +static ssize_t skcipher_sendpage_nokey(struct socket *sock, struct page *page,
> + int offset, size_t size, int flags)
> +{
> + int err;
> +
> + err = skcipher_check_key(sock);
> + if (err)
> + return err;
> +
> + return skcipher_sendpage(sock, page, offset, size, flags);
> +}
> +
> +static int skcipher_recvmsg_nokey(struct socket *sock, struct msghdr *msg,
> + size_t ignored, int flags)
> +{
> + int err;
> +
> + err = skcipher_check_key(sock);
> + if (err)
> + return err;
> +
> + return skcipher_recvmsg(sock, msg, ignored, flags);
> +}
> +
> +static struct proto_ops algif_skcipher_ops_nokey = {
> + .family = PF_ALG,
> +
> + .connect = sock_no_connect,
> + .socketpair = sock_no_socketpair,
> + .getname = sock_no_getname,
> + .ioctl = sock_no_ioctl,
> + .listen = sock_no_listen,
> + .shutdown = sock_no_shutdown,
> + .getsockopt = sock_no_getsockopt,
> + .mmap = sock_no_mmap,
> + .bind = sock_no_bind,
> + .accept = sock_no_accept,
> + .setsockopt = sock_no_setsockopt,
> +
> + .release = af_alg_release,
> + .sendmsg = skcipher_sendmsg_nokey,
> + .sendpage = skcipher_sendpage_nokey,
> + .recvmsg = skcipher_recvmsg_nokey,
> + .poll = skcipher_poll,
> +};
> +
> static void *skcipher_bind(const char *name, u32 type, u32 mask)
> {
> - return crypto_alloc_ablkcipher(name, type, mask);
> + struct skcipher_tfm *tfm;
> + struct crypto_ablkcipher *skcipher;
> +
> + tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
> + if (!tfm)
> + return ERR_PTR(-ENOMEM);
> +
> + skcipher = crypto_alloc_ablkcipher(name, type, mask);
> + if (IS_ERR(skcipher)) {
> + kfree(tfm);
> + return ERR_CAST(skcipher);
> + }
> +
> + tfm->skcipher = skcipher;
> +
> + return tfm;
> }
>
> static void skcipher_release(void *private)
> {
> - crypto_free_ablkcipher(private);
> + struct skcipher_tfm *tfm = private;
> +
> + crypto_free_ablkcipher(tfm->skcipher);
> + kfree(tfm);
> }
>
> static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
> {
> - return crypto_ablkcipher_setkey(private, key, keylen);
> + struct skcipher_tfm *tfm = private;
> + int err;
> +
> + err = crypto_ablkcipher_setkey(tfm->skcipher, key, keylen);
> + tfm->has_key = !err;
> +
> + return err;
> }
>
> static void skcipher_wait(struct sock *sk)
> @@ -775,7 +897,7 @@
> msleep(100);
> }
>
> -static void skcipher_sock_destruct(struct sock *sk)
> +static void skcipher_sock_destruct_common(struct sock *sk)
> {
> struct alg_sock *ask = alg_sk(sk);
> struct skcipher_ctx *ctx = ask->private;
> @@ -790,24 +912,50 @@
> af_alg_release_parent(sk);
> }
>
> -static int skcipher_accept_parent(void *private, struct sock *sk)
> +static void skcipher_sock_destruct(struct sock *sk)
> +{
> + skcipher_sock_destruct_common(sk);
> + af_alg_release_parent(sk);
> +}
> +
> +static void skcipher_release_parent_nokey(struct sock *sk)
> +{
> + struct alg_sock *ask = alg_sk(sk);
> +
> + if (!ask->refcnt) {
> + sock_put(ask->parent);
> + return;
> + }
> +
> + af_alg_release_parent(sk);
> +}
> +
> +static void skcipher_sock_destruct_nokey(struct sock *sk)
> +{
> + skcipher_sock_destruct_common(sk);
> + skcipher_release_parent_nokey(sk);
> +}
> +
> +static int skcipher_accept_parent_common(void *private, struct sock *sk)
> {
> struct skcipher_ctx *ctx;
> struct alg_sock *ask = alg_sk(sk);
> - unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(private);
> + struct skcipher_tfm *tfm = private;
> + struct crypto_ablkcipher *skcipher = tfm->skcipher;
> + unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(skcipher);
>
> ctx = sock_kmalloc(sk, len, GFP_KERNEL);
> if (!ctx)
> return -ENOMEM;
>
> - ctx->iv = sock_kmalloc(sk, crypto_ablkcipher_ivsize(private),
> + ctx->iv = sock_kmalloc(sk, crypto_ablkcipher_ivsize(skcipher),
> GFP_KERNEL);
> if (!ctx->iv) {
> sock_kfree_s(sk, ctx, len);
> return -ENOMEM;
> }
>
> - memset(ctx->iv, 0, crypto_ablkcipher_ivsize(private));
> + memset(ctx->iv, 0, crypto_ablkcipher_ivsize(skcipher));
>
> INIT_LIST_HEAD(&ctx->tsgl);
> ctx->len = len;
> @@ -820,7 +968,7 @@
>
> ask->private = ctx;
>
> - ablkcipher_request_set_tfm(&ctx->req, private);
> + ablkcipher_request_set_tfm(&ctx->req, skcipher);
> ablkcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> af_alg_complete, &ctx->completion);
>
> @@ -829,12 +977,38 @@
> return 0;
> }
>
> +static int skcipher_accept_parent(void *private, struct sock *sk)
> +{
> + struct skcipher_tfm *tfm = private;
> +
> + if (!tfm->has_key)
> + return -ENOKEY;
> +
> + return skcipher_accept_parent_common(private, sk);
> +}
> +
> +static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
> +{
> + int err;
> +
> + err = skcipher_accept_parent_common(private, sk);
> + if (err)
> + goto out;
> +
> + sk->sk_destruct = skcipher_sock_destruct_nokey;
> +
> +out:
> + return err;
> +}
> +
> static const struct af_alg_type algif_type_skcipher = {
> .bind = skcipher_bind,
> .release = skcipher_release,
> .setkey = skcipher_setkey,
> .accept = skcipher_accept_parent,
> + .accept_nokey = skcipher_accept_parent_nokey,
> .ops = &algif_skcipher_ops,
> + .ops_nokey = &algif_skcipher_ops_nokey,
> .name = "skcipher",
> .owner = THIS_MODULE
> };
>
>
On 02/23/2016 04:02 PM, Milan Broz wrote:
> On 02/21/2016 05:40 PM, Milan Broz wrote:
>> > On 02/20/2016 03:33 PM, Thomas D. wrote:
>>> >> Hi,
>>> >>
>>> >> FYI: v3.10.97, v3.14.61 and 3.18.27 are also affected.
>>> >>
>>> >> v4.3.6 works. Looks like the patch set is only compatible with >=linux-4.3.
>>> >>
>>> >> v3.12.54 works because it doesn't contain the patch in question.
>> >
>> > Hi,
>> >
>> > indeed, because whoever backported this patchset skipped two patches
>> > from series (because of skcipher interface file was introduced later).
> Ping?
>
> I always thought that breaking userspace is not the way mainline kernel
> operates and here we break even stable tree...
>
> Anyone planning to release new kernel version with properly backported patches?
> There is already a lot of downstream distro bugs reported.
Hi Milan,
I'd really like to see an ack on your patch by one of the crypto/ maintainers
before putting it into a -stable release.
Thanks,
Sasha
Hi,
I have applied Milan's patch on top of 4.1.18. I can reboot and open all
of my LUKS-encrypted disks. "cryptsetup benchmark" also works.
However, don't we need all the recent changes from
"crypto/algif_skcipher.c", too?
-Thomas
On Wed, Feb 24, 2016 at 01:10:55AM +0100, Thomas D. wrote:
> Hi,
>
> I have applied Milan's patch on top of 4.1.18. I can reboot and open all
> of my LUKS-encrypted disks. "cryptsetup benchmark" also works.
>
> However, don't we need all the recent changes from
> "crypto/algif_skcipher.c", too?
Can someone just backport the full patches in a proper format that I can
apply them in for the 3.14 and 3.10 kernels? I told people that they
failed to apply, or at least I thought I did...
thanks,
greg k-h
On 02/21/2016, 05:40 PM, Milan Broz wrote:
> On 02/20/2016 03:33 PM, Thomas D. wrote:
>> Hi,
>>
>> FYI: v3.10.97, v3.14.61 and 3.18.27 are also affected.
>>
>> v4.3.6 works. Looks like the patch set is only compatible with >=linux-4.3.
>>
>> v3.12.54 works because it doesn't contain the patch in question.
>
> Hi,
>
> indeed, because whoever backported this patchset skipped two patches
> from series (because of skcipher interface file was introduced later).
>
> I tried to backport it (on 4.1.18 tree, see patch below) and it fixes the problem
> for me.
>
> Anyway, it is just quick test what is the problem, patch need proper review or
> backport from someone who knows the code better.
>
> I will release stable cryptsetup soon that will work with new kernel even without
> this patch but I would strongly recommend that stable kernels get these compatibility
> backports as well to avoid breaking existing userspace.
>
> Thanks,
> Milan
> -
>
> This patch backports missing parts of crypto API compatibility changes:
>
> dd504589577d8e8e70f51f997ad487a4cb6c026f
> crypto: algif_skcipher - Require setkey before accept(2)
>
> a0fa2d037129a9849918a92d91b79ed6c7bd2818
> crypto: algif_skcipher - Add nokey compatibility path
>
> --- crypto/algif_skcipher.c.orig 2016-02-21 16:44:27.172312990 +0100
> +++ crypto/algif_skcipher.c 2016-02-21 17:03:58.555785347 +0100
...
> @@ -790,24 +912,50 @@
> af_alg_release_parent(sk);
This,
> }
>
> -static int skcipher_accept_parent(void *private, struct sock *sk)
> +static void skcipher_sock_destruct(struct sock *sk)
> +{
> + skcipher_sock_destruct_common(sk);
> + af_alg_release_parent(sk);
this,
> +}
> +
> +static void skcipher_release_parent_nokey(struct sock *sk)
> +{
> + struct alg_sock *ask = alg_sk(sk);
> +
> + if (!ask->refcnt) {
> + sock_put(ask->parent);
> + return;
> + }
> +
> + af_alg_release_parent(sk);
and this occurs to me like a double release?
thanks,
--
js
suse labs
On 02/24/2016 09:32 AM, Jiri Slaby wrote:
>> + af_alg_release_parent(sk);
>
> and this occurs to me like a double release?
yes, my copy&paste mistake.
Anyway, there should be also two more patches from series.
If it helps, I copied proper backport here (upstream commit is referenced in header)
https://mbroz.fedorapeople.org/tmp/4.1/
Older kernel probably need some more changes but I hope these should be trivial.
Thanks,
Milan
On Wed, Feb 24, 2016 at 09:54:48AM +0100, Milan Broz wrote:
> On 02/24/2016 09:32 AM, Jiri Slaby wrote:
> >> + af_alg_release_parent(sk);
> >
> > and this occurs to me like a double release?
>
> yes, my copy&paste mistake.
Which is why I want the real patches backported please. Whenever we do
a "just this smaller patch" for a stable kernel, it is ALWAYS wrong.
Please backport the patches in a correct way so that we can apply
them...
thanks,
greg k-h
On 02/24/2016 06:12 PM, Greg KH wrote:
> On Wed, Feb 24, 2016 at 09:54:48AM +0100, Milan Broz wrote:
>> On 02/24/2016 09:32 AM, Jiri Slaby wrote:
>>>> + af_alg_release_parent(sk);
>>>
>>> and this occurs to me like a double release?
>>
>> yes, my copy&paste mistake.
>
> Which is why I want the real patches backported please. Whenever we do
> a "just this smaller patch" for a stable kernel, it is ALWAYS wrong.
I think that it was clear that I do not want you to directly include
this patch, just it points to the direction where is the problem.
Anyway, seems the problem is only in 4.1.18.
> Please backport the patches in a correct way so that we can apply
> them...
Not sure if it is still needed, but I'll reply to this thread with my git version
of backported patches for 4.1.18.
(Resp. only the first need changes, other then applied cleanly from upstream).
Thanks,
Milan
From: Herbert Xu <[email protected]>
commit dd504589577d8e8e70f51f997ad487a4cb6c026f upstream.
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.
Cc: [email protected]
Reported-by: Dmitry Vyukov <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>
Tested-by: Dmitry Vyukov <[email protected]>
[backported to 4.1 by Milan Broz <[email protected]>]
---
crypto/algif_skcipher.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 41 insertions(+), 7 deletions(-)
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 5bc42f9..1c9879d 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -31,6 +31,11 @@ struct skcipher_sg_list {
struct scatterlist sg[0];
};
+struct skcipher_tfm {
+ struct crypto_ablkcipher *skcipher;
+ bool has_key;
+};
+
struct skcipher_ctx {
struct list_head tsgl;
struct af_alg_sgl rsgl;
@@ -752,17 +757,41 @@ static struct proto_ops algif_skcipher_ops = {
static void *skcipher_bind(const char *name, u32 type, u32 mask)
{
- return crypto_alloc_ablkcipher(name, type, mask);
+ struct skcipher_tfm *tfm;
+ struct crypto_ablkcipher *skcipher;
+
+ tfm = kzalloc(sizeof(*tfm), GFP_KERNEL);
+ if (!tfm)
+ return ERR_PTR(-ENOMEM);
+
+ skcipher = crypto_alloc_ablkcipher(name, type, mask);
+ if (IS_ERR(skcipher)) {
+ kfree(tfm);
+ return ERR_CAST(skcipher);
+ }
+
+ tfm->skcipher = skcipher;
+
+ return tfm;
}
static void skcipher_release(void *private)
{
- crypto_free_ablkcipher(private);
+ struct skcipher_tfm *tfm = private;
+
+ crypto_free_ablkcipher(tfm->skcipher);
+ kfree(tfm);
}
static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
{
- return crypto_ablkcipher_setkey(private, key, keylen);
+ struct skcipher_tfm *tfm = private;
+ int err;
+
+ err = crypto_ablkcipher_setkey(tfm->skcipher, key, keylen);
+ tfm->has_key = !err;
+
+ return err;
}
static void skcipher_wait(struct sock *sk)
@@ -794,20 +823,25 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
{
struct skcipher_ctx *ctx;
struct alg_sock *ask = alg_sk(sk);
- unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(private);
+ struct skcipher_tfm *tfm = private;
+ struct crypto_ablkcipher *skcipher = tfm->skcipher;
+ unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(skcipher);
+
+ if (!tfm->has_key)
+ return -ENOKEY;
ctx = sock_kmalloc(sk, len, GFP_KERNEL);
if (!ctx)
return -ENOMEM;
- ctx->iv = sock_kmalloc(sk, crypto_ablkcipher_ivsize(private),
+ ctx->iv = sock_kmalloc(sk, crypto_ablkcipher_ivsize(skcipher),
GFP_KERNEL);
if (!ctx->iv) {
sock_kfree_s(sk, ctx, len);
return -ENOMEM;
}
- memset(ctx->iv, 0, crypto_ablkcipher_ivsize(private));
+ memset(ctx->iv, 0, crypto_ablkcipher_ivsize(skcipher));
INIT_LIST_HEAD(&ctx->tsgl);
ctx->len = len;
@@ -820,7 +854,7 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
ask->private = ctx;
- ablkcipher_request_set_tfm(&ctx->req, private);
+ ablkcipher_request_set_tfm(&ctx->req, skcipher);
ablkcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
af_alg_complete, &ctx->completion);
--
2.7.0
From: Herbert Xu <[email protected]>
commit d7b65aee1e7b4c87922b0232eaba56a8a143a4a0 upstream.
This patch removes the custom release parent function as the
generic af_alg_release_parent now works for nokey sockets too.
Cc: [email protected]
Signed-off-by: Herbert Xu <[email protected]>
---
crypto/algif_skcipher.c | 43 +++----------------------------------------
1 file changed, 3 insertions(+), 40 deletions(-)
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 566df2c..83bcf75 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -897,7 +897,7 @@ static void skcipher_wait(struct sock *sk)
msleep(100);
}
-static void skcipher_sock_destruct_common(struct sock *sk)
+static void skcipher_sock_destruct(struct sock *sk)
{
struct alg_sock *ask = alg_sk(sk);
struct skcipher_ctx *ctx = ask->private;
@@ -909,33 +909,10 @@ static void skcipher_sock_destruct_common(struct sock *sk)
skcipher_free_sgl(sk);
sock_kzfree_s(sk, ctx->iv, crypto_ablkcipher_ivsize(tfm));
sock_kfree_s(sk, ctx, ctx->len);
-}
-
-static void skcipher_sock_destruct(struct sock *sk)
-{
- skcipher_sock_destruct_common(sk);
- af_alg_release_parent(sk);
-}
-
-static void skcipher_release_parent_nokey(struct sock *sk)
-{
- struct alg_sock *ask = alg_sk(sk);
-
- if (!ask->refcnt) {
- sock_put(ask->parent);
- return;
- }
-
af_alg_release_parent(sk);
}
-static void skcipher_sock_destruct_nokey(struct sock *sk)
-{
- skcipher_sock_destruct_common(sk);
- skcipher_release_parent_nokey(sk);
-}
-
-static int skcipher_accept_parent_common(void *private, struct sock *sk)
+static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
{
struct skcipher_ctx *ctx;
struct alg_sock *ask = alg_sk(sk);
@@ -983,21 +960,7 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
if (!tfm->has_key)
return -ENOKEY;
- return skcipher_accept_parent_common(private, sk);
-}
-
-static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
-{
- int err;
-
- err = skcipher_accept_parent_common(private, sk);
- if (err)
- goto out;
-
- sk->sk_destruct = skcipher_sock_destruct_nokey;
-
-out:
- return err;
+ return skcipher_accept_parent_nokey(private, sk);
}
static const struct af_alg_type algif_type_skcipher = {
--
2.7.0
From: Herbert Xu <[email protected]>
commit a0fa2d037129a9849918a92d91b79ed6c7bd2818 upstream.
This patch adds a compatibility path to support old applications
that do acept(2) before setkey.
Cc: [email protected]
Signed-off-by: Herbert Xu <[email protected]>
---
crypto/algif_skcipher.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 144 insertions(+), 5 deletions(-)
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 1c9879d..566df2c 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -755,6 +755,99 @@ static struct proto_ops algif_skcipher_ops = {
.poll = skcipher_poll,
};
+static int skcipher_check_key(struct socket *sock)
+{
+ int err;
+ struct sock *psk;
+ struct alg_sock *pask;
+ struct skcipher_tfm *tfm;
+ struct sock *sk = sock->sk;
+ struct alg_sock *ask = alg_sk(sk);
+
+ if (ask->refcnt)
+ return 0;
+
+ psk = ask->parent;
+ pask = alg_sk(ask->parent);
+ tfm = pask->private;
+
+ err = -ENOKEY;
+ lock_sock(psk);
+ if (!tfm->has_key)
+ goto unlock;
+
+ if (!pask->refcnt++)
+ sock_hold(psk);
+
+ ask->refcnt = 1;
+ sock_put(psk);
+
+ err = 0;
+
+unlock:
+ release_sock(psk);
+
+ return err;
+}
+
+static int skcipher_sendmsg_nokey(struct socket *sock, struct msghdr *msg,
+ size_t size)
+{
+ int err;
+
+ err = skcipher_check_key(sock);
+ if (err)
+ return err;
+
+ return skcipher_sendmsg(sock, msg, size);
+}
+
+static ssize_t skcipher_sendpage_nokey(struct socket *sock, struct page *page,
+ int offset, size_t size, int flags)
+{
+ int err;
+
+ err = skcipher_check_key(sock);
+ if (err)
+ return err;
+
+ return skcipher_sendpage(sock, page, offset, size, flags);
+}
+
+static int skcipher_recvmsg_nokey(struct socket *sock, struct msghdr *msg,
+ size_t ignored, int flags)
+{
+ int err;
+
+ err = skcipher_check_key(sock);
+ if (err)
+ return err;
+
+ return skcipher_recvmsg(sock, msg, ignored, flags);
+}
+
+static struct proto_ops algif_skcipher_ops_nokey = {
+ .family = PF_ALG,
+
+ .connect = sock_no_connect,
+ .socketpair = sock_no_socketpair,
+ .getname = sock_no_getname,
+ .ioctl = sock_no_ioctl,
+ .listen = sock_no_listen,
+ .shutdown = sock_no_shutdown,
+ .getsockopt = sock_no_getsockopt,
+ .mmap = sock_no_mmap,
+ .bind = sock_no_bind,
+ .accept = sock_no_accept,
+ .setsockopt = sock_no_setsockopt,
+
+ .release = af_alg_release,
+ .sendmsg = skcipher_sendmsg_nokey,
+ .sendpage = skcipher_sendpage_nokey,
+ .recvmsg = skcipher_recvmsg_nokey,
+ .poll = skcipher_poll,
+};
+
static void *skcipher_bind(const char *name, u32 type, u32 mask)
{
struct skcipher_tfm *tfm;
@@ -804,7 +897,7 @@ static void skcipher_wait(struct sock *sk)
msleep(100);
}
-static void skcipher_sock_destruct(struct sock *sk)
+static void skcipher_sock_destruct_common(struct sock *sk)
{
struct alg_sock *ask = alg_sk(sk);
struct skcipher_ctx *ctx = ask->private;
@@ -816,10 +909,33 @@ static void skcipher_sock_destruct(struct sock *sk)
skcipher_free_sgl(sk);
sock_kzfree_s(sk, ctx->iv, crypto_ablkcipher_ivsize(tfm));
sock_kfree_s(sk, ctx, ctx->len);
+}
+
+static void skcipher_sock_destruct(struct sock *sk)
+{
+ skcipher_sock_destruct_common(sk);
af_alg_release_parent(sk);
}
-static int skcipher_accept_parent(void *private, struct sock *sk)
+static void skcipher_release_parent_nokey(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+
+ if (!ask->refcnt) {
+ sock_put(ask->parent);
+ return;
+ }
+
+ af_alg_release_parent(sk);
+}
+
+static void skcipher_sock_destruct_nokey(struct sock *sk)
+{
+ skcipher_sock_destruct_common(sk);
+ skcipher_release_parent_nokey(sk);
+}
+
+static int skcipher_accept_parent_common(void *private, struct sock *sk)
{
struct skcipher_ctx *ctx;
struct alg_sock *ask = alg_sk(sk);
@@ -827,9 +943,6 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
struct crypto_ablkcipher *skcipher = tfm->skcipher;
unsigned int len = sizeof(*ctx) + crypto_ablkcipher_reqsize(skcipher);
- if (!tfm->has_key)
- return -ENOKEY;
-
ctx = sock_kmalloc(sk, len, GFP_KERNEL);
if (!ctx)
return -ENOMEM;
@@ -863,12 +976,38 @@ static int skcipher_accept_parent(void *private, struct sock *sk)
return 0;
}
+static int skcipher_accept_parent(void *private, struct sock *sk)
+{
+ struct skcipher_tfm *tfm = private;
+
+ if (!tfm->has_key)
+ return -ENOKEY;
+
+ return skcipher_accept_parent_common(private, sk);
+}
+
+static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
+{
+ int err;
+
+ err = skcipher_accept_parent_common(private, sk);
+ if (err)
+ goto out;
+
+ sk->sk_destruct = skcipher_sock_destruct_nokey;
+
+out:
+ return err;
+}
+
static const struct af_alg_type algif_type_skcipher = {
.bind = skcipher_bind,
.release = skcipher_release,
.setkey = skcipher_setkey,
.accept = skcipher_accept_parent,
+ .accept_nokey = skcipher_accept_parent_nokey,
.ops = &algif_skcipher_ops,
+ .ops_nokey = &algif_skcipher_ops_nokey,
.name = "skcipher",
.owner = THIS_MODULE
};
--
2.7.0
From: Herbert Xu <[email protected]>
commit 1822793a523e5d5730b19cc21160ff1717421bc8 upstream.
We need to lock the child socket in skcipher_check_key as otherwise
two simultaneous calls can cause the parent socket to be freed.
Cc: [email protected]
Signed-off-by: Herbert Xu <[email protected]>
---
crypto/algif_skcipher.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 83bcf75..c0f0356 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -757,22 +757,23 @@ static struct proto_ops algif_skcipher_ops = {
static int skcipher_check_key(struct socket *sock)
{
- int err;
+ int err = 0;
struct sock *psk;
struct alg_sock *pask;
struct skcipher_tfm *tfm;
struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);
+ lock_sock(sk);
if (ask->refcnt)
- return 0;
+ goto unlock_child;
psk = ask->parent;
pask = alg_sk(ask->parent);
tfm = pask->private;
err = -ENOKEY;
- lock_sock(psk);
+ lock_sock_nested(psk, SINGLE_DEPTH_NESTING);
if (!tfm->has_key)
goto unlock;
@@ -786,6 +787,8 @@ static int skcipher_check_key(struct socket *sock)
unlock:
release_sock(psk);
+unlock_child:
+ release_sock(sk);
return err;
}
--
2.7.0
On 02/26/2016 06:25 AM, Milan Broz wrote:
> On 02/24/2016 06:12 PM, Greg KH wrote:
>> On Wed, Feb 24, 2016 at 09:54:48AM +0100, Milan Broz wrote:
>>> On 02/24/2016 09:32 AM, Jiri Slaby wrote:
>>>>> + af_alg_release_parent(sk);
>>>>
>>>> and this occurs to me like a double release?
>>>
>>> yes, my copy&paste mistake.
>>
>> Which is why I want the real patches backported please. Whenever we do
>> a "just this smaller patch" for a stable kernel, it is ALWAYS wrong.
>
> I think that it was clear that I do not want you to directly include
> this patch, just it points to the direction where is the problem.
>
> Anyway, seems the problem is only in 4.1.18.
>
>> Please backport the patches in a correct way so that we can apply
>> them...
>
> Not sure if it is still needed, but I'll reply to this thread with my git version
> of backported patches for 4.1.18.
> (Resp. only the first need changes, other then applied cleanly from upstream).
Please do.
Thanks,
Sasha
On Fri, Feb 26, 2016 at 12:44:08PM +0100, Milan Broz wrote:
> From: Herbert Xu <[email protected]>
>
> commit dd504589577d8e8e70f51f997ad487a4cb6c026f upstream.
>
> 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.
>
> Cc: [email protected]
> Reported-by: Dmitry Vyukov <[email protected]>
> Signed-off-by: Herbert Xu <[email protected]>
> Tested-by: Dmitry Vyukov <[email protected]>
> [backported to 4.1 by Milan Broz <[email protected]>]
Ack to all four patches.
Thanks Milan!
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On 02/26/2016 06:44 AM, Milan Broz wrote:
> From: Herbert Xu <[email protected]>
>
> commit dd504589577d8e8e70f51f997ad487a4cb6c026f upstream.
>
> 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.
>
> Cc: [email protected]
> Reported-by: Dmitry Vyukov <[email protected]>
> Signed-off-by: Herbert Xu <[email protected]>
> Tested-by: Dmitry Vyukov <[email protected]>
> [backported to 4.1 by Milan Broz <[email protected]>]
Thanks Milan. Can I add your Signed-off-by to this series?
Thanks,
Sasha
On 02/27/2016 10:40 PM, Sasha Levin wrote:
> On 02/26/2016 06:44 AM, Milan Broz wrote:
>> From: Herbert Xu <[email protected]>
>>
>> commit dd504589577d8e8e70f51f997ad487a4cb6c026f upstream.
>>
>> 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.
>>
>> Cc: [email protected]
>> Reported-by: Dmitry Vyukov <[email protected]>
>> Signed-off-by: Herbert Xu <[email protected]>
>> Tested-by: Dmitry Vyukov <[email protected]>
>> [backported to 4.1 by Milan Broz <[email protected]>]
>
> Thanks Milan. Can I add your Signed-off-by to this series?
yes,
Signed-off-by: Milan Broz <[email protected]>
Thanks,
Milan
Hi,
Sasha, can you please revert commit
f857638dd72680e2a8faafef7eebb4534cb39fd1 like Greg did with linux-3.10.101
> commit 1f2493fcd87bd810c608aa7976388157852eadb2
> Author: Greg Kroah-Hartman <[email protected]>
> Date: Sat Mar 12 21:30:16 2016 -0800
>
> Revert: "crypto: af_alg - Disallow bind/setkey/... after accept(2)"
>
> This reverts commit 5a707f0972e1c9d8a4a921ddae79d0f9dc36a341 which is
> commit c840ac6af3f8713a71b4d2363419145760bd6044 upstream.
>
> It's been widely reported that this patch breaks existing userspace
> applications when backported to the stable kernel releases. As no fix
> seems to be forthcoming, just revert it to let systems work again.
and linux-3.14.65
> commit c4eb62da6f34bfa9bbcbd005210a90fdfca7e367
> Author: Greg Kroah-Hartman <[email protected]>
> Date: Sat Mar 12 21:30:16 2016 -0800
>
> Revert: "crypto: af_alg - Disallow bind/setkey/... after accept(2)"
>
> This reverts commit 06b4194533ff92ed5888840e3a6beaf29a8fe5d4 which is
> commit c840ac6af3f8713a71b4d2363419145760bd6044 upstream.
>
> It's been widely reported that this patch breaks existing userspace
> applications when backported to the stable kernel releases. As no fix
> seems to be forthcoming, just revert it to let systems work again.
Linux-3.18.x is the only LTS kernel left with this problem. If nobody
cares we should at least revert back to a working kernel...
-Thomas
On 04/17/2016 06:17 PM, Thomas D. wrote:
> Hi,
>
> Sasha, can you please revert commit
> f857638dd72680e2a8faafef7eebb4534cb39fd1 like Greg did with linux-3.10.101
>
>> commit 1f2493fcd87bd810c608aa7976388157852eadb2
>> Author: Greg Kroah-Hartman <[email protected]>
>> Date: Sat Mar 12 21:30:16 2016 -0800
>>
>> Revert: "crypto: af_alg - Disallow bind/setkey/... after accept(2)"
>>
>> This reverts commit 5a707f0972e1c9d8a4a921ddae79d0f9dc36a341 which is
>> commit c840ac6af3f8713a71b4d2363419145760bd6044 upstream.
>>
>> It's been widely reported that this patch breaks existing userspace
>> applications when backported to the stable kernel releases. As no fix
>> seems to be forthcoming, just revert it to let systems work again.
>
> and linux-3.14.65
>
>> commit c4eb62da6f34bfa9bbcbd005210a90fdfca7e367
>> Author: Greg Kroah-Hartman <[email protected]>
>> Date: Sat Mar 12 21:30:16 2016 -0800
>>
>> Revert: "crypto: af_alg - Disallow bind/setkey/... after accept(2)"
>>
>> This reverts commit 06b4194533ff92ed5888840e3a6beaf29a8fe5d4 which is
>> commit c840ac6af3f8713a71b4d2363419145760bd6044 upstream.
>>
>> It's been widely reported that this patch breaks existing userspace
>> applications when backported to the stable kernel releases. As no fix
>> seems to be forthcoming, just revert it to let systems work again.
>
>
> Linux-3.18.x is the only LTS kernel left with this problem. If nobody
> cares we should at least revert back to a working kernel...
So I mixed stuff up here, I've received a backport that would fix this problem
on 4.1, and applied it. However, I forgot about 3.18.
Would Milan's backport work on 3.18 as well (https://www.mail-archive.com/[email protected]/msg17949.html)?
Thanks,
Sasha
On Sun, Apr 17, 2016 at 06:39:18PM -0400, Sasha Levin wrote:
>
> So I mixed stuff up here, I've received a backport that would fix this problem
> on 4.1, and applied it. However, I forgot about 3.18.
>
> Would Milan's backport work on 3.18 as well (https://www.mail-archive.com/[email protected]/msg17949.html)?
It should work.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Hi,
Milan's patches apply against 3.18.30, however I am getting build errors:
> * CC [M] fs/fat/namei_vfat.o
> * CC [M] crypto/algif_hash.o
> * LD [M] fs/isofs/isofs.o
> * CC [M] crypto/algif_skcipher.o
> * LD [M] fs/fat/fat.o
> *crypto/algif_hash.c:350:13: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
> * .sendmsg = hash_sendmsg_nokey,
> * ^
> *crypto/algif_hash.c:350:13: note: (near initialization for ?algif_hash_ops_nokey.sendmsg?)
> *crypto/algif_hash.c:352:13: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
> *--
> *crypto/algif_hash.c:352:13: note: (near initialization for ?algif_hash_ops_nokey.recvmsg?)
> * CC [M] drivers/acpi/fan.o
> * CC [M] crypto/xor.o
> * LD [M] fs/fat/msdos.o
> *crypto/algif_skcipher.c: In function ?skcipher_sendmsg_nokey?:
> *crypto/algif_skcipher.c:599:26: warning: passing argument 1 of ?skcipher_sendmsg? from incompatible pointer type [-Wincompatible-pointer-types]
> * return skcipher_sendmsg(sock, msg, size);
> * ^
> *crypto/algif_skcipher.c:247:12: note: expected ?struct kiocb *? but argument is of type ?struct socket *?
> * static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
> * ^
> *crypto/algif_skcipher.c:599:32: warning: passing argument 2 of ?skcipher_sendmsg? from incompatible pointer type [-Wincompatible-pointer-types]
> * return skcipher_sendmsg(sock, msg, size);
> * ^
> *crypto/algif_skcipher.c:247:12: note: expected ?struct socket *? but argument is of type ?struct msghdr *?
> * static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
> * ^
> *crypto/algif_skcipher.c:599:37: warning: passing argument 3 of ?skcipher_sendmsg? makes pointer from integer without a cast [-Wint-conversion]
> * return skcipher_sendmsg(sock, msg, size);
> * ^
> *crypto/algif_skcipher.c:247:12: note: expected ?struct msghdr *? but argument is of type ?size_t {aka long unsigned int}?
> * static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
> * ^
> *crypto/algif_skcipher.c:599:9: error: too few arguments to function ?skcipher_sendmsg?
> *--
> * ^
> *crypto/algif_skcipher.c:247:12: note: declared here
> * static int skcipher_sendmsg(struct kiocb *unused, struct socket *sock,
> * ^
> *crypto/algif_skcipher.c: In function ?skcipher_recvmsg_nokey?:
> *crypto/algif_skcipher.c:623:26: warning: passing argument 1 of ?skcipher_recvmsg? from incompatible pointer type [-Wincompatible-pointer-types]
> * return skcipher_recvmsg(sock, msg, ignored, flags);
> * ^
> *crypto/algif_skcipher.c:426:12: note: expected ?struct kiocb *? but argument is of type ?struct socket *?
> * static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
> * ^
> *crypto/algif_skcipher.c:623:32: warning: passing argument 2 of ?skcipher_recvmsg? from incompatible pointer type [-Wincompatible-pointer-types]
> * return skcipher_recvmsg(sock, msg, ignored, flags);
> * ^
> *crypto/algif_skcipher.c:426:12: note: expected ?struct socket *? but argument is of type ?struct msghdr *?
> * static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
> * ^
> *crypto/algif_skcipher.c:623:37: warning: passing argument 3 of ?skcipher_recvmsg? makes pointer from integer without a cast [-Wint-conversion]
> * return skcipher_recvmsg(sock, msg, ignored, flags);
> * ^
> *crypto/algif_skcipher.c:426:12: note: expected ?struct msghdr *? but argument is of type ?size_t {aka long unsigned int}?
> * static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
> * ^
> *crypto/algif_skcipher.c:623:9: error: too few arguments to function ?skcipher_recvmsg?
> *--
> * ^
> *crypto/algif_skcipher.c:426:12: note: declared here
> * static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
> * ^
> *crypto/algif_skcipher.c: At top level:
> *crypto/algif_skcipher.c:642:13: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
> * .sendmsg = skcipher_sendmsg_nokey,
> * ^
> *crypto/algif_skcipher.c:642:13: note: (near initialization for ?algif_skcipher_ops_nokey.sendmsg?)
> *crypto/algif_skcipher.c:644:13: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
> * .recvmsg = skcipher_recvmsg_nokey,
> * ^
> *crypto/algif_skcipher.c:644:13: note: (near initialization for ?algif_skcipher_ops_nokey.recvmsg?)
> *crypto/algif_skcipher.c: In function ?skcipher_recvmsg_nokey?:
> *crypto/algif_skcipher.c:624:1: warning: control reaches end of non-void function [-Wreturn-type]
> * }
> * ^
> *crypto/algif_skcipher.c: In function ?skcipher_sendmsg_nokey?:
> * CC [M] crypto/async_tx/async_tx.o
> *crypto/algif_skcipher.c:600:1: warning: control reaches end of non-void function [-Wreturn-type]
> * }
> * ^
> * LD [M] fs/fat/vfat.o
> *scripts/Makefile.build:263: recipe for target 'crypto/algif_skcipher.o' failed
> *make[1]: *** [crypto/algif_skcipher.o] Error 1
> *--
> * CC [M] arch/x86/oprofile/../../../drivers/oprofile/buffer_sync.o
> * CC [M] drivers/i2c/busses/i2c-piix4.o
> * CC [M] arch/x86/oprofile/../../../drivers/oprofile/event_buffer.o
> * CC [M] arch/x86/oprofile/../../../drivers/oprofile/oprofile_files.o
> * CC [M] arch/x86/oprofile/../../../drivers/oprofile/oprofile_stats.o
> *Makefile:937: recipe for target 'crypto' failed
> *make: *** [crypto] Error 2
-Thomas
On 04/18/2016 05:48 AM, Thomas D. wrote:
> Hi,
>
> Milan's patches apply against 3.18.30, however I am getting build errors:
Milan, Herbert, should I just be reverting the offending patches:
bcfa841 crypto: af_alg - Forbid bind(2) when nokey child sockets are present
eb1ab00 crypto: af_alg - Allow af_af_alg_release_parent to be called on nokey path
3db68eb crypto: af_alg - Add nokey compatibility path
ac9c75f crypto: af_alg - Fix socket double-free when accept fails
f857638 crypto: af_alg - Disallow bind/setkey/... after accept(2)
Or will you send me a backport?
On 04/18/2016 02:54 PM, Sasha Levin wrote:
> On 04/18/2016 05:48 AM, Thomas D. wrote:
>> Hi,
>>
>> Milan's patches apply against 3.18.30, however I am getting build errors:
>
> Milan, Herbert, should I just be reverting the offending patches:
>
> bcfa841 crypto: af_alg - Forbid bind(2) when nokey child sockets are present
> eb1ab00 crypto: af_alg - Allow af_af_alg_release_parent to be called on nokey path
> 3db68eb crypto: af_alg - Add nokey compatibility path
> ac9c75f crypto: af_alg - Fix socket double-free when accept fails
> f857638 crypto: af_alg - Disallow bind/setkey/... after accept(2)
>
> Or will you send me a backport?
Hi,
could you please try backported patches here
https://mbroz.fedorapeople.org/tmp/3.18/ ?
It should be the same backport as I sent for 4.1 here (just with some 3.18 socket specifics).
("cryptsetup benchmark" works again and that command uses this API.)
I think similar patches should be in 3.12 stable tree already.
Thanks,
Milan
Hi,
Milan Broz wrote:
> could you please try backported patches here
> https://mbroz.fedorapeople.org/tmp/3.18/ ?
This patch set works for me and fixes my reported problem (tested
against 3.18.30).
Thank you!
-Thomas
On 04/18/2016 04:56 PM, Thomas D. wrote:
> Hi,
>
> Milan Broz wrote:
>> could you please try backported patches here
>> https://mbroz.fedorapeople.org/tmp/3.18/ ?
>
> This patch set works for me and fixes my reported problem (tested
> against 3.18.30).
>
> Thank you!
Excellent, I'll add this in and release. Thank you both.
Sorry for missing it on my end.
Thanks,
Sasha