lkml.org 
[lkml]   [2022]   [Sep]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: possible deadlock in snd_rawmidi_free
Thank you for your solution! I am sorry that I was not able to extract
the reproducer. But I am improving the solution to reproduce the crash.
If I succeed in reproducing it, I will check it for you as soon as possible!

Best Regards,
Rondreis

On Tue, Sep 20, 2022 at 7:35 PM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Mon, 19 Sep 2022 15:49:02 +0200,
> Takashi Iwai wrote:
> >
> > 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(&register_mutex);
> > if (rmidi->ops && rmidi->ops->dev_unregister)
> > rmidi->ops->dev_unregister(rmidi);
> > - mutex_unlock(&register_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.
>
> Could you check whether one of two changes above fixes the bug?
> Once after confirmed, I'll cook a proper patch for the submission.
>
>
> thanks,
>
> Takashi

\
 
 \ /
  Last update: 2022-09-21 17:43    [W:0.048 / U:0.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site