[PATCH] ALSA: seq: Restore the delivery error code in the bounce event

[PATCH] ALSA: seq: Restore the delivery error code in the bounce event

[HyeongJun An]

Commit efc86691e4d8 ("ALSA: seq: Fix kernel heap address leak in
bounce_error_event()") moved the data.quote.value assignment into the
kernel client branch.  A user client that sets SNDRV_SEQ_FILTER_BOUNCE
used to get the delivery error code there.  Now it gets none, and
nothing else reports it, because a queued event's write() has already
returned success by the time delivery fails.

Send struct snd_seq_event_bounce instead, the error code followed by the
original event record, which is what the UAPI header has described all
along.  Store the negative errno.  The kernel client branch negates it
only because data.quote.value is an unsigned short.

The payload grows from 28 to 32 bytes.  The snd_seq_read() rounds a
variable-length payload up to a multiple of the event size, so a legacy
client now needs an 84 byte buffer instead of 56.  That is what any
event carrying 29 payload bytes has always needed.  A UMP client reads
64 bytes either way.

Drop the stale promise to copy the external data after the event record
as well.  That was never implemented, and bounce_error_event() runs with
atomic set from the timer interrupt.

Fixes: efc86691e4d8 ("ALSA: seq: Fix kernel heap address leak in bounce_error_event()")
Assisted-by: Claude:claude-opus-5
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
---
Notes for the reviewer, not part of the change.

A UMP event does not fit the payload.  A struct snd_seq_ump_event is 32
bytes while quoted.event is 28, so ump[3] is dropped while
SNDRV_SEQ_EVENT_UMP stays set in the copy.  That is already true today,
but this locks it into the UAPI.  Clear the flag, skip bouncing UMP
events, or a different layout?  I did not want to guess.

The err field carries the negative errno.  Nothing has ever produced or
consumed this struct, so the sign is being chosen here for the first
time.  Say if you want the positive value instead.

I could not check whether any user space sets SNDRV_SEQ_FILTER_BOUNCE,
or what buffer size it reads with.

 sound/core/seq/seq_clientmgr.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 11fa7e825819..c334f88de542 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -530,9 +530,10 @@ static struct snd_seq_client *get_event_dest_client(struct snd_seq_event *event)
  * Return the error event.
  *
  * If the receiver client is a user client, the original event is
- * encapsulated in SNDRV_SEQ_EVENT_BOUNCE as variable length event.  If
- * the original event is also variable length, the external data is
- * copied after the event record. 
+ * encapsulated in SNDRV_SEQ_EVENT_BOUNCE as variable length event.  The
+ * payload is struct snd_seq_event_bounce, the error code followed by the
+ * original event record.  The external data of a variable length event
+ * is not copied along.
  * If the receiver client is a kernel client, the original event is
  * quoted in SNDRV_SEQ_EVENT_KERNEL_ERROR, since this requires no extra
  * kmalloc.
@@ -541,7 +542,8 @@ 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, quoted;
+	struct snd_seq_event_bounce quoted;
+	struct snd_seq_event bounce_ev;
 	int result;
 
 	if (client == NULL ||
@@ -565,14 +567,16 @@ static int bounce_error_event(struct snd_seq_client *client,
 		 * 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(&quoted)) {
-			quoted.data.ext.len &= ~SNDRV_SEQ_EXT_MASK;
-			quoted.data.ext.ptr = NULL;
+		memset(&quoted, 0, sizeof(quoted));
+		quoted.err = err;
+		quoted.event = *event;
+		if (snd_seq_ev_is_variable(&quoted.event)) {
+			quoted.event.data.ext.len &= ~SNDRV_SEQ_EXT_MASK;
+			quoted.event.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.len = sizeof(quoted);
 		bounce_ev.data.ext.ptr = (char *)&quoted;
 	} else {
 		/*
-- 
2.43.0

Re: [PATCH] ALSA: seq: Restore the delivery error code in the bounce event

[Takashi Iwai]

On Wed, 12 Aug 2026 13:32:36 +0200,
HyeongJun An wrote:
> 
> Commit efc86691e4d8 ("ALSA: seq: Fix kernel heap address leak in
> bounce_error_event()") moved the data.quote.value assignment into the
> kernel client branch.  A user client that sets SNDRV_SEQ_FILTER_BOUNCE
> used to get the delivery error code there.  Now it gets none, and
> nothing else reports it, because a queued event's write() has already
> returned success by the time delivery fails.
> 
> Send struct snd_seq_event_bounce instead, the error code followed by the
> original event record, which is what the UAPI header has described all
> along.  Store the negative errno.  The kernel client branch negates it
> only because data.quote.value is an unsigned short.
> 
> The payload grows from 28 to 32 bytes.  The snd_seq_read() rounds a
> variable-length payload up to a multiple of the event size, so a legacy
> client now needs an 84 byte buffer instead of 56.  That is what any
> event carrying 29 payload bytes has always needed.  A UMP client reads
> 64 bytes either way.
> 
> Drop the stale promise to copy the external data after the event record
> as well.  That was never implemented, and bounce_error_event() runs with
> atomic set from the timer interrupt.
> 
> Fixes: efc86691e4d8 ("ALSA: seq: Fix kernel heap address leak in bounce_error_event()")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
> ---
> Notes for the reviewer, not part of the change.
> 
> A UMP event does not fit the payload.  A struct snd_seq_ump_event is 32
> bytes while quoted.event is 28, so ump[3] is dropped while
> SNDRV_SEQ_EVENT_UMP stays set in the copy.  That is already true today,
> but this locks it into the UAPI.  Clear the flag, skip bouncing UMP
> events, or a different layout?  I did not want to guess.
> 
> The err field carries the negative errno.  Nothing has ever produced or
> consumed this struct, so the sign is being chosen here for the first
> time.  Say if you want the positive value instead.
> 
> I could not check whether any user space sets SNDRV_SEQ_FILTER_BOUNCE,
> or what buffer size it reads with.

Thanks, now I see what you meant previously.

I belive that struct snd_seq_event_bounce is dead, not actually used.
So, it's better to stick with the existing snd_seq_ev_quote even
though we lose the error code in user-space delivery.  The definition
of snd_seq_event_bounce can be dropped for avoiding confusion in
future, too.


Takashi

[PATCH] ALSA: seq: Drop the dead struct snd_seq_event_bounce

[HyeongJun An]

The struct describes a bounce payload of an error code followed by the
original event and its external data.  No kernel has ever sent that.
Before commit efc86691e4d8 ("ALSA: seq: Fix kernel heap address leak in
bounce_error_event()") the kernel emitted no SNDRV_SEQ_EVENT_BOUNCE at
all, and since then it sends the event record alone.

Nothing has ever read it either.  Its only accessor,
snd_seq_event_bounce_ext_data(), has had no caller for the whole git
history, and it did not even compile until commit c7e0b5bf9fff ("[ALSA]
Remove xxx_t typedefs: Sequencer") incidentally repaired the type name
it referred to, three years after the git import.  Drop the accessor
along with the struct.

This removes a definition from a UAPI header.  Since no kernel ever
produced the layout, nothing can have parsed it, but a program that
merely names the type will need to stop.

Suggested-by: Takashi Iwai <tiwai@suse.de>
Assisted-by: Claude:claude-opus-5
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
---
 include/sound/asequencer.h      |  3 ---
 include/uapi/sound/asequencer.h | 10 ----------
 sound/core/seq/seq_clientmgr.c  |  5 ++---
 3 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/include/sound/asequencer.h b/include/sound/asequencer.h
index ddbb6bf801bb..efad366736a4 100644
--- a/include/sound/asequencer.h
+++ b/include/sound/asequencer.h
@@ -11,9 +11,6 @@
 #include <sound/asound.h>
 #include <uapi/sound/asequencer.h>
 
-/* helper macro */
-#define snd_seq_event_bounce_ext_data(ev) ((void*)((char *)(ev)->data.ext.ptr + sizeof(struct snd_seq_event_bounce)))
-
 /*
  * type check macros
  */
diff --git a/include/uapi/sound/asequencer.h b/include/uapi/sound/asequencer.h
index a5c41f771e05..3deba3965ca5 100644
--- a/include/uapi/sound/asequencer.h
+++ b/include/uapi/sound/asequencer.h
@@ -308,16 +308,6 @@ struct snd_seq_ump_event {
 	};
 };
 
-/*
- * bounce event - stored as variable size data
- */
-struct snd_seq_event_bounce {
-	int err;
-	struct snd_seq_event event;
-	/* external data follows here. */
-};
-
-
 	/* system information */
 struct snd_seq_system_info {
 	int queues;			/* maximum queues count */
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 11fa7e825819..5b86e75c2658 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -530,9 +530,8 @@ static struct snd_seq_client *get_event_dest_client(struct snd_seq_event *event)
  * Return the error event.
  *
  * If the receiver client is a user client, the original event is
- * encapsulated in SNDRV_SEQ_EVENT_BOUNCE as variable length event.  If
- * the original event is also variable length, the external data is
- * copied after the event record. 
+ * encapsulated in SNDRV_SEQ_EVENT_BOUNCE as variable length event.  The
+ * external data of a variable length event is not copied along.
  * If the receiver client is a kernel client, the original event is
  * quoted in SNDRV_SEQ_EVENT_KERNEL_ERROR, since this requires no extra
  * kmalloc.
-- 
2.43.0

Re: [PATCH] ALSA: seq: Drop the dead struct snd_seq_event_bounce

[Takashi Iwai]

On Wed, 12 Aug 2026 16:15:06 +0200,
HyeongJun An wrote:
> 
> The struct describes a bounce payload of an error code followed by the
> original event and its external data.  No kernel has ever sent that.
> Before commit efc86691e4d8 ("ALSA: seq: Fix kernel heap address leak in
> bounce_error_event()") the kernel emitted no SNDRV_SEQ_EVENT_BOUNCE at
> all, and since then it sends the event record alone.
> 
> Nothing has ever read it either.  Its only accessor,
> snd_seq_event_bounce_ext_data(), has had no caller for the whole git
> history, and it did not even compile until commit c7e0b5bf9fff ("[ALSA]
> Remove xxx_t typedefs: Sequencer") incidentally repaired the type name
> it referred to, three years after the git import.  Drop the accessor
> along with the struct.
> 
> This removes a definition from a UAPI header.  Since no kernel ever
> produced the layout, nothing can have parsed it, but a program that
> merely names the type will need to stop.
> 
> Suggested-by: Takashi Iwai <tiwai@suse.de>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: HyeongJun An <sammiee5311@gmail.com>

Applied to for-next branch now.  Thanks.


Takashi