| Message ID | 56C9E865.3050500@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
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 > }; > > -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,
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 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Sasha, can you please revert commit f857638dd72680e2a8faafef7eebb4534cb39fd1 like Greg did with linux-3.10.101 > commit 1f2493fcd87bd810c608aa7976388157852eadb2 > Author: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > 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 <gregkh@linuxfoundation.org> > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <gregkh@linuxfoundation.org> >> 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 <gregkh@linuxfoundation.org> >> 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/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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,
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 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- 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 };