Skip to content

Commit 3c7df2e

Browse files
johnstultz-workmstsirkin
authored andcommitted
sound/virtio: Fix cancel_sync warnings on uninitialized work_structs
Betty reported hitting the following warning: [ 8.709131][ T221] WARNING: CPU: 2 PID: 221 at kernel/workqueue.c:4182 ... [ 8.713282][ T221] Call trace: [ 8.713365][ T221] __flush_work+0x8d0/0x914 [ 8.713468][ T221] __cancel_work_sync+0xac/0xfc [ 8.713570][ T221] cancel_work_sync+0x24/0x34 [ 8.713667][ T221] virtsnd_remove+0xa8/0xf8 [virtio_snd ab15f34d0dd772f6d11327e08a81d46dc9c36276] [ 8.713868][ T221] virtsnd_probe+0x48c/0x664 [virtio_snd ab15f34d0dd772f6d11327e08a81d46dc9c36276] [ 8.714035][ T221] virtio_dev_probe+0x28c/0x390 [ 8.714139][ T221] really_probe+0x1bc/0x4c8 ... It seems we're hitting the error path in virtsnd_probe(), which triggers a virtsnd_remove() which iterates over the substreams calling cancel_work_sync() on the elapsed_period work_struct. Looking at the code, from earlier in: virtsnd_probe()->virtsnd_build_devs()->virtsnd_pcm_parse_cfg() We set snd->nsubstreams, allocate the snd->substreams, and if we then hit an error on the info allocation or something in virtsnd_ctl_query_info() fails, we will exit without having initialized the elapsed_period work_struct. When that error path unwinds we then call virtsnd_remove() which as long as the substreams array is allocated, will iterate through calling cancel_work_sync() on the uninitialized work struct hitting this warning. Takashi Iwai suggested this fix, which initializes the substreams structure right after allocation, so that if we hit the error paths we avoid trying to cleanup uninitialized data. Note: I have not yet managed to reproduce the issue myself, so this patch has had limited testing. Feedback or thoughts would be appreciated! Cc: Anton Yakovlev <[email protected]> Cc: "Michael S. Tsirkin" <[email protected]> Cc: Jaroslav Kysela <[email protected]> Cc: Takashi Iwai <[email protected]> Cc: [email protected] Cc: [email protected] Cc: [email protected] Reported-by: Betty Zhou <[email protected]> Suggested-by: Takashi Iwai <[email protected]> Signed-off-by: John Stultz <[email protected]> Message-Id: <[email protected]> Signed-off-by: Michael S. Tsirkin <[email protected]>
1 parent a6097e0 commit 3c7df2e

File tree

1 file changed

+15
-6
lines changed

1 file changed

+15
-6
lines changed

sound/virtio/virtio_pcm.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,21 @@ int virtsnd_pcm_parse_cfg(struct virtio_snd *snd)
339339
if (!snd->substreams)
340340
return -ENOMEM;
341341

342+
/*
343+
* Initialize critical substream fields early in case we hit an
344+
* error path and end up trying to clean up uninitialized structures
345+
* elsewhere.
346+
*/
347+
for (i = 0; i < snd->nsubstreams; ++i) {
348+
struct virtio_pcm_substream *vss = &snd->substreams[i];
349+
350+
vss->snd = snd;
351+
vss->sid = i;
352+
INIT_WORK(&vss->elapsed_period, virtsnd_pcm_period_elapsed);
353+
init_waitqueue_head(&vss->msg_empty);
354+
spin_lock_init(&vss->lock);
355+
}
356+
342357
info = kcalloc(snd->nsubstreams, sizeof(*info), GFP_KERNEL);
343358
if (!info)
344359
return -ENOMEM;
@@ -352,12 +367,6 @@ int virtsnd_pcm_parse_cfg(struct virtio_snd *snd)
352367
struct virtio_pcm_substream *vss = &snd->substreams[i];
353368
struct virtio_pcm *vpcm;
354369

355-
vss->snd = snd;
356-
vss->sid = i;
357-
INIT_WORK(&vss->elapsed_period, virtsnd_pcm_period_elapsed);
358-
init_waitqueue_head(&vss->msg_empty);
359-
spin_lock_init(&vss->lock);
360-
361370
rc = virtsnd_pcm_build_hw(vss, &info[i]);
362371
if (rc)
363372
goto on_exit;

0 commit comments

Comments
 (0)