[bug report] potential deadlock in snd_emu10k1_voice_alloc()

[bug report] potential deadlock in snd_emu10k1_voice_alloc()

[Dan Carpenter]

Hello ALSA devs,

The patch 1da177e4c3f4: "Linux-2.6.12-rc2" from Apr 16, 2005, leads
to the following static checker warning:

	sound/pci/emu10k1/voice.c:112 snd_emu10k1_voice_alloc()
	warn: called with lock held.  '&emu->voice_lock'

sound/pci/emu10k1/voice.c
   101  int snd_emu10k1_voice_alloc(struct snd_emu10k1 *emu, int type, int number,
   102                              struct snd_emu10k1_voice **rvoice)
   103  {
   104          unsigned long flags;
   105          int result;
   106  
   107          if (snd_BUG_ON(!rvoice))
   108                  return -EINVAL;
   109          if (snd_BUG_ON(!number))
   110                  return -EINVAL;
   111  
   112          spin_lock_irqsave(&emu->voice_lock, flags);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This lock is already held in the caller.

   113          for (;;) {
   114                  result = voice_alloc(emu, type, number, rvoice);
   115                  if (result == 0 || type == EMU10K1_SYNTH || type == EMU10K1_MIDI)
   116                          break;
   117  
   118                  /* free a voice from synth */
   119                  if (emu->get_synth_voice) {

The call tree is:

	 snd_emux_note_on()
		takes the lock: spin_lock_irqsave(&emu->voice_lock, flags);
		calls vp = emu->ops.get_voice(emu, port);
	   --> get_voice()
		--> snd_emu10k1_voice_alloc()

regards,
dan carpenter

Re: [bug report] potential deadlock in snd_emu10k1_voice_alloc()

[Takashi Iwai]

On Tue, 02 Feb 2021 06:56:08 +0100,
Dan Carpenter wrote:
> 
> Hello ALSA devs,
> 
> The patch 1da177e4c3f4: "Linux-2.6.12-rc2" from Apr 16, 2005, leads
> to the following static checker warning:
> 
> 	sound/pci/emu10k1/voice.c:112 snd_emu10k1_voice_alloc()
> 	warn: called with lock held.  '&emu->voice_lock'
> 
> sound/pci/emu10k1/voice.c
>    101  int snd_emu10k1_voice_alloc(struct snd_emu10k1 *emu, int type, int number,
>    102                              struct snd_emu10k1_voice **rvoice)
>    103  {
>    104          unsigned long flags;
>    105          int result;
>    106  
>    107          if (snd_BUG_ON(!rvoice))
>    108                  return -EINVAL;
>    109          if (snd_BUG_ON(!number))
>    110                  return -EINVAL;
>    111  
>    112          spin_lock_irqsave(&emu->voice_lock, flags);
>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This lock is already held in the caller.
> 
>    113          for (;;) {
>    114                  result = voice_alloc(emu, type, number, rvoice);
>    115                  if (result == 0 || type == EMU10K1_SYNTH || type == EMU10K1_MIDI)
>    116                          break;
>    117  
>    118                  /* free a voice from synth */
>    119                  if (emu->get_synth_voice) {
> 
> The call tree is:
> 
> 	 snd_emux_note_on()
> 		takes the lock: spin_lock_irqsave(&emu->voice_lock, flags);
> 		calls vp = emu->ops.get_voice(emu, port);
> 	   --> get_voice()
> 		--> snd_emu10k1_voice_alloc()

This report looks like a false-positive, too.
Both are from different structs, one is from struct snd_emu10k1 and
another is struct snd_emux.


thanks,

Takashi