Re: [PATCH v4 01/25] drm/dumb-buffers: Sanitize output on errors

From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: dri-devel@lists.freedesktop.org,
	linux-mediatek@lists.infradead.org,
	freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org,
	imx@lists.linux.dev, linux-samsung-soc@vger.kernel.org,
	nouveau@lists.freedesktop.org, virtualization@lists.linux.dev,
	spice-devel@lists.freedesktop.org,
	linux-renesas-soc@vger.kernel.org,
	linux-rockchip@lists.infradead.org, linux-tegra@vger.kernel.org,
	intel-xe@lists.freedesktop.org, xen-devel@lists.xenproject.org,
	simona@ffwll.ch, airlied@gmail.com, mripard@kernel.org,
	maarten.lankhorst@linux.intel.com, geert@linux-m68k.org
Subject: Re: [PATCH v4 01/25] drm/dumb-buffers: Sanitize output on errors
Date: Thu, 12 Jun 2025 11:11:57 +0300	[thread overview]
Message-ID: <e2545be1-583d-4ad7-8a17-b2ee157f82d7@ideasonboard.com> (raw)
In-Reply-To: <20250311155120.442633-2-tzimmermann@suse.de>

Hi,

On 11/03/2025 17:47, Thomas Zimmermann wrote:
> The ioctls MODE_CREATE_DUMB and MODE_MAP_DUMB return results into a
> memory buffer supplied by user space. On errors, it is possible that
> intermediate values are being returned. The exact semantics depends
> on the DRM driver's implementation of these ioctls. Although this is
> most-likely not a security problem in practice, avoid any uncertainty
> by clearing the memory to 0 on errors.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_dumb_buffers.c | 40 ++++++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c
> index 70032bba1c97..9916aaf5b3f2 100644
> --- a/drivers/gpu/drm/drm_dumb_buffers.c
> +++ b/drivers/gpu/drm/drm_dumb_buffers.c
> @@ -99,7 +99,30 @@ int drm_mode_create_dumb(struct drm_device *dev,
>  int drm_mode_create_dumb_ioctl(struct drm_device *dev,
>  			       void *data, struct drm_file *file_priv)
>  {
> -	return drm_mode_create_dumb(dev, data, file_priv);
> +	struct drm_mode_create_dumb *args = data;
> +	int err;
> +
> +	err = drm_mode_create_dumb(dev, args, file_priv);
> +	if (err) {
> +		args->handle = 0;
> +		args->pitch = 0;
> +		args->size = 0;
> +	}
> +	return err;
> +}
> +
> +static int drm_mode_mmap_dumb(struct drm_device *dev, struct drm_mode_map_dumb *args,
> +			      struct drm_file *file_priv)
> +{
> +	if (!dev->driver->dumb_create)
> +		return -ENOSYS;
> +
> +	if (dev->driver->dumb_map_offset)
> +		return dev->driver->dumb_map_offset(file_priv, dev, args->handle,
> +						    &args->offset);
> +	else
> +		return drm_gem_dumb_map_offset(file_priv, dev, args->handle,
> +					       &args->offset);
>  }
>  
>  /**
> @@ -120,17 +143,12 @@ int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
>  			     void *data, struct drm_file *file_priv)
>  {
>  	struct drm_mode_map_dumb *args = data;
> +	int err;
>  
> -	if (!dev->driver->dumb_create)
> -		return -ENOSYS;
> -
> -	if (dev->driver->dumb_map_offset)
> -		return dev->driver->dumb_map_offset(file_priv, dev,
> -						    args->handle,
> -						    &args->offset);
> -	else
> -		return drm_gem_dumb_map_offset(file_priv, dev, args->handle,
> -					       &args->offset);
> +	err = drm_mode_mmap_dumb(dev, args, file_priv);
> +	if (err)
> +		args->offset = 0;
> +	return err;
>  }
>  
>  int drm_mode_destroy_dumb(struct drm_device *dev, u32 handle,

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip