Re: [PATCH 3/3] drm/xe/oa: Limit num_syncs to prevent oversized allocations

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Fri, 05 Dec 2025 11:05:10 -0800, Shuicheng Lin wrote:
>

Hi Shuicheng,

> The OA open parameters did not validate num_syncs, allowing
> userspace to pass arbitrarily large values, potentially
> leading to excessive allocations.
>
> Add checks to ensure that num_syncs does not exceed XE_MAX_SYNCS,
> returning -EINVAL when the limit is violated.
>
> Fixes: c8507a25cebd ("drm/xe/oa/uapi: Define and parse OA sync properties")
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_oa.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
> index cc48663c2b48..7477c27a4ba2 100644
> --- a/drivers/gpu/drm/xe/xe_oa.c
> +++ b/drivers/gpu/drm/xe/xe_oa.c
> @@ -1254,6 +1254,11 @@ static int xe_oa_set_no_preempt(struct xe_oa *oa, u64 value,
>  static int xe_oa_set_prop_num_syncs(struct xe_oa *oa, u64 value,
>				    struct xe_oa_open_param *param)
>  {
> +	if (value > XE_MAX_SYNCS) {
> +		drm_dbg(&oa->xe->drm, "num_syncs %llu must be <= %u\n",
> +			value, XE_MAX_SYNCS);
> +		return -EINVAL;
> +	}

Like the other patches, let's just do:

	if (XE_IOCTL_DBG(xe, value > XE_MAX_SYNCS))
		return -EINVAL;

I might change the other functions here to also use XE_IOCTL_DBG(). They
may have been written before XE_IOCTL_DBG() was implemented.

>	param->num_syncs = value;
>	return 0;
>  }
> @@ -1404,6 +1409,12 @@ static int xe_oa_parse_syncs(struct xe_oa *oa,
>	}
>
>	if (param->num_syncs) {
> +		if (param->num_syncs > XE_MAX_SYNCS) {
> +			drm_dbg(&oa->xe->drm, "num_syncs %d must be <= %u\n",
> +				param->num_syncs, XE_MAX_SYNCS);
> +			ret = -EINVAL;
> +			goto exit;
> +		}

This is not needed. If we return -EINVAL from the code at the top, this
will never get executed.

Also, maybe squash the 3 patches into a single patch? Not sure if we need 3
patches, a single patch with XE_IOCTL_DBG() in all 3 places might make more
sense?

>		param->syncs = kcalloc(param->num_syncs, sizeof(*param->syncs), GFP_KERNEL);
>		if (!param->syncs) {
>			ret = -ENOMEM;
> --
> 2.50.1
>

Thanks.
--
Ashutosh