Re: [bug report] ALSA: firewire-motu: notify event for parameter change in register DSP model

7 Jan 2023


      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