| Message ID | 20200210230943.2874-1-jajones@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/nouveau: Fix NULL ptr access in nv50_wndw_prepare_fb() | expand |
Hi I'm surprised that prepare_fb is called with fb == NULL. But, OK Acked-by: Thomas Zimmermann <tzimmermann@suse.de> Thanks for the fix. Am 11.02.20 um 00:09 schrieb James Jones: > This fixes a kernel oops when loading the nouveau > module with fb console enabled after the change: > > drm/nouveau: Remove field nvbo from struct nouveau_framebuffer > > state->fb may be NULL in nv50_wndw_prepare_fb(), > so defer initializing nvbo from its obj[] array > until after the NULL check. > > Signed-off-by: James Jones <jajones@nvidia.com> > --- > drivers/gpu/drm/nouveau/dispnv50/wndw.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > index 4a67a656e007..68c0dc2dc2d3 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > @@ -490,7 +490,7 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) > struct nouveau_drm *drm = nouveau_drm(plane->dev); > struct nv50_wndw *wndw = nv50_wndw(plane); > struct nv50_wndw_atom *asyw = nv50_wndw_atom(state); > - struct nouveau_bo *nvbo = nouveau_gem_object(fb->obj[0]); > + struct nouveau_bo *nvbo; > struct nv50_head_atom *asyh; > struct nv50_wndw_ctxdma *ctxdma; > int ret; > @@ -499,6 +499,7 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) > if (!asyw->state.fb) > return 0; > > + nvbo = nouveau_gem_object(fb->obj[0]); > ret = nouveau_bo_pin(nvbo, TTM_PL_FLAG_VRAM, true); > if (ret) > return ret; >
On Tue, Feb 11, 2020 at 06:28:47AM +0100, Thomas Zimmermann wrote: > Hi > > I'm surprised that prepare_fb is called with fb == NULL. But, OK Yeah we don't filter that ... maybe an oversight? I'm not sure whether there's any driver that needs to do something special for when the plane is disabled here (since "plane off" iff "plane_state-fb == NULL"). In general I think we have an awful lot of bugs in most drivers for the case when the plane state is in the atomic commit, but not enabled. -Daniel > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > Thanks for the fix. > > Am 11.02.20 um 00:09 schrieb James Jones: > > This fixes a kernel oops when loading the nouveau > > module with fb console enabled after the change: > > > > drm/nouveau: Remove field nvbo from struct nouveau_framebuffer > > > > state->fb may be NULL in nv50_wndw_prepare_fb(), > > so defer initializing nvbo from its obj[] array > > until after the NULL check. > > > > Signed-off-by: James Jones <jajones@nvidia.com> > > --- > > drivers/gpu/drm/nouveau/dispnv50/wndw.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > > index 4a67a656e007..68c0dc2dc2d3 100644 > > --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c > > +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > > @@ -490,7 +490,7 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) > > struct nouveau_drm *drm = nouveau_drm(plane->dev); > > struct nv50_wndw *wndw = nv50_wndw(plane); > > struct nv50_wndw_atom *asyw = nv50_wndw_atom(state); > > - struct nouveau_bo *nvbo = nouveau_gem_object(fb->obj[0]); > > + struct nouveau_bo *nvbo; > > struct nv50_head_atom *asyh; > > struct nv50_wndw_ctxdma *ctxdma; > > int ret; > > @@ -499,6 +499,7 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) > > if (!asyw->state.fb) > > return 0; > > > > + nvbo = nouveau_gem_object(fb->obj[0]); > > ret = nouveau_bo_pin(nvbo, TTM_PL_FLAG_VRAM, true); > > if (ret) > > return ret; > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer > > _______________________________________________ > Nouveau mailing list > Nouveau@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c index 4a67a656e007..68c0dc2dc2d3 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c @@ -490,7 +490,7 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) struct nouveau_drm *drm = nouveau_drm(plane->dev); struct nv50_wndw *wndw = nv50_wndw(plane); struct nv50_wndw_atom *asyw = nv50_wndw_atom(state); - struct nouveau_bo *nvbo = nouveau_gem_object(fb->obj[0]); + struct nouveau_bo *nvbo; struct nv50_head_atom *asyh; struct nv50_wndw_ctxdma *ctxdma; int ret; @@ -499,6 +499,7 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) if (!asyw->state.fb) return 0; + nvbo = nouveau_gem_object(fb->obj[0]); ret = nouveau_bo_pin(nvbo, TTM_PL_FLAG_VRAM, true); if (ret) return ret;
This fixes a kernel oops when loading the nouveau module with fb console enabled after the change: drm/nouveau: Remove field nvbo from struct nouveau_framebuffer state->fb may be NULL in nv50_wndw_prepare_fb(), so defer initializing nvbo from its obj[] array until after the NULL check. Signed-off-by: James Jones <jajones@nvidia.com> --- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)