Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 9 May 2022 14:56:31 -0700
From:   Eric Biggers <ebiggers@kernel.org>
To:     Nathan Huckleberry <nhuck@google.com>
Cc:     linux-crypto@vger.kernel.org, linux-fscrypt@vger.kernel.org,
        Herbert Xu <herbert@gondor.apana.org.au>,
        "David S. Miller" <davem@davemloft.net>,
        linux-arm-kernel@lists.infradead.org,
        Paul Crowley <paulcrowley@google.com>,
        Sami Tolvanen <samitolvanen@google.com>,
        Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH v7 6/9] crypto: arm64/aes-xctr: Improve readability of
 XCTR and CTR modes
Message-ID: <YnmOD+bzabjoxaEN@sol.localdomain>
References: <20220509191107.3556468-1-nhuck@google.com>
 <20220509191107.3556468-7-nhuck@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220509191107.3556468-7-nhuck@google.com>
Precedence: bulk

On Mon, May 09, 2022 at 07:11:04PM +0000, Nathan Huckleberry wrote:
> Added some clarifying comments, changed the register allocations to make
> the code clearer, and added register aliases.
> 
> Signed-off-by: Nathan Huckleberry <nhuck@google.com>
> Reviewed-by: Eric Biggers <ebiggers@google.com>

Did you mean to add Ard's Reviewed-by that he gave on v6 as well?

One comment about the v7 changes below:

>  	/*
>  	 * aes_ctr_encrypt(u8 out[], u8 const in[], u8 const rk[], int rounds,
>  	 *		   int bytes, u8 ctr[])
> +	 *
> +	 * The input and output buffers must always be at least 16 bytes even if
> +	 * encrypting/decrypting less than 16 bytes.  Otherwise out of bounds
> +	 * accesses will occur.
>  	 */

This comment, along with the other similar ones you added, doesn't properly
describe the behavior when bytes < 16, as it's not mentioned that the extra
space needs to be before the pointed-to regions rather than after.  That's the
most unusual part of these functions, so it really should be mentioned.

Separately, applying this patch and the previous one causes the following
whitespace errors to be reported:

Applying: crypto: arm64/aes-xctr: Add accelerated implementation of XCTR
.git/rebase-apply/patch:299: space before tab in indent.
        ld1             {v5.16b-v7.16b}, [x1], #48
warning: 1 line adds whitespace errors.
Applying: crypto: arm64/aes-xctr: Improve readability of XCTR and CTR modes
.git/rebase-apply/patch:216: space before tab in indent.
        ld1             {v5.16b-v7.16b}, [IN], #48
warning: 1 line adds whitespace errors.

- Eric