Re: [PATCH] drm: vc4: Fix race during binding

From: Eric Anholt <eric@anholt.net>
To: David Airlie <airlied@linux.ie>
Cc: Stefan Wahren <stefan.wahren@i2se.com>, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm: vc4: Fix race during binding
Date: Fri, 06 Oct 2017 12:42:58 -0700	[thread overview]
Message-ID: <87y3oo2hbx.fsf@anholt.net> (raw)
In-Reply-To: <1507315383-8107-1-git-send-email-stefan.wahren@i2se.com>

[-- Attachment #1.1: Type: text/plain, Size: 1728 bytes --]

Stefan Wahren <stefan.wahren@i2se.com> writes:

> This fixes the race between vc4_overflow_mem_work and the init of the
> job lock. Otherwise we could trigger a NULL pointer dereference
> during VC4 binding.
>
> Link: https://github.com/anholt/linux/issues/114
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> Fixes: d5b1a78a772f ("drm/vc4: Add support for drawing 3D frames.")
> ---
>  drivers/gpu/drm/vc4/vc4_gem.c | 1 -
>  drivers/gpu/drm/vc4/vc4_irq.c | 1 +
>  2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
> index d0c6bfb..47d964f 100644
> --- a/drivers/gpu/drm/vc4/vc4_gem.c
> +++ b/drivers/gpu/drm/vc4/vc4_gem.c
> @@ -1088,7 +1088,6 @@ vc4_gem_init(struct drm_device *dev)
>  	INIT_LIST_HEAD(&vc4->render_job_list);
>  	INIT_LIST_HEAD(&vc4->job_done_list);
>  	INIT_LIST_HEAD(&vc4->seqno_cb_list);
> -	spin_lock_init(&vc4->job_lock);
>  
>  	INIT_WORK(&vc4->hangcheck.reset_work, vc4_reset_work);
>  	setup_timer(&vc4->hangcheck.timer,
> diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c
> index 7d7af3a..d508d13 100644
> --- a/drivers/gpu/drm/vc4/vc4_irq.c
> +++ b/drivers/gpu/drm/vc4/vc4_irq.c
> @@ -195,6 +195,7 @@ vc4_irq_preinstall(struct drm_device *dev)
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  
>  	init_waitqueue_head(&vc4->job_wait_queue);
> +	spin_lock_init(&vc4->job_lock);
>  	INIT_WORK(&vc4->overflow_mem_work, vc4_overflow_mem_work);
>  
>  	/* Clear any pending interrupts someone might have left around

Are you sure this is a fix?  We don't attach the IRQ handler until V3D
bind, and vc4_gem_init happens before component_bind_all(), right?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel