[PATCH] ALSA: seq: Don't leak the extension cell pointer in the bounce payload
HyeongJun An <[email protected]>
| Newsgroups | gmane.linux.sound,gmane.linux.kernel |
|---|---|
| Message-ID | <[email protected]> |
The bounce_error_event() embeds the failed event in the bounce payload
by pointing data.ext.ptr at it. When that event is a queued
variable-length event, its own data.ext.ptr holds the address of its
first extension cell, put there by snd_seq_event_dup(). The payload
goes out verbatim through snd_seq_expand_var_event(), so the address
reaches userspace.
That is the same address commit 705dd6dcbc0e ("ALSA: seq: Clear
variable event pointer on read") removed from the event header. The
read path still clears it there, just above the call that expands the
payload.
Embed a sanitised copy instead, treated exactly as snd_seq_read()
treats the header. A stack copy is enough because delivery is
synchronous and snd_seq_event_dup() copies before returning.
An unprivileged client reaches this by setting SNDRV_SEQ_FILTER_BOUNCE,
queueing a variable-length event to a port that does not exist and
reading the bounce back. Eight bytes on 64-bit, from its own pool.
Fixes: efc86691e4d8 ("ALSA: seq: Fix kernel heap address leak in bounce_error_event()")
Assisted-by: Claude:claude-opus-5
Signed-off-by: HyeongJun An <[email protected]>
---
Unrelated, and not part of the change: the payload does not match struct
snd_seq_event_bounce in include/uapi/sound/asequencer.h - the err field is
not sent. Nothing regressed, since the kernel never produced the event
before efc86691e4d8. I can fix that separately if you want it.
sound/core/seq/seq_clientmgr.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 28782e1776fa..9ec6c1c0cabc 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -528,7 +528,7 @@ static int bounce_error_event(struct snd_seq_client *client,
struct snd_seq_event *event,
int err, int atomic, int hop)
{
- struct snd_seq_event bounce_ev;
+ struct snd_seq_event bounce_ev, quoted;
int result;
if (client == NULL ||
@@ -548,15 +548,19 @@ static int bounce_error_event(struct snd_seq_client *client,
* For user clients, send SNDRV_SEQ_EVENT_BOUNCE with the
* original event embedded as variable-length data. This
* avoids exposing data.quote.event (a kernel pointer) to
- * userspace. The variable-length path in snd_seq_event_dup()
- * copies the event data from data.ext.ptr into chained cells,
- * and snd_seq_expand_var_event() copies only the data content
- * -- never the pointer -- to userspace.
+ * userspace. Sanitise the embedded copy too - a queued
+ * variable-length event carries the address of its own
+ * extension cell, and the payload goes out verbatim.
*/
+ quoted = *event;
+ if (snd_seq_ev_is_variable("ed)) {
+ quoted.data.ext.len &= ~SNDRV_SEQ_EXT_MASK;
+ quoted.data.ext.ptr = NULL;
+ }
bounce_ev.type = SNDRV_SEQ_EVENT_BOUNCE;
bounce_ev.flags = SNDRV_SEQ_EVENT_LENGTH_VARIABLE;
bounce_ev.data.ext.len = sizeof(struct snd_seq_event);
- bounce_ev.data.ext.ptr = (char *)event;
+ bounce_ev.data.ext.ptr = (char *)"ed;
} else {
/*
* For kernel clients, quote the event pointer directly.
--
2.43.0