Patch "drm: damage_helper: Fix race checking plane->state->fb" has been added to the 5.4-stable tree

2019-12-09 Thread gregkh

This is a note to let you know that I've just added the patch titled

drm: damage_helper: Fix race checking plane->state->fb

to the 5.4-stable tree which can be found at:

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
 drm-damage_helper-fix-race-checking-plane-state-fb.patch
and it can be found in the queue-5.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let  know about it.


From 354c2d310082d1c384213ba76c3757dd3cd8755d Mon Sep 17 00:00:00 2001
From: Sean Paul 
Date: Wed, 4 Sep 2019 16:29:13 -0400
Subject: drm: damage_helper: Fix race checking plane->state->fb

From: Sean Paul 

commit 354c2d310082d1c384213ba76c3757dd3cd8755d upstream.

Since the dirtyfb ioctl doesn't give us any hints as to which plane is
scanning out the fb it's marking as damaged, we need to loop through
planes to find it.

Currently we just reach into plane state and check, but that can race
with another commit changing the fb out from under us. This patch locks
the plane before checking the fb and will release the lock if the plane
is not displaying the dirty fb.

Fixes: b9fc5e01d1ce ("drm: Add helper to implement legacy dirtyfb")
Cc: Rob Clark 
Cc: Deepak Rawat 
Cc: Daniel Vetter 
Cc: Thomas Hellstrom 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: dri-devel@lists.freedesktop.org
Cc:  # v5.0+
Reported-by: Daniel Vetter 
Reviewed-by: Daniel Vetter 
Signed-off-by: Sean Paul 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20190904202938.110207-1-s...@poorly.run
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/gpu/drm/drm_damage_helper.c |8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/drm_damage_helper.c
+++ b/drivers/gpu/drm/drm_damage_helper.c
@@ -212,8 +212,14 @@ retry:
drm_for_each_plane(plane, fb->dev) {
struct drm_plane_state *plane_state;
 
-   if (plane->state->fb != fb)
+   ret = drm_modeset_lock(>mutex, state->acquire_ctx);
+   if (ret)
+   goto out;
+
+   if (plane->state->fb != fb) {
+   drm_modeset_unlock(>mutex);
continue;
+   }
 
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state)) {


Patches currently in stable-queue which might be from seanp...@chromium.org are

queue-5.4/drm-msm-fix-memleak-on-release.patch
queue-5.4/drm-damage_helper-fix-race-checking-plane-state-fb.patch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Patch "drm: damage_helper: Fix race checking plane->state->fb" has been added to the 5.3-stable tree

2019-12-09 Thread gregkh

This is a note to let you know that I've just added the patch titled

drm: damage_helper: Fix race checking plane->state->fb

to the 5.3-stable tree which can be found at:

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
 drm-damage_helper-fix-race-checking-plane-state-fb.patch
and it can be found in the queue-5.3 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let  know about it.


From 354c2d310082d1c384213ba76c3757dd3cd8755d Mon Sep 17 00:00:00 2001
From: Sean Paul 
Date: Wed, 4 Sep 2019 16:29:13 -0400
Subject: drm: damage_helper: Fix race checking plane->state->fb

From: Sean Paul 

commit 354c2d310082d1c384213ba76c3757dd3cd8755d upstream.

Since the dirtyfb ioctl doesn't give us any hints as to which plane is
scanning out the fb it's marking as damaged, we need to loop through
planes to find it.

Currently we just reach into plane state and check, but that can race
with another commit changing the fb out from under us. This patch locks
the plane before checking the fb and will release the lock if the plane
is not displaying the dirty fb.

Fixes: b9fc5e01d1ce ("drm: Add helper to implement legacy dirtyfb")
Cc: Rob Clark 
Cc: Deepak Rawat 
Cc: Daniel Vetter 
Cc: Thomas Hellstrom 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: dri-devel@lists.freedesktop.org
Cc:  # v5.0+
Reported-by: Daniel Vetter 
Reviewed-by: Daniel Vetter 
Signed-off-by: Sean Paul 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20190904202938.110207-1-s...@poorly.run
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/gpu/drm/drm_damage_helper.c |8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/drm_damage_helper.c
+++ b/drivers/gpu/drm/drm_damage_helper.c
@@ -212,8 +212,14 @@ retry:
drm_for_each_plane(plane, fb->dev) {
struct drm_plane_state *plane_state;
 
-   if (plane->state->fb != fb)
+   ret = drm_modeset_lock(>mutex, state->acquire_ctx);
+   if (ret)
+   goto out;
+
+   if (plane->state->fb != fb) {
+   drm_modeset_unlock(>mutex);
continue;
+   }
 
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state)) {


Patches currently in stable-queue which might be from seanp...@chromium.org are

queue-5.3/drm-msm-fix-memleak-on-release.patch
queue-5.3/drm-damage_helper-fix-race-checking-plane-state-fb.patch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: damage_helper: Fix race checking plane->state->fb

2019-10-08 Thread Sean Paul
On Tue, Oct 08, 2019 at 11:50:33AM +0200, Daniel Vetter wrote:
> On Thu, Sep 19, 2019 at 11:04:01AM -0400, Sean Paul wrote:
> > On Thu, Sep 05, 2019 at 12:41:27PM +0200, Daniel Vetter wrote:
> > > On Wed, Sep 4, 2019 at 10:29 PM Sean Paul  wrote:
> > > >
> > > > From: Sean Paul 
> > > >
> > > > Since the dirtyfb ioctl doesn't give us any hints as to which plane is
> > > > scanning out the fb it's marking as damaged, we need to loop through
> > > > planes to find it.
> > > >
> > > > Currently we just reach into plane state and check, but that can race
> > > > with another commit changing the fb out from under us. This patch locks
> > > > the plane before checking the fb and will release the lock if the plane
> > > > is not displaying the dirty fb.
> > > >
> > > > Fixes: b9fc5e01d1ce ("drm: Add helper to implement legacy dirtyfb")
> > > > Cc: Rob Clark 
> > > > Cc: Deepak Rawat 
> > > > Cc: Daniel Vetter 
> > > > Cc: Thomas Hellstrom 
> > > > Cc: Maarten Lankhorst 
> > > > Cc: Maxime Ripard 
> > > > Cc: Sean Paul 
> > > > Cc: David Airlie 
> > > > Cc: Daniel Vetter 
> > > > Cc: dri-devel@lists.freedesktop.org
> > > > Cc:  # v5.0+
> > > > Reported-by: Daniel Vetter 
> > > > Signed-off-by: Sean Paul 
> > > > ---
> > > >  drivers/gpu/drm/drm_damage_helper.c | 8 +++-
> > > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_damage_helper.c 
> > > > b/drivers/gpu/drm/drm_damage_helper.c
> > > > index 8230dac01a89..3a4126dc2520 100644
> > > > --- a/drivers/gpu/drm/drm_damage_helper.c
> > > > +++ b/drivers/gpu/drm/drm_damage_helper.c
> > > > @@ -212,8 +212,14 @@ int drm_atomic_helper_dirtyfb(struct 
> > > > drm_framebuffer *fb,
> > > > drm_for_each_plane(plane, fb->dev) {
> > > > struct drm_plane_state *plane_state;
> > > >
> > > > -   if (plane->state->fb != fb)
> > > > +   ret = drm_modeset_lock(>mutex, 
> > > > state->acquire_ctx);
> > > > +   if (ret)
> > > 
> > > I think for paranoid safety we should have a WARN_ON(ret == -EALREADY)
> > > here. It should be impossible, but if it's not for some oddball
> > > reason, we'll blow up.
> > 
> > drm_modeset_lock eats EALREADY and returns 0 for that case, so I guess it
> > depends _how_ paranoid you want to be here :-)
> 
> Ah silly me, r-b as-is then.

Thanks, pushed to -misc-next

Sean

> -Daniel
> 
> > 
> > > 
> > > With that: Reviewed-by: Daniel Vetter 
> > > 
> > > But please give this a spin with some workloads and the ww_mutex
> > > slowpath debugging enabled, just to makre sure.
> > 
> > Ok, had a chance to run through some tests this morning with
> > CONFIG_DEBUG_WW_MUTEX_SLOWPATH and things lgtm
> > 
> > Sean
> > 
> > > -Daniel
> > > 
> > > > +   goto out;
> > > > +
> > > > +   if (plane->state->fb != fb) {
> > > > +   drm_modeset_unlock(>mutex);
> > > > continue;
> > > > +   }
> > > >
> > > > plane_state = drm_atomic_get_plane_state(state, plane);
> > > > if (IS_ERR(plane_state)) {
> > > > --
> > > > Sean Paul, Software Engineer, Google / Chromium OS
> > > >
> > > 
> > > 
> > > -- 
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > 
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: damage_helper: Fix race checking plane->state->fb

2019-10-08 Thread Daniel Vetter
On Thu, Sep 19, 2019 at 11:04:01AM -0400, Sean Paul wrote:
> On Thu, Sep 05, 2019 at 12:41:27PM +0200, Daniel Vetter wrote:
> > On Wed, Sep 4, 2019 at 10:29 PM Sean Paul  wrote:
> > >
> > > From: Sean Paul 
> > >
> > > Since the dirtyfb ioctl doesn't give us any hints as to which plane is
> > > scanning out the fb it's marking as damaged, we need to loop through
> > > planes to find it.
> > >
> > > Currently we just reach into plane state and check, but that can race
> > > with another commit changing the fb out from under us. This patch locks
> > > the plane before checking the fb and will release the lock if the plane
> > > is not displaying the dirty fb.
> > >
> > > Fixes: b9fc5e01d1ce ("drm: Add helper to implement legacy dirtyfb")
> > > Cc: Rob Clark 
> > > Cc: Deepak Rawat 
> > > Cc: Daniel Vetter 
> > > Cc: Thomas Hellstrom 
> > > Cc: Maarten Lankhorst 
> > > Cc: Maxime Ripard 
> > > Cc: Sean Paul 
> > > Cc: David Airlie 
> > > Cc: Daniel Vetter 
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc:  # v5.0+
> > > Reported-by: Daniel Vetter 
> > > Signed-off-by: Sean Paul 
> > > ---
> > >  drivers/gpu/drm/drm_damage_helper.c | 8 +++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_damage_helper.c 
> > > b/drivers/gpu/drm/drm_damage_helper.c
> > > index 8230dac01a89..3a4126dc2520 100644
> > > --- a/drivers/gpu/drm/drm_damage_helper.c
> > > +++ b/drivers/gpu/drm/drm_damage_helper.c
> > > @@ -212,8 +212,14 @@ int drm_atomic_helper_dirtyfb(struct drm_framebuffer 
> > > *fb,
> > > drm_for_each_plane(plane, fb->dev) {
> > > struct drm_plane_state *plane_state;
> > >
> > > -   if (plane->state->fb != fb)
> > > +   ret = drm_modeset_lock(>mutex, state->acquire_ctx);
> > > +   if (ret)
> > 
> > I think for paranoid safety we should have a WARN_ON(ret == -EALREADY)
> > here. It should be impossible, but if it's not for some oddball
> > reason, we'll blow up.
> 
> drm_modeset_lock eats EALREADY and returns 0 for that case, so I guess it
> depends _how_ paranoid you want to be here :-)

Ah silly me, r-b as-is then.
-Daniel

> 
> > 
> > With that: Reviewed-by: Daniel Vetter 
> > 
> > But please give this a spin with some workloads and the ww_mutex
> > slowpath debugging enabled, just to makre sure.
> 
> Ok, had a chance to run through some tests this morning with
> CONFIG_DEBUG_WW_MUTEX_SLOWPATH and things lgtm
> 
> Sean
> 
> > -Daniel
> > 
> > > +   goto out;
> > > +
> > > +   if (plane->state->fb != fb) {
> > > +   drm_modeset_unlock(>mutex);
> > > continue;
> > > +   }
> > >
> > > plane_state = drm_atomic_get_plane_state(state, plane);
> > > if (IS_ERR(plane_state)) {
> > > --
> > > Sean Paul, Software Engineer, Google / Chromium OS
> > >
> > 
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: damage_helper: Fix race checking plane->state->fb

2019-09-19 Thread Sean Paul
On Thu, Sep 05, 2019 at 12:41:27PM +0200, Daniel Vetter wrote:
> On Wed, Sep 4, 2019 at 10:29 PM Sean Paul  wrote:
> >
> > From: Sean Paul 
> >
> > Since the dirtyfb ioctl doesn't give us any hints as to which plane is
> > scanning out the fb it's marking as damaged, we need to loop through
> > planes to find it.
> >
> > Currently we just reach into plane state and check, but that can race
> > with another commit changing the fb out from under us. This patch locks
> > the plane before checking the fb and will release the lock if the plane
> > is not displaying the dirty fb.
> >
> > Fixes: b9fc5e01d1ce ("drm: Add helper to implement legacy dirtyfb")
> > Cc: Rob Clark 
> > Cc: Deepak Rawat 
> > Cc: Daniel Vetter 
> > Cc: Thomas Hellstrom 
> > Cc: Maarten Lankhorst 
> > Cc: Maxime Ripard 
> > Cc: Sean Paul 
> > Cc: David Airlie 
> > Cc: Daniel Vetter 
> > Cc: dri-devel@lists.freedesktop.org
> > Cc:  # v5.0+
> > Reported-by: Daniel Vetter 
> > Signed-off-by: Sean Paul 
> > ---
> >  drivers/gpu/drm/drm_damage_helper.c | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_damage_helper.c 
> > b/drivers/gpu/drm/drm_damage_helper.c
> > index 8230dac01a89..3a4126dc2520 100644
> > --- a/drivers/gpu/drm/drm_damage_helper.c
> > +++ b/drivers/gpu/drm/drm_damage_helper.c
> > @@ -212,8 +212,14 @@ int drm_atomic_helper_dirtyfb(struct drm_framebuffer 
> > *fb,
> > drm_for_each_plane(plane, fb->dev) {
> > struct drm_plane_state *plane_state;
> >
> > -   if (plane->state->fb != fb)
> > +   ret = drm_modeset_lock(>mutex, state->acquire_ctx);
> > +   if (ret)
> 
> I think for paranoid safety we should have a WARN_ON(ret == -EALREADY)
> here. It should be impossible, but if it's not for some oddball
> reason, we'll blow up.

drm_modeset_lock eats EALREADY and returns 0 for that case, so I guess it
depends _how_ paranoid you want to be here :-)

> 
> With that: Reviewed-by: Daniel Vetter 
> 
> But please give this a spin with some workloads and the ww_mutex
> slowpath debugging enabled, just to makre sure.

Ok, had a chance to run through some tests this morning with
CONFIG_DEBUG_WW_MUTEX_SLOWPATH and things lgtm

Sean

> -Daniel
> 
> > +   goto out;
> > +
> > +   if (plane->state->fb != fb) {
> > +   drm_modeset_unlock(>mutex);
> > continue;
> > +   }
> >
> > plane_state = drm_atomic_get_plane_state(state, plane);
> > if (IS_ERR(plane_state)) {
> > --
> > Sean Paul, Software Engineer, Google / Chromium OS
> >
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: damage_helper: Fix race checking plane->state->fb

2019-09-05 Thread Daniel Vetter
On Wed, Sep 4, 2019 at 10:29 PM Sean Paul  wrote:
>
> From: Sean Paul 
>
> Since the dirtyfb ioctl doesn't give us any hints as to which plane is
> scanning out the fb it's marking as damaged, we need to loop through
> planes to find it.
>
> Currently we just reach into plane state and check, but that can race
> with another commit changing the fb out from under us. This patch locks
> the plane before checking the fb and will release the lock if the plane
> is not displaying the dirty fb.
>
> Fixes: b9fc5e01d1ce ("drm: Add helper to implement legacy dirtyfb")
> Cc: Rob Clark 
> Cc: Deepak Rawat 
> Cc: Daniel Vetter 
> Cc: Thomas Hellstrom 
> Cc: Maarten Lankhorst 
> Cc: Maxime Ripard 
> Cc: Sean Paul 
> Cc: David Airlie 
> Cc: Daniel Vetter 
> Cc: dri-devel@lists.freedesktop.org
> Cc:  # v5.0+
> Reported-by: Daniel Vetter 
> Signed-off-by: Sean Paul 
> ---
>  drivers/gpu/drm/drm_damage_helper.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_damage_helper.c 
> b/drivers/gpu/drm/drm_damage_helper.c
> index 8230dac01a89..3a4126dc2520 100644
> --- a/drivers/gpu/drm/drm_damage_helper.c
> +++ b/drivers/gpu/drm/drm_damage_helper.c
> @@ -212,8 +212,14 @@ int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb,
> drm_for_each_plane(plane, fb->dev) {
> struct drm_plane_state *plane_state;
>
> -   if (plane->state->fb != fb)
> +   ret = drm_modeset_lock(>mutex, state->acquire_ctx);
> +   if (ret)

I think for paranoid safety we should have a WARN_ON(ret == -EALREADY)
here. It should be impossible, but if it's not for some oddball
reason, we'll blow up.

With that: Reviewed-by: Daniel Vetter 

But please give this a spin with some workloads and the ww_mutex
slowpath debugging enabled, just to makre sure.
-Daniel

> +   goto out;
> +
> +   if (plane->state->fb != fb) {
> +   drm_modeset_unlock(>mutex);
> continue;
> +   }
>
> plane_state = drm_atomic_get_plane_state(state, plane);
> if (IS_ERR(plane_state)) {
> --
> Sean Paul, Software Engineer, Google / Chromium OS
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: damage_helper: Fix race checking plane->state->fb

2019-09-04 Thread Sean Paul
From: Sean Paul 

Since the dirtyfb ioctl doesn't give us any hints as to which plane is
scanning out the fb it's marking as damaged, we need to loop through
planes to find it.

Currently we just reach into plane state and check, but that can race
with another commit changing the fb out from under us. This patch locks
the plane before checking the fb and will release the lock if the plane
is not displaying the dirty fb.

Fixes: b9fc5e01d1ce ("drm: Add helper to implement legacy dirtyfb")
Cc: Rob Clark 
Cc: Deepak Rawat 
Cc: Daniel Vetter 
Cc: Thomas Hellstrom 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: dri-devel@lists.freedesktop.org
Cc:  # v5.0+
Reported-by: Daniel Vetter 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/drm_damage_helper.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_damage_helper.c 
b/drivers/gpu/drm/drm_damage_helper.c
index 8230dac01a89..3a4126dc2520 100644
--- a/drivers/gpu/drm/drm_damage_helper.c
+++ b/drivers/gpu/drm/drm_damage_helper.c
@@ -212,8 +212,14 @@ int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb,
drm_for_each_plane(plane, fb->dev) {
struct drm_plane_state *plane_state;
 
-   if (plane->state->fb != fb)
+   ret = drm_modeset_lock(>mutex, state->acquire_ctx);
+   if (ret)
+   goto out;
+
+   if (plane->state->fb != fb) {
+   drm_modeset_unlock(>mutex);
continue;
+   }
 
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state)) {
-- 
Sean Paul, Software Engineer, Google / Chromium OS

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