Skip to contentSkip to conversation

Patch "ALSA: seq: Fix copy_from_user() call inside lock" has been added to the 4.13-stable tree

PATCH [email protected] 1 participant 1 message Oct 15, 2017

Subsystems (2)
Stable branch Sound
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

ALSA: seq: Fix copy_from_user() call inside lock

to the 4.13-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
alsa-seq-fix-copy_from_user-call-inside-lock.patch
and it can be found in the queue-4.13 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 5803b023881857db32ffefa0d269c90280a67ee0 Mon Sep 17 00:00:00 2001

From: Takashi Iwai <[email protected]>
Date: Mon, 9 Oct 2017 10:02:56 +0200
Subject: ALSA: seq: Fix copy_from_user() call inside lock

From: Takashi Iwai <[email protected]>

commit 5803b023881857db32ffefa0d269c90280a67ee0 upstream.

The event handler in the virmidi sequencer code takes a read-lock for the linked list traverse, while it's calling snd_seq_dump_var_event() in the loop. The latter function may expand the user-space data depending on the event type. It eventually invokes copy_from_user(), which might be a potential dead-lock.

The sequencer core guarantees that the user-space data is passed only with atomic=0 argument, but snd_virmidi_dev_receive_event() ignores it and always takes read-lock(). For avoiding the problem above, this patch introduces rwsem for non-atomic case, while keeping rwlock for atomic case.

Also while we're at it: the superfluous irq flags is dropped in snd_virmidi_input_open().

Reported-by Jia-Ju Bai <[email protected]>
Signed-off-by Takashi Iwai <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 include/sound/seq_virmidi.h  |    1 +
 sound/core/seq/seq_virmidi.c |   27 +++++++++++++++++++--------
 2 files changed, 20 insertions(+), 8 deletions(-)
include/sound/seq_virmidi.h +1 -0
@@ -60,6 +60,7 @@ struct snd_virmidi_dev {
 	int port;			/* created/attached port */
 	unsigned int flags;		/* SNDRV_VIRMIDI_* */
 	rwlock_t filelist_lock;
+	struct rw_semaphore filelist_sem;
 	struct list_head filelist;
 };
sound/core/seq/seq_virmidi.c +19 -8
@@ -77,13 +77,17 @@ static void snd_virmidi_init_event(struc
  * decode input event and put to read buffer of each opened file
  */
 static int snd_virmidi_dev_receive_event(struct snd_virmidi_dev *rdev,
-					 struct snd_seq_event *ev)
+					 struct snd_seq_event *ev,
+					 bool atomic)
 {
 	struct snd_virmidi *vmidi;
 	unsigned char msg[4];
 	int len;
 
-	read_lock(&rdev->filelist_lock);
+	if (atomic)
+		read_lock(&rdev->filelist_lock);
+	else
+		down_read(&rdev->filelist_sem);
 	list_for_each_entry(vmidi, &rdev->filelist, list) {
 		if (!vmidi->trigger)
 			continue;

@@ -97,7 +101,10 @@ static int snd_virmidi_dev_receive_event
 				snd_rawmidi_receive(vmidi->substream, msg, len);
 		}
 	}
-	read_unlock(&rdev->filelist_lock);
+	if (atomic)
+		read_unlock(&rdev->filelist_lock);
+	else
+		up_read(&rdev->filelist_sem);
 
 	return 0;
 }

@@ -115,7 +122,7 @@ int snd_virmidi_receive(struct snd_rawmi
 	struct snd_virmidi_dev *rdev;
 
 	rdev = rmidi->private_data;
-	return snd_virmidi_dev_receive_event(rdev, ev);
+	return snd_virmidi_dev_receive_event(rdev, ev, true);
 }
 #endif  /*  0  */
 

@@ -130,7 +137,7 @@ static int snd_virmidi_event_input(struc
 	rdev = private_data;
 	if (!(rdev->flags & SNDRV_VIRMIDI_USE))
 		return 0; /* ignored */
-	return snd_virmidi_dev_receive_event(rdev, ev);
+	return snd_virmidi_dev_receive_event(rdev, ev, atomic);
 }
 
 /*

@@ -209,7 +216,6 @@ static int snd_virmidi_input_open(struct
 	struct snd_virmidi_dev *rdev = substream->rmidi->private_data;
 	struct snd_rawmidi_runtime *runtime = substream->runtime;
 	struct snd_virmidi *vmidi;
-	unsigned long flags;
 
 	vmidi = kzalloc(sizeof(*vmidi), GFP_KERNEL);
 	if (vmidi == NULL)

@@ -223,9 +229,11 @@ static int snd_virmidi_input_open(struct
 	vmidi->client = rdev->client;
 	vmidi->port = rdev->port;	
 	runtime->private_data = vmidi;
-	write_lock_irqsave(&rdev->filelist_lock, flags);
+	down_write(&rdev->filelist_sem);
+	write_lock_irq(&rdev->filelist_lock);
 	list_add_tail(&vmidi->list, &rdev->filelist);
-	write_unlock_irqrestore(&rdev->filelist_lock, flags);
+	write_unlock_irq(&rdev->filelist_lock);
+	up_write(&rdev->filelist_sem);
 	vmidi->rdev = rdev;
 	return 0;
 }

@@ -264,9 +272,11 @@ static int snd_virmidi_input_close(struc
 	struct snd_virmidi_dev *rdev = substream->rmidi->private_data;
 	struct snd_virmidi *vmidi = substream->runtime->private_data;
 
+	down_write(&rdev->filelist_sem);
 	write_lock_irq(&rdev->filelist_lock);
 	list_del(&vmidi->list);
 	write_unlock_irq(&rdev->filelist_lock);
+	up_write(&rdev->filelist_sem);
 	snd_midi_event_free(vmidi->parser);
 	substream->runtime->private_data = NULL;
 	kfree(vmidi);

@@ -520,6 +530,7 @@ int snd_virmidi_new(struct snd_card *car
 	rdev->rmidi = rmidi;
 	rdev->device = device;
 	rdev->client = -1;
+	init_rwsem(&rdev->filelist_sem);
 	rwlock_init(&rdev->filelist_lock);
 	INIT_LIST_HEAD(&rdev->filelist);
 	rdev->seq_mode = SNDRV_VIRMIDI_SEQ_DISPATCH;

Patches currently in stable-queue which might be from [email protected] are

queue-4.13/alsa-line6-fix-null-dereference-at-podhd_disconnect.patch queue-4.13/alsa-line6-fix-missing-initialization-before-error-path.patch queue-4.13/alsa-caiaq-fix-stray-urb-at-probe-error-path.patch queue-4.13/alsa-seq-fix-copy_from_user-call-inside-lock.patch queue-4.13/alsa-seq-fix-use-after-free-at-creating-a-port.patch queue-4.13/alsa-line6-fix-leftover-urb-at-error-path-during-probe.patch queue-4.13/alsa-usb-audio-kill-stray-urb-at-exiting.patch