On Mon, 29 Jul 2024 15:48:00 +0200
Lukas Wunner <lukas@xxxxxxxxx> wrote:
> Commit 6cb8815f41a9 ("crypto: sig - Add interface for sign/verify")
> introduced an API which accepts kernel buffers instead of sglists for
> signature generation and verification.
>
> Commit 63ba4d67594a ("KEYS: asymmetric: Use new crypto interface without
> scatterlists") converted the sole user in the tree to the new API.
>
> Although the API externally accepts kernel buffers, internally it still
> converts them to sglists, which results in overhead for asymmetric
> algorithms because they need to copy the sglists back into kernel
> buffers.
>
> Take the next step and switch signature verification over to using
> kernel buffers internally, thereby avoiding the sglists overhead.
>
> Because all ->verify implementations are synchronous, forego invocation
> of crypto_akcipher_sync_{prep,post}() and call crypto_akcipher_verify()
> directly from crypto_sig_verify().
>
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
I'm a little out of my depth in this code, but one question did
come to mind. Rather than passing scatter lists and buffers
via the same function void akcipher_request_set_crypt()
why not add a variant that takes buffers for signing cases so
that you can maintain the scatterlist pointers for the encrypt/decrypt
cases?
> diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
> index 18a10cad07aa..2c5bc35d297a 100644
> --- a/include/crypto/akcipher.h
> +++ b/include/crypto/akcipher.h
> @@ -16,28 +16,39 @@
> *
> * @base: Common attributes for async crypto requests
> * @src: Source data
> - * For verify op this is signature + digest, in that case
> - * total size of @src is @src_len + @dst_len.
> - * @dst: Destination data (Should be NULL for verify op)
> + * @dst: Destination data
> * @src_len: Size of the input buffer
> - * For verify op it's size of signature part of @src, this part
> - * is supposed to be operated by cipher.
> - * @dst_len: Size of @dst buffer (for all ops except verify).
> + * @dst_len: Size of @dst buffer
> * It needs to be at least as big as the expected result
> * depending on the operation.
> * After operation it will be updated with the actual size of the
> * result.
> * In case of error where the dst sgl size was insufficient,
> * it will be updated to the size required for the operation.
> - * For verify op this is size of digest part in @src.
> + * @sig: Signature
> + * @digest: Digest
> + * @sig_len: Size of @sig
> + * @digest_len: Size of @digest
> * @__ctx: Start of private context data
> */
> struct akcipher_request {
> struct crypto_async_request base;
> - struct scatterlist *src;
> - struct scatterlist *dst;
> - unsigned int src_len;
> - unsigned int dst_len;
> + union {
> + struct {
> + /* sign, encrypt, decrypt operations */
> + struct scatterlist *src;
> + struct scatterlist *dst;
> + unsigned int src_len;
> + unsigned int dst_len;
> + };
> + struct {
> + /* verify operation */
> + const void *sig;
> + const void *digest;
> + unsigned int sig_len;
> + unsigned int digest_len;
> + };
> + };
> void *__ctx[] CRYPTO_MINALIGN_ATTR;
> };
>
> @@ -242,20 +253,18 @@ static inline void akcipher_request_set_callback(struct akcipher_request *req,
> * Sets parameters required by crypto operation
> *
> * @req: public key request
> - * @src: ptr to input scatter list
> - * @dst: ptr to output scatter list or NULL for verify op
> - * @src_len: size of the src input scatter list to be processed
> - * @dst_len: size of the dst output scatter list or size of signature
> - * portion in @src for verify op
> + * @src: ptr to input scatter list or signature for verify op
> + * @dst: ptr to output scatter list or digest for verify op
> + * @src_len: size of @src
> + * @dst_len: size of @dst
> */
> static inline void akcipher_request_set_crypt(struct akcipher_request *req,
> - struct scatterlist *src,
> - struct scatterlist *dst,
> + const void *src, const void *dst,
Maybe it's worth a 'special' variant.
static inline void akcipher_request_set_crypt_for_verify()
so that you can keep the other list typed and not rely
on overlapping pointer fields via the union.
> unsigned int src_len,
> unsigned int dst_len)
> {
> - req->src = src;
> - req->dst = dst;
> + req->sig = src;
> + req->digest = dst;
> req->src_len = src_len;
> req->dst_len = dst_len;
> }
> @@ -372,10 +381,6 @@ static inline int crypto_akcipher_sign(struct akcipher_request *req)
> *
> * @req: asymmetric key request
> *
> - * Note: req->dst should be NULL, req->src should point to SG of size
> - * (req->src_size + req->dst_size), containing signature (of req->src_size
> - * length) with appended digest (of req->dst_size length).
> - *
> * Return: zero on verification success; error code in case of error.
> */
> static inline int crypto_akcipher_verify(struct akcipher_request *req)
