Re: [PATCH] drm: Limit individual events to 1 KiB

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Dec 15, 2016 at 12:48:34PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Ever since support for vblank handling and event processing was added to
> libdrm (v2.4.16), events were read in 1 KiB chunks in drmHandleEvent().
> In addition, the implementation of drm_read() will not unqueue events if
> they can't be successfully passed to userspace. The result is that it's
> possible for a driver to send applications into a busy loop by creating
> events larger than 1 KiB.
> 
> There is a limit to the event space that each DRM device can carry, but
> it is set to 4 KiB.
> 
> While no driver currently produces events larger than 1 KiB, and it's
> fairly unlikely that anyone ever will, it's best to err on the side of
> caution and limit the size of individual events to 1 KiB. This ensures
> that libdrm will never run into the busy loop. In order to allow for
> the DRM device to queue multiple events, the event space limit is kept
> at 4 KiB.
> 
> A complementary patch will be applied to libdrm to bump the read size
> for events to 4 KiB to ensure that newer versions of libdrm running on
> kernels without this fix will not run into the busy loop either.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/drm_fops.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index 48e106557c92..3792e4434319 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -582,6 +582,15 @@ EXPORT_SYMBOL(drm_poll);
>   * This is the locked version of drm_event_reserve_init() for callers which
>   * already hold dev->event_lock.
>   *
> + * Note that libdrm has been reading events in chunks of 1 KiB since forever
> + * (see drmHandleEvent()) and up to and including v2.4.74. Unfortunately the
> + * drm_read() implementation won't return an error if an event doesn't fit
> + * into that 1 KiB and instead these events will simply be queued again. If
> + * a driver was to send out an event larger than 1 KiB, an application that
> + * polls the DRM file descriptor and calling drmHandleEvent() to process the
> + * events would go into a busy loop. This is caused by drm_poll() always
> + * return immediately, but drm_read() never being able to dequeue the event.
> + *
>   * RETURNS:
>   *
>   * 0 on success or a negative error code on failure.
> @@ -591,6 +600,15 @@ int drm_event_reserve_init_locked(struct drm_device *dev,
>  				  struct drm_pending_event *p,
>  				  struct drm_event *e)
>  {
> +	/*
> +	 * Limit events to 1 KiB, which is the read size of libdrm (up to
> +	 * to and including v2.4.74) when processing events, in order to
> +	 * prevent any of the applications using libdrm from potentially
> +	 * going into a busy loop.
> +	 */
> +	if (e->length > 1024)
> +		return -EINVAL;

Are we really expecting events that large? A much lower limit,
accompanied by a WARN/BUG, is what I'd go for.

> +
>  	if (file_priv->event_space < e->length)
>  		return -ENOMEM;
>  
> -- 
> 2.10.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel