Re: [PATCH v2 17/19] Documentation: ioctl: add CMH ioctl documentation and register 'J'

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Introduction of a vendor-specific custom ioctl UAPI for standard cryptographic operations, with a stated plan to deprecate it later, violating the 'UAPI is forever' kernel principle.
- [Low] The documentation omits the C structure definitions for numerous PKE, PQC, and SM2 ioctl commands, rendering the UAPI reference incomplete.
--

--- Patch [17]: [PATCH v2 17/19] Documentation: ioctl: add CMH ioctl documentation and register 'J' ---
commit e840789907bd334a011304ecafd76c3e8f7309d6
Author: Alex Ousherovitch <aousherovitch@rambus.com>

Documentation: ioctl: add CMH ioctl documentation and register 'J'

This patch adds documentation for the CRI CryptoManager Hub (CMH) driver's
ioctl interface. It covers key management, KIC key derivation, PKE, PQC,
and SM2 operations, and registers the ioctl magic number 'J'.

> diff --git a/Documentation/userspace-api/ioctl/cmh_mgmt.rst b/Documentation/userspace-api/ioctl/cmh_mgmt.rst
> new file mode 100644
> index 0000000000000..b0968ba6b1533
> --- /dev/null
> +++ b/Documentation/userspace-api/ioctl/cmh_mgmt.rst

[ ... ]

> +RSA Operations
> +--------------
> +
> +CMH_IOCTL_PKE_RSA_ENC
> +~~~~~~~~~~~~~~~~~~~~~~
> +
> +RSA public-key encryption.
> +
> +:Direction: ``_IOWR``
> +:Number: 0x10
> +:Argument: ``struct cmh_ioctl_pke_rsa_enc``
> +
> +The public key (e, n) is passed as raw user-space buffers.
> +

[Severity: Low]
Is it intentional that the UAPI C structure definitions are omitted starting
here?

Unlike the earlier key management ioctls in this document, this section and
the following cryptographic operations (PKE, PQC, SM2) reference an argument
structure (such as struct cmh_ioctl_pke_rsa_enc) but do not provide its
layout.

Would it make sense to include the C structure definitions for these ioctls?
Having them in the documentation allows reviewers to easily verify ABI
details like padding and 32-bit/64-bit compatibility.

[ ... ]

> +Migration Plan
> +==============
> +
> +Several ioctl commands provide operations that may gain dedicated kernel
> +crypto API bindings in the future.  When those APIs land, the driver will
> +register through them and the corresponding ioctls will be deprecated
> +(retained for backward compatibility but no longer the primary interface):

[Severity: High]
Could this proposed migration plan conflict with the kernel's guarantee that
userspace ABIs are permanent?

By introducing vendor-specific ioctls for standard cryptographic operations
with the explicit intent to deprecate them later, this establishes a
permanent, redundant custom crypto ABI that the kernel will have to
maintain indefinitely.

Should these operations be delayed until they can integrate with existing
kernel abstractions, or propose extensions to them, rather than implementing
temporary ioctl backdoors?

> +
> +- **EdDSA** (``CMH_IOCTL_PKE_EDDSA_*``): will migrate to the kernel ``sig``
> +  API once ed25519/ed448 algorithm types are accepted upstream.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709203037.1884436-1-skrishnamoorthy@rambus.com?part=17