Re: [PATCH qemu 01/10] hw/cxl: Check size of input data to dynamic capacity mailbox commands

[Fan Ni <nifan.cxl@gmail.com>]

On Fri, Nov 01, 2024 at 01:39:08PM +0000, Jonathan Cameron wrote:
> cxl_cmd_dcd_release_dyn_cap() and cmd_dcd_add_dyn_cap_rsp() are missing
> input message size checks.  These must be done in the individual
> commands when the command has a variable length input payload.
> 
> A buggy or malicious guest might send undersized messages via the mailbox.
> As that size is used to take a copy of the mailbox content, each command
> must check there is sufficient data. In this case the first check is that
> there is enough data to read how many extents there are, and the second
> that there is enough for those elements to be accessed.
> 
> 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 | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 97cb8bbcec..17924410dd 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -2465,11 +2465,20 @@ static CXLRetCode cmd_dcd_add_dyn_cap_rsp(const struct cxl_cmd *cmd,
>      uint64_t dpa, len;
>      CXLRetCode ret;
>  
> +    if (len_in < sizeof(*in)) {
> +        return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
> +    }
> +
>      if (in->num_entries_updated == 0) {
>          cxl_extent_group_list_delete_front(&ct3d->dc.extents_pending);
>          return CXL_MBOX_SUCCESS;
>      }
>  
> +    if (len_in <
> +        sizeof(*in) + sizeof(*in->updated_entries) * in->num_entries_updated) {
> +        return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
> +    }
> +
>      /* Adding extents causes exceeding device's extent tracking ability. */
>      if (in->num_entries_updated + ct3d->dc.total_extent_count >
>          CXL_NUM_EXTENTS_SUPPORTED) {
> @@ -2624,10 +2633,19 @@ static CXLRetCode cmd_dcd_release_dyn_cap(const struct cxl_cmd *cmd,
>      uint32_t updated_list_size;
>      CXLRetCode ret;
>  
> +    if (len_in < sizeof(*in)) {
> +        return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
> +    }
> +
>      if (in->num_entries_updated == 0) {
>          return CXL_MBOX_INVALID_INPUT;
>      }
>  
> +    if (len_in <
> +        sizeof(*in) + sizeof(*in->updated_entries) * in->num_entries_updated) {
> +        return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
> +    }
> +
>      ret = cxl_detect_malformed_extent_list(ct3d, in);
>      if (ret != CXL_MBOX_SUCCESS) {
>          return ret;
> -- 
> 2.43.0
> 

-- 
Fan Ni