On Mon, 19 Sep 2022 14:46:13 +0200, Rondreis wrote:
Hello,
When fuzzing the Linux kernel driver v6.0-rc4, the following crash was triggered.
HEAD commit: 7e18e42e4b280c85b76967a9106a13ca61c16179 git tree: upstream
kernel config: https://pastebin.com/raw/xtrgsXP3 console output: https://pastebin.com/raw/9tabWDtu
Sorry for failing to extract the reproducer, and the crash occurred at the moment of disconnecting the midi device. On other versions of Linux, I also triggered this crash.
I would appreciate it if you have any idea how to solve this bug.
I think there are two ways to work around it.
The first one is to move the unregister_sound*() calls out of the sound_oss_mutex, something like: -- 8< --
--- a/sound/core/sound_oss.c +++ b/sound/core/sound_oss.c @@ -162,7 +162,6 @@ int snd_unregister_oss_device(int type, struct snd_card *card, int dev) mutex_unlock(&sound_oss_mutex); return -ENOENT; } - unregister_sound_special(minor); switch (SNDRV_MINOR_OSS_DEVICE(minor)) { case SNDRV_MINOR_OSS_PCM: track2 = SNDRV_MINOR_OSS(cidx, SNDRV_MINOR_OSS_AUDIO); @@ -174,12 +173,18 @@ int snd_unregister_oss_device(int type, struct snd_card *card, int dev) track2 = SNDRV_MINOR_OSS(cidx, SNDRV_MINOR_OSS_DMMIDI1); break; } - if (track2 >= 0) { - unregister_sound_special(track2); + if (track2 >= 0) snd_oss_minors[track2] = NULL; - } snd_oss_minors[minor] = NULL; mutex_unlock(&sound_oss_mutex); + + /* call unregister_sound_special() outside sound_oss_mutex; + * otherwise may deadlock, as it can trigger the release of a card + */ + unregister_sound_special(minor); + if (track2 >= 0) + unregister_sound_special(track2); + kfree(mptr); return 0; } -- 8< --
This should be OK, as the unregister_sound_*() itself can be called concurrently.
Another workaround would be just to remove the register_mutex call at snd_rawmidi_free(), e.g. something like:
-- 8< -- --- a/sound/core/rawmidi.c +++ b/sound/core/rawmidi.c @@ -1899,10 +1899,8 @@ static int snd_rawmidi_free(struct snd_rawmidi *rmidi)
snd_info_free_entry(rmidi->proc_entry); rmidi->proc_entry = NULL; - mutex_lock(®ister_mutex); if (rmidi->ops && rmidi->ops->dev_unregister) rmidi->ops->dev_unregister(rmidi); - mutex_unlock(®ister_mutex);
snd_rawmidi_free_substreams(&rmidi->streams[SNDRV_RAWMIDI_STREAM_INPUT]); snd_rawmidi_free_substreams(&rmidi->streams[SNDRV_RAWMIDI_STREAM_OUTPUT]); -- 8< --
This register_mutex there should be superfluous since the device has been already processed and detached by snd_rawmidi_dev_disconnect() beforehand. But if the first one is confirmed to work, the second one can be left untouched.
thanks,
Takashi