[PATCH AUTOSEL 5.1 03/60] ALSA: seq: Fix race of get-subscription call vs port-delete ioctls

From: Sasha Levin
Date: Tue Jun 04 2019 - 19:26:21 EST


From: Takashi Iwai <tiwai@xxxxxxx>

[ Upstream commit 2eabc5ec8ab4d4748a82050dfcb994119b983750 ]

The snd_seq_ioctl_get_subscription() retrieves the port subscriber
information as a pointer, while the object isn't protected, hence it
may be deleted before the actual reference. This race was spotted by
syzkaller and may lead to a UAF.

The fix is simply copying the data in the lookup function that
performs in the rwsem to protect against the deletion.

Reported-by: syzbot+9437020c82413d00222d@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
sound/core/seq/seq_clientmgr.c | 10 ++--------
sound/core/seq/seq_ports.c | 13 ++++++++-----
sound/core/seq/seq_ports.h | 5 +++--
3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index b3280e81bfd1..8599f2937ac1 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1900,20 +1900,14 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
int result;
struct snd_seq_client *sender = NULL;
struct snd_seq_client_port *sport = NULL;
- struct snd_seq_subscribers *p;

result = -EINVAL;
if ((sender = snd_seq_client_use_ptr(subs->sender.client)) == NULL)
goto __end;
if ((sport = snd_seq_port_use_ptr(sender, subs->sender.port)) == NULL)
goto __end;
- p = snd_seq_port_get_subscription(&sport->c_src, &subs->dest);
- if (p) {
- result = 0;
- *subs = p->info;
- } else
- result = -ENOENT;
-
+ result = snd_seq_port_get_subscription(&sport->c_src, &subs->dest,
+ subs);
__end:
if (sport)
snd_seq_port_unlock(sport);
diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c
index 24d90abfc64d..a31e16cc012e 100644
--- a/sound/core/seq/seq_ports.c
+++ b/sound/core/seq/seq_ports.c
@@ -635,20 +635,23 @@ int snd_seq_port_disconnect(struct snd_seq_client *connector,


/* get matched subscriber */
-struct snd_seq_subscribers *snd_seq_port_get_subscription(struct snd_seq_port_subs_info *src_grp,
- struct snd_seq_addr *dest_addr)
+int snd_seq_port_get_subscription(struct snd_seq_port_subs_info *src_grp,
+ struct snd_seq_addr *dest_addr,
+ struct snd_seq_port_subscribe *subs)
{
- struct snd_seq_subscribers *s, *found = NULL;
+ struct snd_seq_subscribers *s;
+ int err = -ENOENT;

down_read(&src_grp->list_mutex);
list_for_each_entry(s, &src_grp->list_head, src_list) {
if (addr_match(dest_addr, &s->info.dest)) {
- found = s;
+ *subs = s->info;
+ err = 0;
break;
}
}
up_read(&src_grp->list_mutex);
- return found;
+ return err;
}

/*
diff --git a/sound/core/seq/seq_ports.h b/sound/core/seq/seq_ports.h
index 26bd71f36c41..06003b36652e 100644
--- a/sound/core/seq/seq_ports.h
+++ b/sound/core/seq/seq_ports.h
@@ -135,7 +135,8 @@ int snd_seq_port_subscribe(struct snd_seq_client_port *port,
struct snd_seq_port_subscribe *info);

/* get matched subscriber */
-struct snd_seq_subscribers *snd_seq_port_get_subscription(struct snd_seq_port_subs_info *src_grp,
- struct snd_seq_addr *dest_addr);
+int snd_seq_port_get_subscription(struct snd_seq_port_subs_info *src_grp,
+ struct snd_seq_addr *dest_addr,
+ struct snd_seq_port_subscribe *subs);

#endif
--
2.20.1