hw/cxl: Mailbox input parser hardening against invalid input.

[PATCH qemu 04/10] hw/cxl: Check enough data in cmd_firmware_update_transfer()

Posted by Jonathan Cameron via 3 weeks, 1 day ago

Buggy guest can write a message that advertises more data that
is provided. As QEMU internally duplicates the reported message
size, this may result in an out of bounds access.
Add sanity checks on the size to avoid this.

Reported-by: Esifiel <esifiel@gmail.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 hw/cxl/cxl-mailbox-utils.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 3cb499a24f..27fadc4fa8 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -705,6 +705,10 @@ static CXLRetCode cmd_firmware_update_transfer(const struct cxl_cmd *cmd,
     } QEMU_PACKED *fw_transfer = (void *)payload_in;
     size_t offset, length;
 
+    if (len < sizeof(*fw_transfer)) {
+        return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
+    }
+
     if (fw_transfer->action == CXL_FW_XFER_ACTION_ABORT) {
         /*
          * At this point there aren't any on-going transfers
-- 
2.43.0

Re: [PATCH qemu 04/10] hw/cxl: Check enough data in cmd_firmware_update_transfer()

Posted by Fan Ni 2 weeks, 3 days ago

On Fri, Nov 01, 2024 at 01:39:11PM +0000, Jonathan Cameron wrote:
> Buggy guest can write a message that advertises more data that
> is provided. As QEMU internally duplicates the reported message
> size, this may result in an out of bounds access.
> Add sanity checks on the size to avoid this.
> 
> Reported-by: Esifiel <esifiel@gmail.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

Reviewed-by: Fan Ni <fan.ni@samsung.com>

>  hw/cxl/cxl-mailbox-utils.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 3cb499a24f..27fadc4fa8 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -705,6 +705,10 @@ static CXLRetCode cmd_firmware_update_transfer(const struct cxl_cmd *cmd,
>      } QEMU_PACKED *fw_transfer = (void *)payload_in;
>      size_t offset, length;
>  
> +    if (len < sizeof(*fw_transfer)) {
> +        return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
> +    }
> +
>      if (fw_transfer->action == CXL_FW_XFER_ACTION_ABORT) {
>          /*
>           * At this point there aren't any on-going transfers
> -- 
> 2.43.0
> 

-- 
Fan Ni