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

[Sasha Levin]

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

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

[Milan Broz]

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

[Sasha Levin]

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

[Herbert Xu]

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/linux-crypto@vger.kernel.org/msg17949.html)?

It should work.

Cheers,
-- 
Email: Herbert Xu 
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

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

[Sasha Levin]

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

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/linux-crypto@vger.kernel.org/msg17949.html)?


Thanks,
Sasha

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

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

[Sasha Levin]

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

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

[Greg KH]

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

[Milan Broz]

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

[Jiri Slaby]

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

[Greg KH]

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
--
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: [PATCH] Re: Broken userspace crypto in linux-4.1.18

[Sasha Levin]

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

[Milan Broz]

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 = 

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

[Milan Broz]

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