Hi,

On Fri, Jan 06, 2023 at 12:31:12PM +0300, Dan Carpenter wrote:

Hello Takashi Sakamoto,

The patch 634ec0b2906e: "ALSA: firewire-motu: notify event for parameter change in register DSP model" from Oct 15, 2021, leads to the following Smatch static checker warning:

sound/firewire/motu/motu-hwdep.c:92 hwdep_read() warn: inconsistent returns '&motu->lock'.

Indeed. When no event is available, the bug appears. Later, I'll post quick fix. Thanks.

sound/firewire/motu/motu-hwdep.c 27 static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf, long count, 28 loff_t *offset) 29 { 30 struct snd_motu *motu = hwdep->private_data; 31 DEFINE_WAIT(wait); 32 union snd_firewire_event event; 33 34 spin_lock_irq(&motu->lock); 35 36 while (!motu->dev_lock_changed && motu->msg == 0 && !has_dsp_event(motu)) { 37 prepare_to_wait(&motu->hwdep_wait, &wait, TASK_INTERRUPTIBLE); 38 spin_unlock_irq(&motu->lock); 39 schedule(); 40 finish_wait(&motu->hwdep_wait, &wait); 41 if (signal_pending(current)) 42 return -ERESTARTSYS; 43 spin_lock_irq(&motu->lock); 44 } 45 46 memset(&event, 0, sizeof(event)); 47 if (motu->dev_lock_changed) { 48 event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS; 49 event.lock_status.status = (motu->dev_lock_count > 0); 50 motu->dev_lock_changed = false; 51 spin_unlock_irq(&motu->lock); 52 53 count = min_t(long, count, sizeof(event)); 54 if (copy_to_user(buf, &event, count)) 55 return -EFAULT; 56 } else if (motu->msg > 0) { 57 event.motu_notification.type = SNDRV_FIREWIRE_EVENT_MOTU_NOTIFICATION; 58 event.motu_notification.message = motu->msg; 59 motu->msg = 0; 60 spin_unlock_irq(&motu->lock); 61 62 count = min_t(long, count, sizeof(event)); 63 if (copy_to_user(buf, &event, count)) 64 return -EFAULT; 65 } else if (has_dsp_event(motu)) { 66 size_t consumed = 0; 67 u32 __user *ptr; 68 u32 ev; 69 70 spin_unlock_irq(&motu->lock); 71 72 // Header is filled later. 73 consumed += sizeof(event.motu_register_dsp_change); 74 75 while (consumed < count && 76 snd_motu_register_dsp_message_parser_copy_event(motu, &ev)) { 77 ptr = (u32 __user *)(buf + consumed); 78 if (put_user(ev, ptr)) 79 return -EFAULT; 80 consumed += sizeof(ev); 81 } 82 83 event.motu_register_dsp_change.type = SNDRV_FIREWIRE_EVENT_MOTU_REGISTER_DSP_CHANGE; 84 event.motu_register_dsp_change.count = 85 (consumed - sizeof(event.motu_register_dsp_change)) / 4; 86 if (copy_to_user(buf, &event, sizeof(event.motu_register_dsp_change))) 87 return -EFAULT; 88 89 count = consumed; 90 }

Smatch complains that there is no "} else {" path which unlocks.

91 

--> 92 return count; 93 }

regards, dan carpenter

Takashi Sakamoto