| Message ID | 20200901215942.2559119-2-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 22f760941844dbcee6ee446e1896532f6dff01ef |
| Headers | show |
| Series | A couple drm/msm fixes | expand |
On 2020-09-01 14:59, Stephen Boyd wrote: > The cstate->num_mixers member is only set to a non-zero value once > dpu_encoder_virt_mode_set() is called, but the atomic check function > can > be called by userspace before that. Let's avoid the div-by-zero here > and > inside _dpu_crtc_setup_lm_bounds() by skipping this part of the atomic > check if dpu_encoder_virt_mode_set() hasn't been called yet. This fixes > an UBSAN warning: > > UBSAN: Undefined behaviour in > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:860:31 > division by zero > CPU: 7 PID: 409 Comm: frecon Tainted: G S 5.4.31 #128 > Hardware name: Google Trogdor (rev0) (DT) > Call trace: > dump_backtrace+0x0/0x14c > show_stack+0x20/0x2c > dump_stack+0xa0/0xd8 > __ubsan_handle_divrem_overflow+0xec/0x110 > dpu_crtc_atomic_check+0x97c/0x9d4 > drm_atomic_helper_check_planes+0x160/0x1c8 > drm_atomic_helper_check+0x54/0xbc > drm_atomic_check_only+0x6a8/0x880 > drm_atomic_commit+0x20/0x5c > drm_atomic_helper_set_config+0x98/0xa0 > drm_mode_setcrtc+0x308/0x5dc > drm_ioctl_kernel+0x9c/0x114 > drm_ioctl+0x2ac/0x4b0 > drm_compat_ioctl+0xe8/0x13c > __arm64_compat_sys_ioctl+0x184/0x324 > el0_svc_common+0xa4/0x154 > el0_svc_compat_handler+0x > > Cc: Abhinav Kumar <abhinavk@codeaurora.org> > Cc: Jeykumar Sankaran <jsanka@codeaurora.org> > Cc: Jordan Crouse <jcrouse@codeaurora.org> > Cc: Sean Paul <seanpaul@chromium.org> > Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") > Signed-off-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index f272a8d0f95b..74294b5ed93f 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -881,7 +881,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc > *crtc, > struct drm_plane *plane; > struct drm_display_mode *mode; > > - int cnt = 0, rc = 0, mixer_width, i, z_pos; > + int cnt = 0, rc = 0, mixer_width = 0, i, z_pos; > > struct dpu_multirect_plane_states multirect_plane[DPU_STAGE_MAX * 2]; > int multirect_count = 0; > @@ -914,9 +914,11 @@ static int dpu_crtc_atomic_check(struct drm_crtc > *crtc, > > memset(pipe_staged, 0, sizeof(pipe_staged)); > > - mixer_width = mode->hdisplay / cstate->num_mixers; > + if (cstate->num_mixers) { > + mixer_width = mode->hdisplay / cstate->num_mixers; > > - _dpu_crtc_setup_lm_bounds(crtc, state); > + _dpu_crtc_setup_lm_bounds(crtc, state); > + } > > crtc_rect.x2 = mode->hdisplay; > crtc_rect.y2 = mode->vdisplay;
On 2020-09-02 03:29, Stephen Boyd wrote: > The cstate->num_mixers member is only set to a non-zero value once > dpu_encoder_virt_mode_set() is called, but the atomic check function > can > be called by userspace before that. Let's avoid the div-by-zero here > and > inside _dpu_crtc_setup_lm_bounds() by skipping this part of the atomic > check if dpu_encoder_virt_mode_set() hasn't been called yet. This fixes > an UBSAN warning: > > UBSAN: Undefined behaviour in > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:860:31 > division by zero > CPU: 7 PID: 409 Comm: frecon Tainted: G S 5.4.31 #128 > Hardware name: Google Trogdor (rev0) (DT) > Call trace: > dump_backtrace+0x0/0x14c > show_stack+0x20/0x2c > dump_stack+0xa0/0xd8 > __ubsan_handle_divrem_overflow+0xec/0x110 > dpu_crtc_atomic_check+0x97c/0x9d4 > drm_atomic_helper_check_planes+0x160/0x1c8 > drm_atomic_helper_check+0x54/0xbc > drm_atomic_check_only+0x6a8/0x880 > drm_atomic_commit+0x20/0x5c > drm_atomic_helper_set_config+0x98/0xa0 > drm_mode_setcrtc+0x308/0x5dc > drm_ioctl_kernel+0x9c/0x114 > drm_ioctl+0x2ac/0x4b0 > drm_compat_ioctl+0xe8/0x13c > __arm64_compat_sys_ioctl+0x184/0x324 > el0_svc_common+0xa4/0x154 > el0_svc_compat_handler+0x > > Cc: Abhinav Kumar <abhinavk@codeaurora.org> > Cc: Jeykumar Sankaran <jsanka@codeaurora.org> > Cc: Jordan Crouse <jcrouse@codeaurora.org> > Cc: Sean Paul <seanpaul@chromium.org> > Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index f272a8d0f95b..74294b5ed93f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -881,7 +881,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc, struct drm_plane *plane; struct drm_display_mode *mode; - int cnt = 0, rc = 0, mixer_width, i, z_pos; + int cnt = 0, rc = 0, mixer_width = 0, i, z_pos; struct dpu_multirect_plane_states multirect_plane[DPU_STAGE_MAX * 2]; int multirect_count = 0; @@ -914,9 +914,11 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc, memset(pipe_staged, 0, sizeof(pipe_staged)); - mixer_width = mode->hdisplay / cstate->num_mixers; + if (cstate->num_mixers) { + mixer_width = mode->hdisplay / cstate->num_mixers; - _dpu_crtc_setup_lm_bounds(crtc, state); + _dpu_crtc_setup_lm_bounds(crtc, state); + } crtc_rect.x2 = mode->hdisplay; crtc_rect.y2 = mode->vdisplay;
The cstate->num_mixers member is only set to a non-zero value once dpu_encoder_virt_mode_set() is called, but the atomic check function can be called by userspace before that. Let's avoid the div-by-zero here and inside _dpu_crtc_setup_lm_bounds() by skipping this part of the atomic check if dpu_encoder_virt_mode_set() hasn't been called yet. This fixes an UBSAN warning: UBSAN: Undefined behaviour in drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:860:31 division by zero CPU: 7 PID: 409 Comm: frecon Tainted: G S 5.4.31 #128 Hardware name: Google Trogdor (rev0) (DT) Call trace: dump_backtrace+0x0/0x14c show_stack+0x20/0x2c dump_stack+0xa0/0xd8 __ubsan_handle_divrem_overflow+0xec/0x110 dpu_crtc_atomic_check+0x97c/0x9d4 drm_atomic_helper_check_planes+0x160/0x1c8 drm_atomic_helper_check+0x54/0xbc drm_atomic_check_only+0x6a8/0x880 drm_atomic_commit+0x20/0x5c drm_atomic_helper_set_config+0x98/0xa0 drm_mode_setcrtc+0x308/0x5dc drm_ioctl_kernel+0x9c/0x114 drm_ioctl+0x2ac/0x4b0 drm_compat_ioctl+0xe8/0x13c __arm64_compat_sys_ioctl+0x184/0x324 el0_svc_common+0xa4/0x154 el0_svc_compat_handler+0x Cc: Abhinav Kumar <abhinavk@codeaurora.org> Cc: Jeykumar Sankaran <jsanka@codeaurora.org> Cc: Jordan Crouse <jcrouse@codeaurora.org> Cc: Sean Paul <seanpaul@chromium.org> Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support") Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)