Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200629073925.127538-1-ardb@kernel.org> <20200629073925.127538-6-ardb@kernel.org>
 <20200709082200.GA1892@gondor.apana.org.au>
In-Reply-To: <20200709082200.GA1892@gondor.apana.org.au>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Thu, 9 Jul 2020 11:51:10 +0300
Message-ID: <CAMj1kXE8HELm1j3jx-+mHrK3OjG6Rjp4jtP_QEYorRBnRxA+=w@mail.gmail.com>
Subject: Re: [PATCH 5/5] crypto: arm/ghash - use variably sized key struct
To:     Herbert Xu <herbert@gondor.apana.org.au>
Cc:     Linux Crypto Mailing List <linux-crypto@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Thu, 9 Jul 2020 at 11:22, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Mon, Jun 29, 2020 at 09:39:25AM +0200, Ard Biesheuvel wrote:
> > Of the two versions of GHASH that the ARM driver implements, only one
> > performs aggregation, and so the other one has no use for the powers
> > of H to be precomputed, or space to be allocated for them in the key
> > struct. So make the context size dependent on which version is being
> > selected, and while at it, use a static key to carry this decision,
> > and get rid of the function pointer.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  arch/arm/crypto/ghash-ce-glue.c | 51 +++++++++-----------
> >  1 file changed, 24 insertions(+), 27 deletions(-)
>
> This introduces some new sparse warnings:
>
> ../arch/arm/crypto/ghash-ce-glue.c:67:65: warning: incorrect type in argument 4 (different modifiers)
> ../arch/arm/crypto/ghash-ce-glue.c:67:65:    expected unsigned long long const [usertype] ( *h )[2]
> ../arch/arm/crypto/ghash-ce-glue.c:67:65:    got unsigned long long [usertype] ( * )[2]
> ../arch/arm/crypto/ghash-ce-glue.c:69:64: warning: incorrect type in argument 4 (different modifiers)
> ../arch/arm/crypto/ghash-ce-glue.c:69:64:    expected unsigned long long const [usertype] ( *h )[2]
> ../arch/arm/crypto/ghash-ce-glue.c:69:64:    got unsigned long long [usertype] ( * )[2]
>


That looks like a sparse bug to me. Since when is it not allowed to
pass a non-const value as a const parameter?

I.e., you can pass a u64[] to a function that takes a u64 const *,
giving the caller the guarantee that their u64[] will not be modified
during the call, even if it is passed by reference.

Here, we are dealing with u64[][2], but the same reasoning holds. A
const u64[][2] formal parameter (or u64 const (*)[2] which comes down
to the same thing) does not require a const argument, it only tells
the caller that the array will be left untouched. This is why the
compiler is perfectly happy with this arrangement.