On Tue, 23 Oct 2018, Ard Biesheuvel wrote:

On 23 October 2018 at 04:01, James Bottomley
[off-list ref] wrote:

quoted

On Mon, 2018-10-22 at 19:19 -0300, Ard Biesheuvel wrote:
[...]

quoted

quoted

+static void hmac_init(struct shash_desc *desc, u8 *key, int
keylen)
+{
+       u8 pad[SHA256_BLOCK_SIZE];
+       int i;
+
+       desc->tfm = sha256_hash;
+       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;

I don't think this actually does anything in the shash API
implementation, so you can drop this.

OK, I find crypto somewhat hard to follow.  There were bits I had to
understand, like when I wrote the CFB implementation or when I fixed
the ECDH scatterlist handling, but I've got to confess, in time
honoured tradition I simply copied this from EVM crypto without
actually digging into the code to understand why.

Yeah, it is notoriously hard to use, and we should try to improve that.

James,

I would hope (already said in my review) to use longer than one
character variable names for most of the stuff. I did not quite
understand why you decided to use 'counter' for obvious counter
variable and one character names for non-obvious stuff :-)

I'm not sure where the 'encoded' exactly comes in the variable
name 'encoded_key' especially in the context of these cryptic
names.

/Jarkko