ALSA: seq: Fix race of get-subscription call vs port-delete ioctls
authorTakashi Iwai <tiwai@suse.de>
Tue, 9 Apr 2019 16:04:17 +0000 (18:04 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sat, 22 Jun 2019 06:17:19 +0000 (08:17 +0200)
[ 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@syzkaller.appspotmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
sound/core/seq/seq_clientmgr.c
sound/core/seq/seq_ports.c
sound/core/seq/seq_ports.h

index 3b1b2e9fb33e5d0f4f3f37070ca2479666db875d..881c965555c5f13e2f8470d47ec553f4834ff81a 100644 (file)
@@ -1905,20 +1905,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);
index a42e2ce4a726a62787f42d46a3e6c61811b96fab..9cfe4fcee9a589fa8ab82070c0f87e7823b01a61 100644 (file)
@@ -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;
 }
 
 /*
index 26bd71f36c41d1961d3be970d0264e6aef06483d..06003b36652ef355f9876987d64cc76ccd2c7cda 100644 (file)
@@ -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