[PATCH] drm/vkms: Fix flush_work() without INIT_WORK().

Daniel Vetter daniel at ffwll.ch
Fri Jan 25 16:07:39 UTC 2019


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=a5954455fcfa51c29ca2ab55b203076337e1c770
> > 
> > Reported-and-tested-by: syzbot <syzbot+12f1b031b6da017e34f8 at syzkaller.appspotmail.com>
> > Signed-off-by: Tetsuo Handa <penguin-kernel at 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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the dri-devel mailing list