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: <20210610062150.212779-1-ardb@kernel.org> <YMJeZMiRW7Xjo/sZ@gmail.com>
In-Reply-To: <YMJeZMiRW7Xjo/sZ@gmail.com>
From:   Sami Tolvanen <samitolvanen@google.com>
Date:   Thu, 10 Jun 2021 13:39:00 -0700
Message-ID: <CABCJKufOnMPPvsNFh1s3f=3DL24Dh8ShdEMNXx2OJjoGDiBpNQ@mail.gmail.com>
Subject: Re: [PATCH v3] crypto: shash - avoid comparing pointers to exported
 functions under CFI
To:     Eric Biggers <ebiggers@kernel.org>
Cc:     Ard Biesheuvel <ardb@kernel.org>,
        linux-crypto <linux-crypto@vger.kernel.org>,
        Herbert Xu <herbert@gondor.apana.org.au>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jun 10, 2021 at 11:48 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Thu, Jun 10, 2021 at 08:21:50AM +0200, Ard Biesheuvel wrote:
> > crypto_shash_alg_has_setkey() is implemented by testing whether the
> > .setkey() member of a struct shash_alg points to the default version,
> > called shash_no_setkey(). As crypto_shash_alg_has_setkey() is a static
> > inline, this requires shash_no_setkey() to be exported to modules.
> >
> > Unfortunately, when building with CFI, function pointers are routed
> > via CFI stubs which are private to each module (or to the kernel proper)
> > and so this function pointer comparison may fail spuriously.
> >
> > Let's fix this by turning crypto_shash_alg_has_setkey() into an out of
> > line function.
> >
> > Cc: Sami Tolvanen <samitolvanen@google.com>
> > Cc: Eric Biggers <ebiggers@kernel.org>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > v3: improve comment as per Eric's suggestion
> > v2: add code comment to explain why the function needs to remain out of
> > line
> >
> >  crypto/shash.c                 | 18 +++++++++++++++---
> >  include/crypto/internal/hash.h |  8 +-------
> >  2 files changed, 16 insertions(+), 10 deletions(-)
> >
>
> Reviewed-by: Eric Biggers <ebiggers@google.com>

Looks good to me as well. Thank you for fixing this!

Reviewed-by: Sami Tolvanen <samitolvanen@google.com>

Sami