Skip to content

Commit 12a4d42

Browse files
ALSA: rawmidi: Fix racy buffer resize under concurrent accesses
jira VULN-7721 cve CVE-2020-27786 commit-author Takashi Iwai <tiwai@suse.de> commit c1f6e3c The rawmidi core allows user to resize the runtime buffer via ioctl, and this may lead to UAF when performed during concurrent reads or writes: the read/write functions unlock the runtime lock temporarily during copying form/to user-space, and that's the race window. This patch fixes the hole by introducing a reference counter for the runtime buffer read/write access and returns -EBUSY error when the resize is performed concurrently against read/write. Note that the ref count field is a simple integer instead of refcount_t here, since the all contexts accessing the buffer is basically protected with a spinlock, hence we need no expensive atomic ops. Also, note that this busy check is needed only against read / write functions, and not in receive/transmit callbacks; the race can happen only at the spinlock hole mentioned in the above, while the whole function is protected for receive / transmit callbacks. Reported-by: butt3rflyh4ck <butterflyhuangxx@gmail.com> Cc: <stable@vger.kernel.org> Link: https://lore.kernel.org/r/CAFcO6XMWpUVK_yzzCpp8_XP7+=oUpQvuBeCbMffEDkpe8jWrfg@mail.gmail.com Link: https://lore.kernel.org/r/s5heerw3r5z.wl-tiwai@suse.de Signed-off-by: Takashi Iwai <tiwai@suse.de> (cherry picked from commit c1f6e3c) Signed-off-by: Pratham Patel <ppatel@ciq.com>
1 parent 63c5b11 commit 12a4d42

File tree

2 files changed

+28
-4
lines changed

2 files changed

+28
-4
lines changed

include/sound/rawmidi.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ struct snd_rawmidi_runtime {
7676
size_t avail_min; /* min avail for wakeup */
7777
size_t avail; /* max used buffer for wakeup */
7878
size_t xruns; /* over/underruns counter */
79+
int buffer_ref; /* buffer reference count */
7980
/* misc */
8081
spinlock_t lock;
8182
wait_queue_head_t sleep;

sound/core/rawmidi.c

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,17 @@ static void snd_rawmidi_input_event_work(struct work_struct *work)
112112
runtime->event(runtime->substream);
113113
}
114114

115+
/* buffer refcount management: call with runtime->lock held */
116+
static inline void snd_rawmidi_buffer_ref(struct snd_rawmidi_runtime *runtime)
117+
{
118+
runtime->buffer_ref++;
119+
}
120+
121+
static inline void snd_rawmidi_buffer_unref(struct snd_rawmidi_runtime *runtime)
122+
{
123+
runtime->buffer_ref--;
124+
}
125+
115126
static int snd_rawmidi_runtime_create(struct snd_rawmidi_substream *substream)
116127
{
117128
struct snd_rawmidi_runtime *runtime;
@@ -661,6 +672,11 @@ static int resize_runtime_buffer(struct snd_rawmidi_runtime *runtime,
661672
if (!newbuf)
662673
return -ENOMEM;
663674
spin_lock_irq(&runtime->lock);
675+
if (runtime->buffer_ref) {
676+
spin_unlock_irq(&runtime->lock);
677+
kvfree(newbuf);
678+
return -EBUSY;
679+
}
664680
oldbuf = runtime->buffer;
665681
runtime->buffer = newbuf;
666682
runtime->buffer_size = params->buffer_size;
@@ -960,8 +976,10 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream,
960976
long result = 0, count1;
961977
struct snd_rawmidi_runtime *runtime = substream->runtime;
962978
unsigned long appl_ptr;
979+
int err = 0;
963980

964981
spin_lock_irqsave(&runtime->lock, flags);
982+
snd_rawmidi_buffer_ref(runtime);
965983
while (count > 0 && runtime->avail) {
966984
count1 = runtime->buffer_size - runtime->appl_ptr;
967985
if (count1 > count)
@@ -980,16 +998,19 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream,
980998
if (userbuf) {
981999
spin_unlock_irqrestore(&runtime->lock, flags);
9821000
if (copy_to_user(userbuf + result,
983-
runtime->buffer + appl_ptr, count1)) {
984-
return result > 0 ? result : -EFAULT;
985-
}
1001+
runtime->buffer + appl_ptr, count1))
1002+
err = -EFAULT;
9861003
spin_lock_irqsave(&runtime->lock, flags);
1004+
if (err)
1005+
goto out;
9871006
}
9881007
result += count1;
9891008
count -= count1;
9901009
}
1010+
out:
1011+
snd_rawmidi_buffer_unref(runtime);
9911012
spin_unlock_irqrestore(&runtime->lock, flags);
992-
return result;
1013+
return result > 0 ? result : err;
9931014
}
9941015

9951016
long snd_rawmidi_kernel_read(struct snd_rawmidi_substream *substream,
@@ -1283,6 +1304,7 @@ static long snd_rawmidi_kernel_write1(struct snd_rawmidi_substream *substream,
12831304
return -EAGAIN;
12841305
}
12851306
}
1307+
snd_rawmidi_buffer_ref(runtime);
12861308
while (count > 0 && runtime->avail > 0) {
12871309
count1 = runtime->buffer_size - runtime->appl_ptr;
12881310
if (count1 > count)
@@ -1314,6 +1336,7 @@ static long snd_rawmidi_kernel_write1(struct snd_rawmidi_substream *substream,
13141336
}
13151337
__end:
13161338
count1 = runtime->avail < runtime->buffer_size;
1339+
snd_rawmidi_buffer_unref(runtime);
13171340
spin_unlock_irqrestore(&runtime->lock, flags);
13181341
if (count1)
13191342
snd_rawmidi_output_trigger(substream, 1);

0 commit comments

Comments
 (0)