ALSA: rawmidi: Access runtime->avail always in spinlock

The runtime->avail field may be accessed concurrently while some
places refer to it without taking the runtime->lock spinlock, as
detected by KCSAN.  Usually this isn't a big problem, but for
consistency and safety, we should take the spinlock at each place
referencing this field.

Reported-by: syzbot+a23a6f1215c84756577c@syzkaller.appspotmail.com
Reported-by: syzbot+3d367d1df1d2b67f5c19@syzkaller.appspotmail.com
Link: https://lore.kernel.org/r/20201206083527.21163-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c
index c78720a3299c42b7b8b40e80858954bb4dac4a28..257ad5206240fd0cff47545f505a38a2dca8c81b 100644 (file)
--- a/sound/core/rawmidi.c
+++ b/sound/core/rawmidi.c
@@ -95,11 +95,21 @@ static inline unsigned short snd_rawmidi_file_flags(struct file *file)
        }
 }
 
-static inline int snd_rawmidi_ready(struct snd_rawmidi_substream *substream)
+static inline bool __snd_rawmidi_ready(struct snd_rawmidi_runtime *runtime)
+{
+       return runtime->avail >= runtime->avail_min;
+}
+
+static bool snd_rawmidi_ready(struct snd_rawmidi_substream *substream)
 {
        struct snd_rawmidi_runtime *runtime = substream->runtime;
+       unsigned long flags;
+       bool ready;
 
-       return runtime->avail >= runtime->avail_min;
+       spin_lock_irqsave(&runtime->lock, flags);
+       ready = __snd_rawmidi_ready(runtime);
+       spin_unlock_irqrestore(&runtime->lock, flags);
+       return ready;
 }
 
 static inline int snd_rawmidi_ready_append(struct snd_rawmidi_substream *substream,
@@ -1019,7 +1029,7 @@ int snd_rawmidi_receive(struct snd_rawmidi_substream *substream,
        if (result > 0) {
                if (runtime->event)
                        schedule_work(&runtime->event_work);
-               else if (snd_rawmidi_ready(substream))
+               else if (__snd_rawmidi_ready(runtime))
                        wake_up(&runtime->sleep);
        }
        spin_unlock_irqrestore(&runtime->lock, flags);
@@ -1098,7 +1108,7 @@ static ssize_t snd_rawmidi_read(struct file *file, char __user *buf, size_t coun
        result = 0;
        while (count > 0) {
                spin_lock_irq(&runtime->lock);
-               while (!snd_rawmidi_ready(substream)) {
+               while (!__snd_rawmidi_ready(runtime)) {
                        wait_queue_entry_t wait;
 
                        if ((file->f_flags & O_NONBLOCK) != 0 || result > 0) {
@@ -1115,9 +1125,11 @@ static ssize_t snd_rawmidi_read(struct file *file, char __user *buf, size_t coun
                                return -ENODEV;
                        if (signal_pending(current))
                                return result > 0 ? result : -ERESTARTSYS;
-                       if (!runtime->avail)
-                               return result > 0 ? result : -EIO;
                        spin_lock_irq(&runtime->lock);
+                       if (!runtime->avail) {
+                               spin_unlock_irq(&runtime->lock);
+                               return result > 0 ? result : -EIO;
+                       }
                }
                spin_unlock_irq(&runtime->lock);
                count1 = snd_rawmidi_kernel_read1(substream,
@@ -1255,7 +1267,7 @@ int __snd_rawmidi_transmit_ack(struct snd_rawmidi_substream *substream, int coun
        runtime->avail += count;
        substream->bytes += count;
        if (count > 0) {
-               if (runtime->drain || snd_rawmidi_ready(substream))
+               if (runtime->drain || __snd_rawmidi_ready(runtime))
                        wake_up(&runtime->sleep);
        }
        return count;
@@ -1444,9 +1456,11 @@ static ssize_t snd_rawmidi_write(struct file *file, const char __user *buf,
                                return -ENODEV;
                        if (signal_pending(current))
                                return result > 0 ? result : -ERESTARTSYS;
-                       if (!runtime->avail && !timeout)
-                               return result > 0 ? result : -EIO;
                        spin_lock_irq(&runtime->lock);
+                       if (!runtime->avail && !timeout) {
+                               spin_unlock_irq(&runtime->lock);
+                               return result > 0 ? result : -EIO;
+                       }
                }
                spin_unlock_irq(&runtime->lock);
                count1 = snd_rawmidi_kernel_write1(substream, buf, NULL, count);
@@ -1526,6 +1540,7 @@ static void snd_rawmidi_proc_info_read(struct snd_info_entry *entry,
        struct snd_rawmidi *rmidi;
        struct snd_rawmidi_substream *substream;
        struct snd_rawmidi_runtime *runtime;
+       unsigned long buffer_size, avail, xruns;
 
        rmidi = entry->private_data;
        snd_iprintf(buffer, "%s\n\n", rmidi->name);
@@ -1544,13 +1559,16 @@ static void snd_rawmidi_proc_info_read(struct snd_info_entry *entry,
                                    "  Owner PID    : %d\n",
                                    pid_vnr(substream->pid));
                                runtime = substream->runtime;
+                               spin_lock_irq(&runtime->lock);
+                               buffer_size = runtime->buffer_size;
+                               avail = runtime->avail;
+                               spin_unlock_irq(&runtime->lock);
                                snd_iprintf(buffer,
                                    "  Mode         : %s\n"
                                    "  Buffer size  : %lu\n"
                                    "  Avail        : %lu\n",
                                    runtime->oss ? "OSS compatible" : "native",
-                                   (unsigned long) runtime->buffer_size,
-                                   (unsigned long) runtime->avail);
+                                   buffer_size, avail);
                        }
                }
        }
@@ -1568,13 +1586,16 @@ static void snd_rawmidi_proc_info_read(struct snd_info_entry *entry,
                                            "  Owner PID    : %d\n",
                                            pid_vnr(substream->pid));
                                runtime = substream->runtime;
+                               spin_lock_irq(&runtime->lock);
+                               buffer_size = runtime->buffer_size;
+                               avail = runtime->avail;
+                               xruns = runtime->xruns;
+                               spin_unlock_irq(&runtime->lock);
                                snd_iprintf(buffer,
                                            "  Buffer size  : %lu\n"
                                            "  Avail        : %lu\n"
                                            "  Overruns     : %lu\n",
-                                           (unsigned long) runtime->buffer_size,
-                                           (unsigned long) runtime->avail,
-                                           (unsigned long) runtime->xruns);
+                                           buffer_size, avail, xruns);
                        }
                }
        }