Skip to content

Commit ca2523c

Browse files
tiwaigregkh
authored andcommitted
ALSA: seq: Fix copy_from_user() call inside lock
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 <baijiaju1990@163.com> Signed-off-by: Takashi Iwai <tiwai@suse.de> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 23709ae commit ca2523c

2 files changed

Lines changed: 20 additions & 8 deletions

File tree

include/sound/seq_virmidi.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ struct snd_virmidi_dev {
6060
int port; /* created/attached port */
6161
unsigned int flags; /* SNDRV_VIRMIDI_* */
6262
rwlock_t filelist_lock;
63+
struct rw_semaphore filelist_sem;
6364
struct list_head filelist;
6465
};
6566

sound/core/seq/seq_virmidi.c

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,17 @@ static void snd_virmidi_init_event(struct snd_virmidi *vmidi,
7777
* decode input event and put to read buffer of each opened file
7878
*/
7979
static int snd_virmidi_dev_receive_event(struct snd_virmidi_dev *rdev,
80-
struct snd_seq_event *ev)
80+
struct snd_seq_event *ev,
81+
bool atomic)
8182
{
8283
struct snd_virmidi *vmidi;
8384
unsigned char msg[4];
8485
int len;
8586

86-
read_lock(&rdev->filelist_lock);
87+
if (atomic)
88+
read_lock(&rdev->filelist_lock);
89+
else
90+
down_read(&rdev->filelist_sem);
8791
list_for_each_entry(vmidi, &rdev->filelist, list) {
8892
if (!vmidi->trigger)
8993
continue;
@@ -97,7 +101,10 @@ static int snd_virmidi_dev_receive_event(struct snd_virmidi_dev *rdev,
97101
snd_rawmidi_receive(vmidi->substream, msg, len);
98102
}
99103
}
100-
read_unlock(&rdev->filelist_lock);
104+
if (atomic)
105+
read_unlock(&rdev->filelist_lock);
106+
else
107+
up_read(&rdev->filelist_sem);
101108

102109
return 0;
103110
}
@@ -115,7 +122,7 @@ int snd_virmidi_receive(struct snd_rawmidi *rmidi, struct snd_seq_event *ev)
115122
struct snd_virmidi_dev *rdev;
116123

117124
rdev = rmidi->private_data;
118-
return snd_virmidi_dev_receive_event(rdev, ev);
125+
return snd_virmidi_dev_receive_event(rdev, ev, true);
119126
}
120127
#endif /* 0 */
121128

@@ -130,7 +137,7 @@ static int snd_virmidi_event_input(struct snd_seq_event *ev, int direct,
130137
rdev = private_data;
131138
if (!(rdev->flags & SNDRV_VIRMIDI_USE))
132139
return 0; /* ignored */
133-
return snd_virmidi_dev_receive_event(rdev, ev);
140+
return snd_virmidi_dev_receive_event(rdev, ev, atomic);
134141
}
135142

136143
/*
@@ -209,7 +216,6 @@ static int snd_virmidi_input_open(struct snd_rawmidi_substream *substream)
209216
struct snd_virmidi_dev *rdev = substream->rmidi->private_data;
210217
struct snd_rawmidi_runtime *runtime = substream->runtime;
211218
struct snd_virmidi *vmidi;
212-
unsigned long flags;
213219

214220
vmidi = kzalloc(sizeof(*vmidi), GFP_KERNEL);
215221
if (vmidi == NULL)
@@ -223,9 +229,11 @@ static int snd_virmidi_input_open(struct snd_rawmidi_substream *substream)
223229
vmidi->client = rdev->client;
224230
vmidi->port = rdev->port;
225231
runtime->private_data = vmidi;
226-
write_lock_irqsave(&rdev->filelist_lock, flags);
232+
down_write(&rdev->filelist_sem);
233+
write_lock_irq(&rdev->filelist_lock);
227234
list_add_tail(&vmidi->list, &rdev->filelist);
228-
write_unlock_irqrestore(&rdev->filelist_lock, flags);
235+
write_unlock_irq(&rdev->filelist_lock);
236+
up_write(&rdev->filelist_sem);
229237
vmidi->rdev = rdev;
230238
return 0;
231239
}
@@ -264,9 +272,11 @@ static int snd_virmidi_input_close(struct snd_rawmidi_substream *substream)
264272
struct snd_virmidi_dev *rdev = substream->rmidi->private_data;
265273
struct snd_virmidi *vmidi = substream->runtime->private_data;
266274

275+
down_write(&rdev->filelist_sem);
267276
write_lock_irq(&rdev->filelist_lock);
268277
list_del(&vmidi->list);
269278
write_unlock_irq(&rdev->filelist_lock);
279+
up_write(&rdev->filelist_sem);
270280
snd_midi_event_free(vmidi->parser);
271281
substream->runtime->private_data = NULL;
272282
kfree(vmidi);
@@ -520,6 +530,7 @@ int snd_virmidi_new(struct snd_card *card, int device, struct snd_rawmidi **rrmi
520530
rdev->rmidi = rmidi;
521531
rdev->device = device;
522532
rdev->client = -1;
533+
init_rwsem(&rdev->filelist_sem);
523534
rwlock_init(&rdev->filelist_lock);
524535
INIT_LIST_HEAD(&rdev->filelist);
525536
rdev->seq_mode = SNDRV_VIRMIDI_SEQ_DISPATCH;

0 commit comments

Comments
 (0)