Re: [PATCH] Re: Broken userspace crypto in linux-4.1.18

[Thomas D.]

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

--
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

Re: [PATCH] Re: Broken userspace crypto in linux-4.1.18

[Thomas D.]

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]
> * }
> * ^
> 

Re: [PATCH] Re: Broken userspace crypto in linux-4.1.18

[Thomas D.]

Hi,

Sasha, can you please revert commit
f857638dd72680e2a8faafef7eebb4534cb39fd1 like Greg did with linux-3.10.101

> commit 1f2493fcd87bd810c608aa7976388157852eadb2
> Author: Greg Kroah-Hartman 
> 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 
> Date:   Sat Mar 12 21:30:16 2016 -0800
> 
> Revert: "crypto: af_alg - Disallow bind/setkey/... after accept(2)"
> 
> This reverts commit 06b4194533ff92ed540e3a6beaf29a8fe5d4 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
--
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

Re: [PATCH] Re: Broken userspace crypto in linux-4.1.18

[Thomas D.]

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
--
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

Re: Broken userspace crypto in linux-4.1.18

[Thomas D.]

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
--
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

Re: Broken userspace crypto in linux-4.1.18

[Thomas D.]

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

--
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

Re: Broken userspace crypto in linux-4.1.18

[Thomas D.]

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

--
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

Re: Broken userspace crypto in linux-4.1.18

[Thomas D.]

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

--
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

Broken userspace crypto in linux-4.1.18

[Thomas D.]

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.172016-02-17 01:40:02.028647072 +0100
> +++ proc_crypto_4.1.182016-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: 
> -
> -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