Jia-Ju Bai
2019-Jul-25 10:21 UTC
[PATCH] gpu: drm: qxl: Fix possible null-pointer dereferences in qxl_crtc_atomic_flush()
In qxl_crtc_atomic_flush(), there is an if statement on line 376 to check whether crtc->state is NULL: if (crtc->state && crtc->state->event) When crtc->state is NULL and qxl_crtc_update_monitors_config() is call, qxl_crtc_update_monitors_config() uses crtc->state on line 326: if (crtc->state->active) and on line 358: DRM_DEBUG_KMS(..., crtc->state->active, ...); Thus, possible null-pointer dereferences may occur. To fix these bugs, crtc->state is checked before calling qxl_crtc_update_monitors_config(). These bugs are found by a static analysis tool STCheck written by us. Signed-off-by: Jia-Ju Bai <baijiaju1990 at gmail.com> --- drivers/gpu/drm/qxl/qxl_display.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 8b319ebbb0fb..fae18ef1ba59 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -382,7 +382,8 @@ static void qxl_crtc_atomic_flush(struct drm_crtc *crtc, spin_unlock_irqrestore(&dev->event_lock, flags); } - qxl_crtc_update_monitors_config(crtc, "flush"); + if (crtc->state) + qxl_crtc_update_monitors_config(crtc, "flush"); } static void qxl_crtc_destroy(struct drm_crtc *crtc) -- 2.17.0
Daniel Vetter
2019-Jul-25 12:03 UTC
[PATCH] gpu: drm: qxl: Fix possible null-pointer dereferences in qxl_crtc_atomic_flush()
On Thu, Jul 25, 2019 at 06:21:27PM +0800, Jia-Ju Bai wrote:> In qxl_crtc_atomic_flush(), there is an if statement on line 376 to > check whether crtc->state is NULL: > if (crtc->state && crtc->state->event) > > When crtc->state is NULL and qxl_crtc_update_monitors_config() is call, > qxl_crtc_update_monitors_config() uses crtc->state on line 326: > if (crtc->state->active) > and on line 358: > DRM_DEBUG_KMS(..., crtc->state->active, ...); > > Thus, possible null-pointer dereferences may occur. > > To fix these bugs, crtc->state is checked before calling > qxl_crtc_update_monitors_config(). > > These bugs are found by a static analysis tool STCheck written by us. > > Signed-off-by: Jia-Ju Bai <baijiaju1990 at gmail.com>crtc->state should never be NULL in this function, ever. Imo correct fix is to remove that other NULL check (since obviously it would blow up, hence it's dead code). Atomic kms drivers use drm_mode_config_reset() to make sure the various ->state pointers are always set and valid. -Daniel> --- > drivers/gpu/drm/qxl/qxl_display.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c > index 8b319ebbb0fb..fae18ef1ba59 100644 > --- a/drivers/gpu/drm/qxl/qxl_display.c > +++ b/drivers/gpu/drm/qxl/qxl_display.c > @@ -382,7 +382,8 @@ static void qxl_crtc_atomic_flush(struct drm_crtc *crtc, > spin_unlock_irqrestore(&dev->event_lock, flags); > } > > - qxl_crtc_update_monitors_config(crtc, "flush"); > + if (crtc->state) > + qxl_crtc_update_monitors_config(crtc, "flush"); > } > > static void qxl_crtc_destroy(struct drm_crtc *crtc) > -- > 2.17.0 >-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Seemingly Similar Threads
- [PATCH 3/4] qxl: hook monitors_config updates into crtc, not encoder.
- [PATCH v2 3/4] qxl: hook monitors_config updates into crtc, not encoder.
- [PATCH v2 4/4] qxl: drop dummy functions
- [PATCH 6/6] drm/qxl: use qxl_num_crtc directly
- [PATCH] drm/qxl: use qxl_num_crtc directly