linux-kernel - Re: [v2 PATCH] crypto: af_alg - Work around empty control messages without MSG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 27 Aug 2020 09:40:01 +0200
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Herbert Xu <herbert@...dor.apana.org.au>
Cc:     Denis Kenzior <denkenz@...il.com>,
        Andrew Zaborowski <andrew.zaborowski@...el.com>,
        Paul Menzel <pmenzel@...gen.mpg.de>,
        Caleb Jorden <caljorden@...mail.com>,
        Sasha Levin <sashal@...nel.org>, iwd@...ts.01.org,
        "# 3.4.x" <stable@...r.kernel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>
Subject: Re: [v2 PATCH] crypto: af_alg - Work around empty control messages
 without MSG_MORE

On Thu, 27 Aug 2020 at 09:15, Herbert Xu <herbert@...dor.apana.org.au> wrote:
>
> On Thu, Aug 27, 2020 at 08:40:01AM +0200, Ard Biesheuvel wrote:
> >
> > It is part of iwd - just build that and run 'make check'
> >
> > With your patch applied, the occurrence of sendmsg() in
> > operate_cipher() triggers the warn_once(), but if I add MSG_MORE
> > there, the test hangs.
>
> I see.  This is a different issue.  The original kernel change
> was a bit too strict here and it is barfing at the fact that two
> successive sendmsg's of the same request both contain a control
> message.
>
> Here's an updated patch to allow this.
>
> ---8<---
> The iwd daemon uses libell which sets up the skcipher operation with
> two separate control messages.  As the first control message is sent
> without MSG_MORE, it is interpreted as an empty request.
>
> While libell should be fixed to use MSG_MORE where appropriate, this
> patch works around the bug in the kernel so that existing binaries
> continue to work.
>
> We will print a warning however.
>
> A separate issue is that the new kernel code no longer allows the
> control message to be sent twice within the same request.  This
> restriction is obviously incompatible with what iwd was doing (first
> setting an IV and then sending the real control message).  This
> patch changes the kernel so that this is explicitly allowed.
>
> Reported-by: Caleb Jorden <caljorden@...mail.com>
> Fixes: f3c802a1f300 ("crypto: algif_aead - Only wake up when...")
> Cc: <stable@...r.kernel.org>
> Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>
>
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index a6f581ab200c..8be8bec07cdd 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -16,6 +16,7 @@
>  #include <linux/module.h>
>  #include <linux/net.h>
>  #include <linux/rwsem.h>
> +#include <linux/sched.h>
>  #include <linux/sched/signal.h>
>  #include <linux/security.h>
>
> @@ -845,9 +846,15 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size,
>         }
>
>         lock_sock(sk);
> -       if (ctx->init && (init || !ctx->more)) {
> -               err = -EINVAL;
> -               goto unlock;
> +       if (ctx->init && !ctx->more) {
> +               if (ctx->used) {
> +                       err = -EINVAL;
> +                       goto unlock;
> +               }
> +
> +               pr_info_once(
> +                       "%s sent an empty control message without MSG_MORE.\n",
> +                       current->comm);
>         }
>         ctx->init = true;
>

Yep, that works.