Re: [PATCH v9 2/2] drm/xe/guc: Compress GuC log and CTB dump with zstd

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/xe: infinite loop in zstd_compress_stream if output buffer fills
- [Medium] drm/xe: KASAN out-of-bounds read in xe_print_blob_ascii85 due to unaligned buffer size
--

commit c797cc861f3e71f2974f41eee132c8271bc241da
Author: Zhanjun Dong <zhanjun.dong@intel.com>

drm/xe/guc: Compress GuC log and CTB dump with zstd

This commit replaces the raw ascii85 dumps of GuC log and CTB snapshots with
a zstd-compressed ascii85 stream. It adds the DRM_XE_COMPRESS_DUMP Kconfig
option to control this behavior, aiming to significantly reduce the size of
error dumps.

> diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
> index 538d4df0f7aa7..aaeb0f76af305 100644
> --- a/drivers/gpu/drm/xe/xe_guc_log.c
> +++ b/drivers/gpu/drm/xe/xe_guc_log.c
[ ... ]
> @@ -225,16 +228,148 @@ struct xe_guc_log_snapshot *xe_guc_log_snapshot_capture(struct xe_guc_log *log,
>  	return snapshot;
>  }
>  
> +#if IS_ENABLED(CONFIG_DRM_XE_COMPRESS_DUMP)
> +static void xe_guc_print_blob_compressed(struct xe_gt *gt, struct drm_printer *p,
> +					 const char *length_label, const char *data_label,
> +					 void **chunks, int num_chunks,
> +					 size_t chunk_size, size_t total_size)
> +{
[ ... ]
> +	dst_size = zstd_compress_bound(total_size);
> +	dst = kvzalloc(dst_size, GFP_KERNEL);

[Severity: Medium]
Will this allocate a buffer that is not 4-byte aligned? It appears dst_size
is determined by zstd_compress_bound(total_size) which is not guaranteed to
be a multiple of 4. When xe_print_blob_ascii85 is later called with a size
rounded up to 4 bytes, this can cause a KASAN out-of-bounds read.

> +	if (!dst)
> +		goto err;
[ ... ]
> +	remain = total_size;
> +	for (i = 0; i < num_chunks && remain; i++) {
> +		size_t size = min(chunk_size, remain);
> +		zstd_in_buffer inbuf = { .src = chunks[i], .size = size, .pos = 0 };
> +
> +		while (inbuf.pos < inbuf.size) {
> +			ret = zstd_compress_stream(cstream, &outbuf, &inbuf);
> +			if (ZSTD_isError(ret)) {

[Severity: High]
Can this loop spin infinitely if the output buffer fills up? The while loop
relies entirely on input consumption (inbuf.pos < inbuf.size) but lacks a
bailout condition or error check for when the output buffer (outbuf) is
full. 

ZSTD explicitly documents that zstd_compress_bound() is not guaranteed to be
sufficient for streaming compression. If the output buffer fills and
zstd_compress_stream() consumes no input, would this cause a hard lockup?

> +				xe_gt_err(gt, "zstd_compress_stream failed: %s\n",
> +					  ZSTD_getErrorName(ret));
> +				goto err;
> +			}
> +		}
> +		remain -= size;
> +	}
> +
> +	do {
> +		ret = zstd_end_stream(cstream, &outbuf);
> +		if (ZSTD_isError(ret)) {
> +			xe_gt_err(gt, "zstd_end_stream failed: %s\n",
> +				  ZSTD_getErrorName(ret));
> +			goto err;
> +		}
> +	} while (ret > 0 && outbuf.pos < outbuf.size);
> +
> +	drm_printf(p, "%s.length: 0x%zx -> 0x%zx Algo: ZSTD\n",
> +		   length_label, total_size, outbuf.pos);
> +	xe_print_blob_ascii85(p, data_label, '\n', dst, 0,
> +			      DIV_ROUND_UP(outbuf.pos, sizeof(u32)) * sizeof(u32));

[Severity: Medium]
If outbuf.pos is close to dst_size and not a multiple of 4, will this
rounded-up size cause xe_print_blob_ascii85 to read beyond the KASAN
boundaries of the allocated buffer?

> +	goto out;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915174840.531473-4-alan.previn.teres.alexis@intel.com?part=2