[Intel-xe] [PATCH 2/2] drm/xe: properly check bounds for xe_wait_user_fence_ioctl()

[Lucas De Marchi]

On Mon, Jun 26, 2023 at 02:22:21PM -0700, Paulo Zanoni wrote:
>If !no_engines, then we use copy_from_user to copy to the 'eci' array,
>which has XE_HW_ENGINE_MAX_INSTANCE members. The amount of members
>copied is given by the user in args->num_engines, so add code to check
>that args->num_engines does not exceed XE_HW_ENGINE_MAX_INSTANCE. It's
>an unsigned value so there's no need to check for negative values.
>
>Fixes error messages such as:
>
>    Buffer overflow detected (54 < 18446744073709551520)!
>
>Very simple reproducer:
>
>    https://people.freedesktop.org/~pzanoni/wait-user-fence-bug/

same comment as in the previous patch. It'd be better to have a code
snippet in the commit message.

>
>Reviewed-by: José Roberto de Souza <jose.souza at intel.com>
>Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>---
> drivers/gpu/drm/xe/xe_wait_user_fence.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_wait_user_fence.c b/drivers/gpu/drm/xe/xe_wait_user_fence.c
>index 3122374341d6..098e2a4cff3f 100644
>--- a/drivers/gpu/drm/xe/xe_wait_user_fence.c
>+++ b/drivers/gpu/drm/xe/xe_wait_user_fence.c
>@@ -121,6 +121,9 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
> 			 addr & 0x7))
> 		return -EINVAL;
>
>+	if (XE_IOCTL_ERR(xe, args->num_engines > XE_HW_ENGINE_MAX_INSTANCE))

XE_IOCTL_ERR() is soon going away, but we don't need to wait for it.

Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

For these 2 commits, let me know if you are sending a new version or if
I should just add the code snippet while applying.

thanks
Lucas De Marchi

>+		return -EINVAL;
>+
> 	if (!no_engines) {
> 		err = copy_from_user(eci, user_eci,
> 				     sizeof(struct drm_xe_engine_class_instance) *
>-- 
>2.39.2
>