| Message ID | 20181203195604.65338-1-sean@poorly.run (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | drm/msm: dpu: Allocate proper amount for dpu_crtc_state | expand |
On Mon, Dec 3, 2018 at 2:56 PM Sean Paul <sean@poorly.run> wrote: > > From: Sean Paul <seanpaul@chromium.org> > > Since dpu_crtc subclasses crtc_state, we need a custom .reset hook in > order to allocate the right amount of memory to accommodate the > additional struct members in dpu_crtc_state. So bring it [partially] > back. > > Relevant KASAN splat: > [ 10.333382] ================================================================== > [ 10.344288] BUG: KASAN: slab-out-of-bounds in kmemdup+0x50/0x80 > [ 10.350390] Read of size 736 at addr ffffffc0d9f06080 by task frecon/394 > > [ 10.358861] CPU: 6 PID: 394 Comm: frecon Tainted: G W 4.19.4 #121 > [ 10.366476] Hardware name: Google Cheza (rev2) (DT) > [ 10.371514] Call trace: > [ 10.374087] dump_backtrace+0x0/0x194 > [ 10.377878] show_stack+0x20/0x28 > [ 10.381330] dump_stack+0xa0/0xc8 > [ 10.384783] print_address_description+0x78/0x2e0 > [ 10.389639] kasan_report+0x290/0x2d0 > [ 10.393428] check_memory_region+0x20/0x14c > [ 10.397740] __asan_loadN+0x14/0x1c > [ 10.401345] kmemdup+0x50/0x80 > [ 10.404524] dpu_crtc_duplicate_state+0x58/0xa0 > [ 10.409228] drm_atomic_get_crtc_state+0xac/0x178 > [ 10.414095] __drm_atomic_helper_set_config+0x54/0x4a4 > [ 10.419393] drm_atomic_helper_set_config+0x60/0xb4 > [ 10.424435] drm_mode_setcrtc+0x720/0x760 > [ 10.428570] drm_ioctl_kernel+0xd8/0x13c > [ 10.432617] drm_ioctl+0x380/0x4f4 > [ 10.436150] drm_compat_ioctl+0x54/0x13c > [ 10.440219] __arm64_compat_sys_ioctl+0x1d8/0xef4 > [ 10.445086] el0_svc_common+0xd8/0x138 > [ 10.448961] el0_svc_compat_handler+0x58/0x68 > [ 10.453463] el0_svc_compat+0x8/0x18 > > [ 10.458712] Allocated by task 56: > [ 10.462148] kasan_kmalloc.part.4+0x48/0xf4 > [ 10.466465] kasan_kmalloc+0x8c/0xa0 > [ 10.470165] kmem_cache_alloc_trace+0x25c/0x27c > [ 10.474848] drm_atomic_helper_crtc_reset+0x68/0x98 > [ 10.479877] drm_mode_config_reset+0xc4/0x19c > [ 10.484383] msm_drm_bind+0x814/0x8dc > [ 10.488169] try_to_bring_up_master.part.7+0x48/0xac > [ 10.493282] component_master_add_with_match+0x158/0x198 > [ 10.498758] msm_pdev_probe+0x328/0x348 > [ 10.502736] platform_drv_probe+0x74/0xc8 > [ 10.506877] really_probe+0x1ac/0x35c > [ 10.510659] driver_probe_device+0xd4/0x118 > [ 10.514975] __device_attach_driver+0xc8/0xf4 > [ 10.519477] bus_for_each_drv+0xb4/0xe4 > [ 10.523439] __device_attach+0xd0/0x158 > [ 10.527394] device_initial_probe+0x24/0x30 > [ 10.531715] bus_probe_device+0x50/0xe4 > [ 10.535681] deferred_probe_work_func+0xac/0xdc > [ 10.540376] process_one_work+0x3f0/0x6d4 > [ 10.544521] worker_thread+0x3f4/0x520 > [ 10.548399] kthread+0x1b4/0x1c8 > [ 10.551740] ret_from_fork+0x10/0x18 > > [ 10.556986] Freed by task 0: > [ 10.559967] (stack is not available) > > [ 10.565216] The buggy address belongs to the object at ffffffc0d9f06080 > which belongs to the cache kmalloc-1024 of size 1024 > [ 10.578268] The buggy address is located 0 bytes inside of > 1024-byte region [ffffffc0d9f06080, ffffffc0d9f06480) > [ 10.590248] The buggy address belongs to the page: > [ 10.595195] page:ffffffbf0367c000 count:1 mapcount:0 mapping:ffffffc0de40f680 index:0x0 compound_mapcount: 0 > [ 10.605321] flags: 0x4000000000008100(slab|head) > [ 10.610100] raw: 4000000000008100 ffffffbf0369fa08 ffffffbf0367f008 ffffffc0de40f680 > [ 10.618077] raw: 0000000000000000 0000000000150015 00000001ffffffff 0000000000000000 > [ 10.626049] page dumped because: kasan: bad access detected > > [ 10.633341] Memory state around the buggy address: > [ 10.638282] ffffffc0d9f06180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > [ 10.645710] ffffffc0d9f06200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > [ 10.653139] >ffffffc0d9f06280: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc > [ 10.660571] ^ > [ 10.665774] ffffffc0d9f06300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > [ 10.673210] ffffffc0d9f06380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > [ 10.680639] ================================================================== > > Fixes: a6ba45afda41 ("drm/msm/dpu: Replace dpu_crtc_reset by atomic helper") > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Bruce Wang <bzwang@chromium.org> > Cc: Rob Clark <robdclark@gmail.com> > Signed-off-by: Sean Paul <seanpaul@chromium.org> Reviewed-by: Bruce Wang <bzwang@chromium.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 1e3e57817b72b..d27ea2a95f85b 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -819,6 +819,18 @@ static void _dpu_crtc_vblank_enable_no_lock( > } > } > > +static void dpu_crtc_reset(struct drm_crtc *crtc) > +{ > + struct dpu_crtc_state *cstate; > + > + if (crtc->state) > + dpu_crtc_destroy_state(crtc, crtc->state); > + > + crtc->state = kzalloc(sizeof(*cstate), GFP_KERNEL); > + if (crtc->state) > + crtc->state->crtc = crtc; > +} > + > /** > * dpu_crtc_duplicate_state - state duplicate hook > * @crtc: Pointer to drm crtc structure > @@ -1466,7 +1478,7 @@ static const struct drm_crtc_funcs dpu_crtc_funcs = { > .set_config = drm_atomic_helper_set_config, > .destroy = dpu_crtc_destroy, > .page_flip = drm_atomic_helper_page_flip, > - .reset = drm_atomic_helper_crtc_reset, > + .reset = dpu_crtc_reset, > .atomic_duplicate_state = dpu_crtc_duplicate_state, > .atomic_destroy_state = dpu_crtc_destroy_state, > .late_register = dpu_crtc_late_register, > -- > Sean Paul, Software Engineer, Google / Chromium OS >
On Tue, Dec 04, 2018 at 10:51:42AM -0500, Bruce Wang wrote: > On Mon, Dec 3, 2018 at 2:56 PM Sean Paul <sean@poorly.run> wrote: > > > > From: Sean Paul <seanpaul@chromium.org> > > > > Since dpu_crtc subclasses crtc_state, we need a custom .reset hook in > > order to allocate the right amount of memory to accommodate the > > additional struct members in dpu_crtc_state. So bring it [partially] > > back. > > > > Relevant KASAN splat: > > [ 10.333382] ================================================================== > > [ 10.344288] BUG: KASAN: slab-out-of-bounds in kmemdup+0x50/0x80 > > [ 10.350390] Read of size 736 at addr ffffffc0d9f06080 by task frecon/394 > > > > [ 10.358861] CPU: 6 PID: 394 Comm: frecon Tainted: G W 4.19.4 #121 > > [ 10.366476] Hardware name: Google Cheza (rev2) (DT) > > [ 10.371514] Call trace: > > [ 10.374087] dump_backtrace+0x0/0x194 > > [ 10.377878] show_stack+0x20/0x28 > > [ 10.381330] dump_stack+0xa0/0xc8 > > [ 10.384783] print_address_description+0x78/0x2e0 > > [ 10.389639] kasan_report+0x290/0x2d0 > > [ 10.393428] check_memory_region+0x20/0x14c > > [ 10.397740] __asan_loadN+0x14/0x1c > > [ 10.401345] kmemdup+0x50/0x80 > > [ 10.404524] dpu_crtc_duplicate_state+0x58/0xa0 > > [ 10.409228] drm_atomic_get_crtc_state+0xac/0x178 > > [ 10.414095] __drm_atomic_helper_set_config+0x54/0x4a4 > > [ 10.419393] drm_atomic_helper_set_config+0x60/0xb4 > > [ 10.424435] drm_mode_setcrtc+0x720/0x760 > > [ 10.428570] drm_ioctl_kernel+0xd8/0x13c > > [ 10.432617] drm_ioctl+0x380/0x4f4 > > [ 10.436150] drm_compat_ioctl+0x54/0x13c > > [ 10.440219] __arm64_compat_sys_ioctl+0x1d8/0xef4 > > [ 10.445086] el0_svc_common+0xd8/0x138 > > [ 10.448961] el0_svc_compat_handler+0x58/0x68 > > [ 10.453463] el0_svc_compat+0x8/0x18 > > > > [ 10.458712] Allocated by task 56: > > [ 10.462148] kasan_kmalloc.part.4+0x48/0xf4 > > [ 10.466465] kasan_kmalloc+0x8c/0xa0 > > [ 10.470165] kmem_cache_alloc_trace+0x25c/0x27c > > [ 10.474848] drm_atomic_helper_crtc_reset+0x68/0x98 > > [ 10.479877] drm_mode_config_reset+0xc4/0x19c > > [ 10.484383] msm_drm_bind+0x814/0x8dc > > [ 10.488169] try_to_bring_up_master.part.7+0x48/0xac > > [ 10.493282] component_master_add_with_match+0x158/0x198 > > [ 10.498758] msm_pdev_probe+0x328/0x348 > > [ 10.502736] platform_drv_probe+0x74/0xc8 > > [ 10.506877] really_probe+0x1ac/0x35c > > [ 10.510659] driver_probe_device+0xd4/0x118 > > [ 10.514975] __device_attach_driver+0xc8/0xf4 > > [ 10.519477] bus_for_each_drv+0xb4/0xe4 > > [ 10.523439] __device_attach+0xd0/0x158 > > [ 10.527394] device_initial_probe+0x24/0x30 > > [ 10.531715] bus_probe_device+0x50/0xe4 > > [ 10.535681] deferred_probe_work_func+0xac/0xdc > > [ 10.540376] process_one_work+0x3f0/0x6d4 > > [ 10.544521] worker_thread+0x3f4/0x520 > > [ 10.548399] kthread+0x1b4/0x1c8 > > [ 10.551740] ret_from_fork+0x10/0x18 > > > > [ 10.556986] Freed by task 0: > > [ 10.559967] (stack is not available) > > > > [ 10.565216] The buggy address belongs to the object at ffffffc0d9f06080 > > which belongs to the cache kmalloc-1024 of size 1024 > > [ 10.578268] The buggy address is located 0 bytes inside of > > 1024-byte region [ffffffc0d9f06080, ffffffc0d9f06480) > > [ 10.590248] The buggy address belongs to the page: > > [ 10.595195] page:ffffffbf0367c000 count:1 mapcount:0 mapping:ffffffc0de40f680 index:0x0 compound_mapcount: 0 > > [ 10.605321] flags: 0x4000000000008100(slab|head) > > [ 10.610100] raw: 4000000000008100 ffffffbf0369fa08 ffffffbf0367f008 ffffffc0de40f680 > > [ 10.618077] raw: 0000000000000000 0000000000150015 00000001ffffffff 0000000000000000 > > [ 10.626049] page dumped because: kasan: bad access detected > > > > [ 10.633341] Memory state around the buggy address: > > [ 10.638282] ffffffc0d9f06180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 10.645710] ffffffc0d9f06200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 10.653139] >ffffffc0d9f06280: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc > > [ 10.660571] ^ > > [ 10.665774] ffffffc0d9f06300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > [ 10.673210] ffffffc0d9f06380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > [ 10.680639] ================================================================== > > > > Fixes: a6ba45afda41 ("drm/msm/dpu: Replace dpu_crtc_reset by atomic helper") > > Cc: Sean Paul <seanpaul@chromium.org> > > Cc: Bruce Wang <bzwang@chromium.org> > > Cc: Rob Clark <robdclark@gmail.com> > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > > Reviewed-by: Bruce Wang <bzwang@chromium.org> Thanks for the review, this has been pushed to dpu-staging/for-next Sean > > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > index 1e3e57817b72b..d27ea2a95f85b 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > @@ -819,6 +819,18 @@ static void _dpu_crtc_vblank_enable_no_lock( > > } > > } > > > > +static void dpu_crtc_reset(struct drm_crtc *crtc) > > +{ > > + struct dpu_crtc_state *cstate; > > + > > + if (crtc->state) > > + dpu_crtc_destroy_state(crtc, crtc->state); > > + > > + crtc->state = kzalloc(sizeof(*cstate), GFP_KERNEL); > > + if (crtc->state) > > + crtc->state->crtc = crtc; > > +} > > + > > /** > > * dpu_crtc_duplicate_state - state duplicate hook > > * @crtc: Pointer to drm crtc structure > > @@ -1466,7 +1478,7 @@ static const struct drm_crtc_funcs dpu_crtc_funcs = { > > .set_config = drm_atomic_helper_set_config, > > .destroy = dpu_crtc_destroy, > > .page_flip = drm_atomic_helper_page_flip, > > - .reset = drm_atomic_helper_crtc_reset, > > + .reset = dpu_crtc_reset, > > .atomic_duplicate_state = dpu_crtc_duplicate_state, > > .atomic_destroy_state = dpu_crtc_destroy_state, > > .late_register = dpu_crtc_late_register, > > -- > > Sean Paul, Software Engineer, Google / Chromium OS > >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 1e3e57817b72b..d27ea2a95f85b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -819,6 +819,18 @@ static void _dpu_crtc_vblank_enable_no_lock( } } +static void dpu_crtc_reset(struct drm_crtc *crtc) +{ + struct dpu_crtc_state *cstate; + + if (crtc->state) + dpu_crtc_destroy_state(crtc, crtc->state); + + crtc->state = kzalloc(sizeof(*cstate), GFP_KERNEL); + if (crtc->state) + crtc->state->crtc = crtc; +} + /** * dpu_crtc_duplicate_state - state duplicate hook * @crtc: Pointer to drm crtc structure @@ -1466,7 +1478,7 @@ static const struct drm_crtc_funcs dpu_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .destroy = dpu_crtc_destroy, .page_flip = drm_atomic_helper_page_flip, - .reset = drm_atomic_helper_crtc_reset, + .reset = dpu_crtc_reset, .atomic_duplicate_state = dpu_crtc_duplicate_state, .atomic_destroy_state = dpu_crtc_destroy_state, .late_register = dpu_crtc_late_register,