2023-06-20 07:52:43

by David Howells

[permalink] [raw]
Subject: Re: [PATCH net-next v2] crypto: af_alg/hash: Fix recvmsg() after sendmsg(MSG_MORE)

Herbert Xu <[email protected]> wrote:

> > + hash_free_result(sk, ctx);
>
> Please revert this change as I explained in the other message.
>
> > + if (!msg_data_left(msg))
> > + goto done; /* Zero-length; don't start new req */
>
> This is still broken in the case of a zero-length message with
> MSG_MORE set. Here you will short-circuit out without ever calling
> crypto_ahash_init. However, hash_recvmsg will directly call
> crypto_ahash_final on this, which is undefined.

Not so. hash_recvmsg() will call crypto_ahash_init() first because ctx->more
is false (hence why we came down this branch in hash_sendmsg()) and the result
was released on the previous line (which you're objecting to). If it goes to
the "done" label, it will skip setting ctx->more to true if MSG_MORE is
passed.

However, given you want sendmsg() to do the init->digest cycle on zero length
data, I think we should revert to the previous version of the patch that makes
a pass of the loop even with no data.

David