Re: [PATCH v9 5/5] s390/zcrypt: Pad trailing CCA or EP11 message with zeros

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but an integer overflow and incomplete bounds checking in hardware response parsing allows a massive out-of-bounds kernel memory leak to userspace.
--

--- Patch [5]: [PATCH v9 5/5] s390/zcrypt: Pad trailing CCA or EP11 message with zeros ---
commit 0141bfa352999fc87f7b7c58c7c727f991c4b416
Author: Harald Freudenberger <[email protected]>

s390/zcrypt: Pad trailing CCA or EP11 message with zeros

This patch fixes an information leak to the crypto card firmware. It zeroes
out the gap between the user space copied message and the message buffer
length sent to the CCA or EP11 message processing.

> diff --git a/drivers/s390/crypto/zcrypt_msgtype6.c b/drivers/s390/crypto/zcrypt_msgtype6.c
> index da7580933c6f2..4a60540b07003 100644
> --- a/drivers/s390/crypto/zcrypt_msgtype6.c
> +++ b/drivers/s390/crypto/zcrypt_msgtype6.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed an integer overflow and
incomplete bounds checking in hardware response parsing that could allow an
out-of-bounds kernel memory leak to userspace.

In zcrypt_msgtype6_receive():

        if (t86r->fmt2.count2)
            len = t86r->fmt2.offset2 + t86r->fmt2.count2;
        else
            len = t86r->fmt2.offset1 + t86r->fmt2.count1;

If a compromised or malicious crypto device firmware returns a response where
offset1 + count1 overflows 32-bit arithmetic, it might bypass the buffer
size check. Alternatively, if count2 is non-zero, the bounds check in
zcrypt_msgtype6_receive() only validates offset2 + count2 and completely
ignores offset1 + count1.

Later in convert_type86_xcrb():

    if (z_copy_to_user(userspace, xcrb->reply_control_blk_addr,
               data + msg->fmt2.offset1, msg->fmt2.count1))
        return -EFAULT;

Since the bounds check can pass under those conditions, could offset1 be
used as an unvalidated offset into kernel memory here, resulting in
z_copy_to_user() copying out-of-bounds kernel memory into the user-provided
buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5