On Fri, Nov 01, 2024 at 01:39:13PM +0000, Jonathan Cameron wrote:
> Add a check that the requested offset + length does not go beyond the end
> of the cel_log.
>
> Whilst the cci->cel_log is large enough to include all possible CEL
> entries, the guest might still ask for entries beyond the end of it.
> Move the comment to this new check rather than before the check on the
> type of log requested.
>
> Reported-by: Esifiel <esif...@gmail.com>
> Signed-off-by: Jonathan Cameron <jonathan.came...@huawei.com>
> ---
> hw/cxl/cxl-mailbox-utils.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 2aa7ffed84..5e571955b6 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -937,24 +937,28 @@ static CXLRetCode cmd_logs_get_log(const struct cxl_cmd
> *cmd,
>
> get_log = (void *)payload_in;
>
> + if (get_log->length > cci->payload_max) {
> + return CXL_MBOX_INVALID_INPUT;
> + }
> +
> + if (!qemu_uuid_is_equal(&get_log->uuid, &cel_uuid)) {
> + return CXL_MBOX_INVALID_LOG;
> + }
> +
> /*
> * CXL r3.1 Section 8.2.9.5.2: Get Log (Opcode 0401h)
> * The device shall return Invalid Input if the Offset or Length
> * fields attempt to access beyond the size of the log as reported by
> Get
> - * Supported Logs.
> + * Supported Log.
> *
> - * The CEL buffer is large enough to fit all commands in the emulation,
> so
> - * the only possible failure would be if the mailbox itself isn't big
> - * enough.
> + * Only valid for there to be one entry per opcode, but the length +
> offset
> + * may still be greater than that if the inputs are not valid and so
> access
> + * beyond the end of cci->cel_log.
> */
> - if (get_log->length > cci->payload_max) {
> + if ((uint64_t)get_log->offset + get_log->length >= sizeof(cci->cel_log))
> {
> return CXL_MBOX_INVALID_INPUT;
Oh. This patch actually addresses my concern in the previous patch.
Maybe we can combine the changes in this patch and the previous one
together. Other than that
Reviewed-by: Fan Ni <fan...@samsung.com>
> }
>
> - if (!qemu_uuid_is_equal(&get_log->uuid, &cel_uuid)) {
> - return CXL_MBOX_INVALID_LOG;
> - }
> -
> /* Store off everything to local variables so we can wipe out the
> payload */
> *len_out = get_log->length;
>
> --
> 2.43.0
>
--
Fan Ni
