[PATCH] drm/cirrus: Fix NULL pointer dereference when registering the fbdev
cirrus_modeset_init() is initializing/registering the emulated fbdev and, since commit c61b93fe51b1 ("drm/atomic: Fix remaining places where !funcs->best_encoder is valid"), DRM internals can access/test some of the fields in mode_config->funcs as part of the fbdev registration process. Make sure dev->mode_config.funcs is properly set to avoid dereferencing a NULL pointer. Signed-off-by: Boris Brezillon Fixes: c61b93fe51b1 ("drm/atomic: Fix remaining places where !funcs->best_encoder is valid") --- Hi Dave, As discussed on IRC, I'm sending this patch in a proper format. That's probably better to wait for Eric's feeback before applying it though. Regards, Boris --- drivers/gpu/drm/cirrus/cirrus_main.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c b/drivers/gpu/drm/cirrus/cirrus_main.c index 80446e2d3ab6..76bcb43e7c06 100644 --- a/drivers/gpu/drm/cirrus/cirrus_main.c +++ b/drivers/gpu/drm/cirrus/cirrus_main.c @@ -185,14 +185,23 @@ int cirrus_driver_load(struct drm_device *dev, unsigned long flags) goto out; } + /* +* cirrus_modeset_init() is initializing/registering the emulated fbdev +* and DRM internals can access/test some of the fields in +* mode_config->funcs as part of the fbdev registration process. +* Make sure dev->mode_config.funcs is properly set to avoid +* dereferencing a NULL pointer. +* FIXME: mode_config.funcs assignment should probably be done in +* cirrus_modeset_init() (that's a common pattern seen in other DRM +* drivers). +*/ + dev->mode_config.funcs = &cirrus_mode_funcs; r = cirrus_modeset_init(cdev); if (r) { dev_err(&dev->pdev->dev, "Fatal error during modeset init: %d\n", r); goto out; } - dev->mode_config.funcs = (void *)&cirrus_mode_funcs; - return 0; out: cirrus_driver_unload(dev); -- 2.7.4
[PATCH] drm/cirrus: Fix NULL pointer dereference when registering the fbdev
On Mon, 08 Aug 2016 22:36:13 -0500 ebiederm at xmission.com (Eric W. Biederman) wrote: > Boris Brezillon writes: > > > cirrus_modeset_init() is initializing/registering the emulated fbdev > > and, since commit c61b93fe51b1 ("drm/atomic: Fix remaining places where > > !funcs->best_encoder is valid"), DRM internals can access/test some of > > the fields in mode_config->funcs as part of the fbdev registration > > process. > > Make sure dev->mode_config.funcs is properly set to avoid dereferencing > > a NULL pointer. > > That fixes the issues I am seeing. > > Tested-by: "Eric W. Biederman" > > > Signed-off-by: Boris Brezillon > > Fixes: c61b93fe51b1 ("drm/atomic: Fix remaining places where > > !funcs->best_encoder is valid") > > --- > > Hi Dave, > > > > As discussed on IRC, I'm sending this patch in a proper format. That's > > probably better to wait for Eric's feeback before applying it though. > > It is weird I didn't see either of your email messages directly only > through lkml. Weird. Probably because of that " : host mx.xmission.com[166.70.12.20] said: 550-XM-RJCT22: [37.187.137.238] is prohibited from connecting to XMission mail 550-servers due to high spam volume. See the following for more information: 550 http://postmaster.xmission.com/senders/rbls.php?check_rbl=Submit&address=37.187.137.238 (in reply to RCPT TO command) " I filled a form to add this IP to the xmission whitelist, let's see if you receive my answer :).
[PATCH] drm/cirrus: Fix NULL pointer dereference when registering the fbdev
Boris Brezillon writes: > cirrus_modeset_init() is initializing/registering the emulated fbdev > and, since commit c61b93fe51b1 ("drm/atomic: Fix remaining places where > !funcs->best_encoder is valid"), DRM internals can access/test some of > the fields in mode_config->funcs as part of the fbdev registration > process. > Make sure dev->mode_config.funcs is properly set to avoid dereferencing > a NULL pointer. That fixes the issues I am seeing. Tested-by: "Eric W. Biederman" > Signed-off-by: Boris Brezillon > Fixes: c61b93fe51b1 ("drm/atomic: Fix remaining places where > !funcs->best_encoder is valid") > --- > Hi Dave, > > As discussed on IRC, I'm sending this patch in a proper format. That's > probably better to wait for Eric's feeback before applying it though. It is weird I didn't see either of your email messages directly only through lkml. Weird. > Regards, > > Boris > --- > drivers/gpu/drm/cirrus/cirrus_main.c | 13 +++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c > b/drivers/gpu/drm/cirrus/cirrus_main.c > index 80446e2d3ab6..76bcb43e7c06 100644 > --- a/drivers/gpu/drm/cirrus/cirrus_main.c > +++ b/drivers/gpu/drm/cirrus/cirrus_main.c > @@ -185,14 +185,23 @@ int cirrus_driver_load(struct drm_device *dev, unsigned > long flags) > goto out; > } > > + /* > + * cirrus_modeset_init() is initializing/registering the emulated fbdev > + * and DRM internals can access/test some of the fields in > + * mode_config->funcs as part of the fbdev registration process. > + * Make sure dev->mode_config.funcs is properly set to avoid > + * dereferencing a NULL pointer. > + * FIXME: mode_config.funcs assignment should probably be done in > + * cirrus_modeset_init() (that's a common pattern seen in other DRM > + * drivers). > + */ > + dev->mode_config.funcs = &cirrus_mode_funcs; > r = cirrus_modeset_init(cdev); > if (r) { > dev_err(&dev->pdev->dev, "Fatal error during modeset init: > %d\n", r); > goto out; > } > > - dev->mode_config.funcs = (void *)&cirrus_mode_funcs; > - > return 0; > out: > cirrus_driver_unload(dev);