syzbot is hitting a lockdep warning [1] because flush_work() is called without INIT_WORK() after kzalloc() at vkms_atomic_crtc_reset().
Commit 6c234fe37c57627a ("drm/vkms: Implement CRC debugfs API") added INIT_WORK() to only vkms_atomic_crtc_duplicate_state() side. Assuming that lifecycle of crc_work is appropriately managed, fix this problem by adding INIT_WORK() to vkms_atomic_crtc_reset() side.
[1] https://syzkaller.appspot.com/bug?id=a5954455fcfa51c29ca2ab55b203076337e1c77...
Reported-and-tested-by: syzbot syzbot+12f1b031b6da017e34f8@syzkaller.appspotmail.com Signed-off-by: Tetsuo Handa penguin-kernel@I-love.SAKURA.ne.jp --- drivers/gpu/drm/vkms/vkms_crtc.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 177bbcb..3c37d8c 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -104,6 +104,7 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc) vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL); if (!vkms_state) return; + INIT_WORK(&vkms_state->crc_work, vkms_crc_work_handle);
crtc->state = &vkms_state->base; crtc->state->crtc = crtc;
syzbot is hitting a lockdep warning [1] because flush_work() is called without INIT_WORK() after kzalloc() at vkms_atomic_crtc_reset().
Commit 6c234fe37c57627a ("drm/vkms: Implement CRC debugfs API") added INIT_WORK() to only vkms_atomic_crtc_duplicate_state() side. Assuming that lifecycle of crc_work is appropriately managed, fix this problem by adding INIT_WORK() to vkms_atomic_crtc_reset() side.
[1] https://syzkaller.appspot.com/bug?id=a5954455fcfa51c29ca2ab55b203076337e1c77...
Reported-and-tested-by: syzbot syzbot+12f1b031b6da017e34f8@syzkaller.appspotmail.com Signed-off-by: Tetsuo Handa penguin-kernel@I-love.SAKURA.ne.jp
drivers/gpu/drm/vkms/vkms_crtc.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 177bbcb..3c37d8c 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -104,6 +104,7 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc) vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL); if (!vkms_state) return;
INIT_WORK(&vkms_state->crc_work, vkms_crc_work_handle); crtc->state = &vkms_state->base; crtc->state->crtc = crtc;-- 1.8.3.1
Thank you for your patch!
Reviewed-by: Shayenne Moura shayenneluzmoura@gmail.com
On Sat, Jan 19, 2019 at 01:43:43AM +0900, Tetsuo Handa wrote:
syzbot is hitting a lockdep warning [1] because flush_work() is called without INIT_WORK() after kzalloc() at vkms_atomic_crtc_reset().
Commit 6c234fe37c57627a ("drm/vkms: Implement CRC debugfs API") added INIT_WORK() to only vkms_atomic_crtc_duplicate_state() side. Assuming that lifecycle of crc_work is appropriately managed, fix this problem by adding INIT_WORK() to vkms_atomic_crtc_reset() side.
[1] https://syzkaller.appspot.com/bug?id=a5954455fcfa51c29ca2ab55b203076337e1c77...
Reported-and-tested-by: syzbot syzbot+12f1b031b6da017e34f8@syzkaller.appspotmail.com Signed-off-by: Tetsuo Handa penguin-kernel@I-love.SAKURA.ne.jp
drivers/gpu/drm/vkms/vkms_crtc.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 177bbcb..3c37d8c 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -104,6 +104,7 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc) vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL); if (!vkms_state) return;
- INIT_WORK(&vkms_state->crc_work, vkms_crc_work_handle);
Presumably we should migrate/duplicate the crc_work state from the old state rather than just re-initializing? My context on the crc_work is foggy, but we should be sure that either:
a) the existing crc work is flushed and inactive, or b) we migrate the existing crc work to the duplicated state to keep track of it
Sean
crtc->state = &vkms_state->base; crtc->state->crtc = crtc; -- 1.8.3.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Jan 25, 2019 at 09:46:15AM -0500, Sean Paul wrote:
On Sat, Jan 19, 2019 at 01:43:43AM +0900, Tetsuo Handa wrote:
syzbot is hitting a lockdep warning [1] because flush_work() is called without INIT_WORK() after kzalloc() at vkms_atomic_crtc_reset().
Commit 6c234fe37c57627a ("drm/vkms: Implement CRC debugfs API") added INIT_WORK() to only vkms_atomic_crtc_duplicate_state() side. Assuming that lifecycle of crc_work is appropriately managed, fix this problem by adding INIT_WORK() to vkms_atomic_crtc_reset() side.
[1] https://syzkaller.appspot.com/bug?id=a5954455fcfa51c29ca2ab55b203076337e1c77...
Reported-and-tested-by: syzbot syzbot+12f1b031b6da017e34f8@syzkaller.appspotmail.com Signed-off-by: Tetsuo Handa penguin-kernel@I-love.SAKURA.ne.jp
drivers/gpu/drm/vkms/vkms_crtc.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 177bbcb..3c37d8c 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -104,6 +104,7 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc) vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL); if (!vkms_state) return;
- INIT_WORK(&vkms_state->crc_work, vkms_crc_work_handle);
Presumably we should migrate/duplicate the crc_work state from the old state rather than just re-initializing? My context on the crc_work is foggy, but we should be sure that either:
a) the existing crc work is flushed and inactive, or b) we migrate the existing crc work to the duplicated state to keep track of it
The various reset callbacks are used at driver load time to initialize atomic kms state. Once we do have atomic states we're only using the various duplicate_state callbacks, which already have the INIT_WORK here. So was missing once at driver load, and we've hit it once on the first frame. There is not existing/preceeding crc work when this code is called. -Daniel
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Sean Paul -
Shayenne Moura -
Tetsuo Handa