ALSA: seq: Break too long mutex context in the write loop [Linux 4.4.187]

This Linux kernel change "ALSA: seq: Break too long mutex context in the write loop" is included in the Linux 4.4.187 release. This change is authored by Takashi Iwai <tiwai [at] suse.de> on Mon Jul 15 22:50:27 2019 +0200. The commit for this change in Linux stable tree is dc427de (patch) which is from upstream commit ede34f3. The same Linux upstream change may have been applied to various maintained Linux releases and you can find all Linux releases containing changes from upstream ede34f3.

ALSA: seq: Break too long mutex context in the write loop

commit ede34f397ddb063b145b9e7d79c6026f819ded13 upstream.

The fix for the racy writes and ioctls to sequencer widened the
application of client->ioctl_mutex to the whole write loop.  Although
it does unlock/relock for the lengthy operation like the event dup,
the loop keeps the ioctl_mutex for the whole time in other
situations.  This may take quite long time if the user-space would
give a huge buffer, and this is a likely cause of some weird behavior
spotted by syzcaller fuzzer.

This patch puts a simple workaround, just adding a mutex break in the
loop when a large number of events have been processed.  This
shouldn't hit any performance drop because the threshold is set high
enough for usual operations.

Fixes: 7bd800915677 ("ALSA: seq: More protection for concurrent write and ioctl races")
Reported-by: [email protected]
Reported-by: [email protected]
Cc: <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

There are 11 lines of Linux source code added/deleted in this change. Code changes to Linux kernel are as follows.

 sound/core/seq/seq_clientmgr.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 0d0e0c2..7fa0219 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1014,7 +1014,7 @@ static ssize_t snd_seq_write(struct file *file, const char __user *buf,
 {
    struct snd_seq_client *client = file->private_data;
    int written = 0, len;
-   int err;
+   int err, handled;
    struct snd_seq_event event;

    if (!(snd_seq_file_flags(file) & SNDRV_SEQ_LFLG_OUTPUT))
@@ -1027,6 +1027,8 @@ static ssize_t snd_seq_write(struct file *file, const char __user *buf,
    if (!client->accept_output || client->pool == NULL)
        return -ENXIO;

+ repeat:
+   handled = 0;
    /* allocate the pool now if the pool is not allocated yet */ 
    mutex_lock(&client->ioctl_mutex);
    if (client->pool->size > 0 && !snd_seq_write_pool_allocated(client)) {
@@ -1086,12 +1088,19 @@ static ssize_t snd_seq_write(struct file *file, const char __user *buf,
                           0, 0, &client->ioctl_mutex);
        if (err < 0)
            break;
+       handled++;

    __skip_event:
        /* Update pointers and counts */
        count -= len;
        buf += len;
        written += len;
+
+       /* let's have a coffee break if too many events are queued */
+       if (++handled >= 200) {
+           mutex_unlock(&client->ioctl_mutex);
+           goto repeat;
+       }
    }

  out:

Leave a Reply

Your email address will not be published. Required fields are marked *