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

[Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>]

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@xxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxxx>

 Tomi