Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

ALSA: seq: Fix copy_from_user() call inside lock

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>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

+20 -8
+1
include/sound/seq_virmidi.h
··· 60 60 int port; /* created/attached port */ 61 61 unsigned int flags; /* SNDRV_VIRMIDI_* */ 62 62 rwlock_t filelist_lock; 63 + struct rw_semaphore filelist_sem; 63 64 struct list_head filelist; 64 65 }; 65 66
+19 -8
sound/core/seq/seq_virmidi.c
··· 77 77 * decode input event and put to read buffer of each opened file 78 78 */ 79 79 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) 81 82 { 82 83 struct snd_virmidi *vmidi; 83 84 unsigned char msg[4]; 84 85 int len; 85 86 86 - read_lock(&rdev->filelist_lock); 87 + if (atomic) 88 + read_lock(&rdev->filelist_lock); 89 + else 90 + down_read(&rdev->filelist_sem); 87 91 list_for_each_entry(vmidi, &rdev->filelist, list) { 88 92 if (!vmidi->trigger) 89 93 continue; ··· 101 97 snd_rawmidi_receive(vmidi->substream, msg, len); 102 98 } 103 99 } 104 - read_unlock(&rdev->filelist_lock); 100 + if (atomic) 101 + read_unlock(&rdev->filelist_lock); 102 + else 103 + up_read(&rdev->filelist_sem); 105 104 106 105 return 0; 107 106 } ··· 122 115 struct snd_virmidi_dev *rdev; 123 116 124 117 rdev = rmidi->private_data; 125 - return snd_virmidi_dev_receive_event(rdev, ev); 118 + return snd_virmidi_dev_receive_event(rdev, ev, true); 126 119 } 127 120 #endif /* 0 */ 128 121 ··· 137 130 rdev = private_data; 138 131 if (!(rdev->flags & SNDRV_VIRMIDI_USE)) 139 132 return 0; /* ignored */ 140 - return snd_virmidi_dev_receive_event(rdev, ev); 133 + return snd_virmidi_dev_receive_event(rdev, ev, atomic); 141 134 } 142 135 143 136 /* ··· 216 209 struct snd_virmidi_dev *rdev = substream->rmidi->private_data; 217 210 struct snd_rawmidi_runtime *runtime = substream->runtime; 218 211 struct snd_virmidi *vmidi; 219 - unsigned long flags; 220 212 221 213 vmidi = kzalloc(sizeof(*vmidi), GFP_KERNEL); 222 214 if (vmidi == NULL) ··· 229 223 vmidi->client = rdev->client; 230 224 vmidi->port = rdev->port; 231 225 runtime->private_data = vmidi; 232 - write_lock_irqsave(&rdev->filelist_lock, flags); 226 + down_write(&rdev->filelist_sem); 227 + write_lock_irq(&rdev->filelist_lock); 233 228 list_add_tail(&vmidi->list, &rdev->filelist); 234 - write_unlock_irqrestore(&rdev->filelist_lock, flags); 229 + write_unlock_irq(&rdev->filelist_lock); 230 + up_write(&rdev->filelist_sem); 235 231 vmidi->rdev = rdev; 236 232 return 0; 237 233 } ··· 272 264 struct snd_virmidi_dev *rdev = substream->rmidi->private_data; 273 265 struct snd_virmidi *vmidi = substream->runtime->private_data; 274 266 267 + down_write(&rdev->filelist_sem); 275 268 write_lock_irq(&rdev->filelist_lock); 276 269 list_del(&vmidi->list); 277 270 write_unlock_irq(&rdev->filelist_lock); 271 + up_write(&rdev->filelist_sem); 278 272 snd_midi_event_free(vmidi->parser); 279 273 substream->runtime->private_data = NULL; 280 274 kfree(vmidi); ··· 530 520 rdev->rmidi = rmidi; 531 521 rdev->device = device; 532 522 rdev->client = -1; 523 + init_rwsem(&rdev->filelist_sem); 533 524 rwlock_init(&rdev->filelist_lock); 534 525 INIT_LIST_HEAD(&rdev->filelist); 535 526 rdev->seq_mode = SNDRV_VIRMIDI_SEQ_DISPATCH;