Re: [PATCH qemu 05/10] hw/cxl: Check the length of data requested fits in get_log()

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

On Fri, Nov 01, 2024 at 01:39:12PM +0000, Jonathan Cameron wrote:
> Checking offset + length is of no relevance when verifying the CEL
> data will fit in the mailbox payload. Only the length is is relevant.
s/is is/is/
> 
> Note that this removes a potential overflow.
> 
> Reported-by: Esifiel <esifiel@gmail.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  hw/cxl/cxl-mailbox-utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 27fadc4fa8..2aa7ffed84 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -947,7 +947,7 @@ static CXLRetCode cmd_logs_get_log(const struct cxl_cmd *cmd,
>       * the only possible failure would be if the mailbox itself isn't big
>       * enough.
>       */
> -    if (get_log->offset + get_log->length > cci->payload_max) {
> +    if (get_log->length > cci->payload_max) {

If offset is beyond the size of cel_log, will it be a problem?

There is a comment just above saying "
 * 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.
 "

Not sure how it avoids the case when the offset is too large.

Fan

>          return CXL_MBOX_INVALID_INPUT;
>      }
>  
> -- 
> 2.43.0
> 

-- 
Fan Ni