drm/msm: correct NULL pointer dereference in context_init

Submitted by Brian Masney on June 27, 2019, 2:05 a.m.

Details

Message ID 20190627020515.5660-1-masneyb@onstation.org
State New
Headers show
Series "drm/msm: correct NULL pointer dereference in context_init" ( rev: 1 ) in Freedreno

Not browsing as part of any series.

Commit Message

Brian Masney June 27, 2019, 2:05 a.m.
Correct attempted NULL pointer dereference in context_init() when
running without an IOMMU.

Signed-off-by: Brian Masney <masneyb@onstation.org>
Fixes: 295b22ae596c ("drm/msm: Pass the MMU domain index in struct msm_file_private")
---
The no IOMMU case seems like functionality that we may want to keep
based on this comment:
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/msm/adreno/a3xx_gpu.c#L523
Once I get the msm8974 interconnect driver done, I'm going to look into
what needs to be done to get the IOMMU working on the Nexus 5.

Alternatively, for development purposes, maybe we could have a NOOP
IOMMU driver that would allow us to remove these NULL checks that are
sprinkled throughout the code. I haven't looked into this in detail.
Thoughts?

 drivers/gpu/drm/msm/msm_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 451bd4508793..83047cb2c735 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -619,7 +619,7 @@  static int context_init(struct drm_device *dev, struct drm_file *file)
 
 	msm_submitqueue_init(dev, ctx);
 
-	ctx->aspace = priv->gpu->aspace;
+	ctx->aspace = priv->gpu ? priv->gpu->aspace : NULL;
 	file->driver_priv = ctx;
 
 	return 0;

Comments

On Wed, Jun 26, 2019 at 7:05 PM Brian Masney <masneyb@onstation.org> wrote:
>
> Correct attempted NULL pointer dereference in context_init() when
> running without an IOMMU.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> Fixes: 295b22ae596c ("drm/msm: Pass the MMU domain index in struct msm_file_private")
> ---
> The no IOMMU case seems like functionality that we may want to keep
> based on this comment:
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/msm/adreno/a3xx_gpu.c#L523
> Once I get the msm8974 interconnect driver done, I'm going to look into
> what needs to be done to get the IOMMU working on the Nexus 5.
>
> Alternatively, for development purposes, maybe we could have a NOOP
> IOMMU driver that would allow us to remove these NULL checks that are
> sprinkled throughout the code. I haven't looked into this in detail.
> Thoughts?

yeah, we probably want to keep !iommu support, it is at least useful
for bringup of new (or old) devices.  But tends to bitrot a since it
isn't a case that gets tested much once iommu is in place.  Perhaps
there is a way to have a null iommu/aspace, although I'm not quite
sure how that would work..

Anyways,

Reviewed-by: Rob Clark <robdclark@gmail.com>

(I guess this can go in via drm-misc-fixes unless we get some more
fixes to justify sending msm-fixes MR..)

>
>  drivers/gpu/drm/msm/msm_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 451bd4508793..83047cb2c735 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -619,7 +619,7 @@ static int context_init(struct drm_device *dev, struct drm_file *file)
>
>         msm_submitqueue_init(dev, ctx);
>
> -       ctx->aspace = priv->gpu->aspace;
> +       ctx->aspace = priv->gpu ? priv->gpu->aspace : NULL;
>         file->driver_priv = ctx;
>
>         return 0;
> --
> 2.20.1
>
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
On Fri, Jun 28, 2019 at 05:57:26AM -0700, Rob Clark wrote:
> On Wed, Jun 26, 2019 at 7:05 PM Brian Masney <masneyb@onstation.org> wrote:
> >
> > Correct attempted NULL pointer dereference in context_init() when
> > running without an IOMMU.
> >
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > Fixes: 295b22ae596c ("drm/msm: Pass the MMU domain index in struct msm_file_private")
> > ---
> > The no IOMMU case seems like functionality that we may want to keep
> > based on this comment:
> > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/msm/adreno/a3xx_gpu.c#L523
> > Once I get the msm8974 interconnect driver done, I'm going to look into
> > what needs to be done to get the IOMMU working on the Nexus 5.
> >
> > Alternatively, for development purposes, maybe we could have a NOOP
> > IOMMU driver that would allow us to remove these NULL checks that are
> > sprinkled throughout the code. I haven't looked into this in detail.
> > Thoughts?
> 
> yeah, we probably want to keep !iommu support, it is at least useful
> for bringup of new (or old) devices.  But tends to bitrot a since it
> isn't a case that gets tested much once iommu is in place.  Perhaps
> there is a way to have a null iommu/aspace, although I'm not quite
> sure how that would work..
> 
> Anyways,
> 
> Reviewed-by: Rob Clark <robdclark@gmail.com>
> 
> (I guess this can go in via drm-misc-fixes unless we get some more
> fixes to justify sending msm-fixes MR..)

Applied to drm-misc-fixes for 5.3

Sean

> 
> >
> >  drivers/gpu/drm/msm/msm_drv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > index 451bd4508793..83047cb2c735 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > @@ -619,7 +619,7 @@ static int context_init(struct drm_device *dev, struct drm_file *file)
> >
> >         msm_submitqueue_init(dev, ctx);
> >
> > -       ctx->aspace = priv->gpu->aspace;
> > +       ctx->aspace = priv->gpu ? priv->gpu->aspace : NULL;
> >         file->driver_priv = ctx;
> >
> >         return 0;
> > --
> > 2.20.1
> >
> > _______________________________________________
> > Freedreno mailing list
> > Freedreno@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/freedreno