One very easy to trigger runtime PM leak, along with a rare never before seen runtime PM leak! Lyude Paul (2): drm/nouveau: Fix runtime PM leak in drm_open() drm/nouveau: Fix runtime PM leak in nv50_disp_atomic_commit() drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- drivers/gpu/drm/nouveau/nouveau_drm.c | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) -- 2.17.1
Lyude Paul
2018-Jul-03 22:05 UTC
[Nouveau] [PATCH 1/2] drm/nouveau: Fix runtime PM leak in drm_open()
Noticed this as I was skimming through, if we fail to allocate memory for cli we'll end up returning without dropping the runtime PM ref we got. Additionally, we'll even return the wrong return code! (ret most likely will == 0 here, we want -ENOMEM). Signed-off-by: Lyude Paul <lyude at redhat.com> Cc: stable at vger.kernel.org --- drivers/gpu/drm/nouveau/nouveau_drm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 0452b18d36b9..0f668e275ee1 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -919,8 +919,10 @@ nouveau_drm_open(struct drm_device *dev, struct drm_file *fpriv) get_task_comm(tmpname, current); snprintf(name, sizeof(name), "%s[%d]", tmpname, pid_nr(fpriv->pid)); - if (!(cli = kzalloc(sizeof(*cli), GFP_KERNEL))) - return ret; + if (!(cli = kzalloc(sizeof(*cli), GFP_KERNEL))) { + ret = -ENOMEM; + goto done; + } ret = nouveau_cli_init(drm, name, cli); if (ret) -- 2.17.1
Lyude Paul
2018-Jul-03 22:06 UTC
[Nouveau] [PATCH 2/2] drm/nouveau: Fix runtime PM leak in nv50_disp_atomic_commit()
A CRTC being enabled doesn't mean it's on! It doesn't even necessarily mean it's being used. This fixes runtime PM leaks on the P50 I've got next to me. Signed-off-by: Lyude Paul <lyude at redhat.com> Cc: stable at vger.kernel.org --- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index d9da69c83ae7..9bae4db84cfb 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -1878,7 +1878,7 @@ nv50_disp_atomic_commit(struct drm_device *dev, nv50_disp_atomic_commit_tail(state); drm_for_each_crtc(crtc, dev) { - if (crtc->state->enable) { + if (crtc->state->active) { if (!drm->have_disp_power_ref) { drm->have_disp_power_ref = true; return 0; -- 2.17.1
Lukas Wunner
2018-Jul-04 02:41 UTC
[Nouveau] [PATCH 1/2] drm/nouveau: Fix runtime PM leak in drm_open()
[cc -= stable] On Tue, Jul 03, 2018 at 06:05:59PM -0400, Lyude Paul wrote:> Noticed this as I was skimming through, if we fail to allocate memory > for cli we'll end up returning without dropping the runtime PM ref we > got. Additionally, we'll even return the wrong return code! (ret most > likely will == 0 here, we want -ENOMEM). > > Signed-off-by: Lyude Paul <lyude at redhat.com>Reviewed-by: Lukas Wunner <lukas at wunner.de>
Possibly Parallel Threads
- [PATCH v2 0/3] drm/nouveau: Fix runtime PM leaks
- [PATCH v2 2/3] drm/nouveau: Fix runtime PM leak in nv50_disp_atomic_commit()
- [PATCH 0/2] drm/nouveau: CRTC Runtime PM ref tracking fixes
- [PATCH v2 0/2] drm/nouveau: CRTC Runtime PM ref tracking fixes
- [PATCH v2 2/3] drm/nouveau: Fix runtime PM leak in nv50_disp_atomic_commit()