Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1333832
| From | Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | [PATCH 3.10 37/64] ALSA: seq: Fix lockdep warnings due to double mutex locks |
| Date | 2016-02-15 00:40 +0100 |
| Message-ID | <r2eqL-1c5-39@gated-at.bofh.it> (permalink) |
| References | <r2dO1-Ir-9@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
3.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: Takashi Iwai <tiwai@suse.de>
commit 7f0973e973cd74aa40747c9d38844560cd184ee8 upstream.
The port subscription code uses double mutex locks for source and
destination ports, and this may become racy once when wrongly set up.
It leads to lockdep warning splat, typically triggered by fuzzer like
syzkaller, although the actual deadlock hasn't been seen, so far.
This patch simplifies the handling by reducing to two single locks, so
that no lockdep warning will be trigger any longer.
By splitting to two actions, a still-in-progress element shall be
added in one list while handling another. For ignoring this element,
a new check is added in deliver_to_subscribers().
Along with it, the code to add/remove the subscribers list element was
cleaned up and refactored.
BugLink: http://lkml.kernel.org/r/CACT4Y+aKQXV7xkBW9hpQbzaDO7LrUvohxWh-UwMxXjDy-yBD=A@mail.gmail.com
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Tested-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
sound/core/seq/seq_clientmgr.c | 3
sound/core/seq/seq_ports.c | 235 ++++++++++++++++++++++-------------------
2 files changed, 134 insertions(+), 104 deletions(-)
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -678,6 +678,9 @@ static int deliver_to_subscribers(struct
else
down_read(&grp->list_mutex);
list_for_each_entry(subs, &grp->list_head, src_list) {
+ /* both ports ready? */
+ if (atomic_read(&subs->ref_count) != 2)
+ continue;
event->dest = subs->info.dest;
if (subs->info.flags & SNDRV_SEQ_PORT_SUBS_TIMESTAMP)
/* convert time according to flag with subscription */
--- a/sound/core/seq/seq_ports.c
+++ b/sound/core/seq/seq_ports.c
@@ -175,10 +175,6 @@ struct snd_seq_client_port *snd_seq_crea
}
/* */
-enum group_type {
- SRC_LIST, DEST_LIST
-};
-
static int subscribe_port(struct snd_seq_client *client,
struct snd_seq_client_port *port,
struct snd_seq_port_subs_info *grp,
@@ -205,6 +201,20 @@ static struct snd_seq_client_port *get_c
return NULL;
}
+static void delete_and_unsubscribe_port(struct snd_seq_client *client,
+ struct snd_seq_client_port *port,
+ struct snd_seq_subscribers *subs,
+ bool is_src, bool ack);
+
+static inline struct snd_seq_subscribers *
+get_subscriber(struct list_head *p, bool is_src)
+{
+ if (is_src)
+ return list_entry(p, struct snd_seq_subscribers, src_list);
+ else
+ return list_entry(p, struct snd_seq_subscribers, dest_list);
+}
+
/*
* remove all subscribers on the list
* this is called from port_delete, for each src and dest list.
@@ -212,7 +222,7 @@ static struct snd_seq_client_port *get_c
static void clear_subscriber_list(struct snd_seq_client *client,
struct snd_seq_client_port *port,
struct snd_seq_port_subs_info *grp,
- int grptype)
+ int is_src)
{
struct list_head *p, *n;
@@ -221,15 +231,13 @@ static void clear_subscriber_list(struct
struct snd_seq_client *c;
struct snd_seq_client_port *aport;
- if (grptype == SRC_LIST) {
- subs = list_entry(p, struct snd_seq_subscribers, src_list);
+ subs = get_subscriber(p, is_src);
+ if (is_src)
aport = get_client_port(&subs->info.dest, &c);
- } else {
- subs = list_entry(p, struct snd_seq_subscribers, dest_list);
+ else
aport = get_client_port(&subs->info.sender, &c);
- }
- list_del(p);
- unsubscribe_port(client, port, grp, &subs->info, 0);
+ delete_and_unsubscribe_port(client, port, subs, is_src, false);
+
if (!aport) {
/* looks like the connected port is being deleted.
* we decrease the counter, and when both ports are deleted
@@ -237,21 +245,14 @@ static void clear_subscriber_list(struct
*/
if (atomic_dec_and_test(&subs->ref_count))
kfree(subs);
- } else {
- /* ok we got the connected port */
- struct snd_seq_port_subs_info *agrp;
- agrp = (grptype == SRC_LIST) ? &aport->c_dest : &aport->c_src;
- down_write(&agrp->list_mutex);
- if (grptype == SRC_LIST)
- list_del(&subs->dest_list);
- else
- list_del(&subs->src_list);
- up_write(&agrp->list_mutex);
- unsubscribe_port(c, aport, agrp, &subs->info, 1);
- kfree(subs);
- snd_seq_port_unlock(aport);
- snd_seq_client_unlock(c);
+ continue;
}
+
+ /* ok we got the connected port */
+ delete_and_unsubscribe_port(c, aport, subs, !is_src, true);
+ kfree(subs);
+ snd_seq_port_unlock(aport);
+ snd_seq_client_unlock(c);
}
}
@@ -264,8 +265,8 @@ static int port_delete(struct snd_seq_cl
snd_use_lock_sync(&port->use_lock);
/* clear subscribers info */
- clear_subscriber_list(client, port, &port->c_src, SRC_LIST);
- clear_subscriber_list(client, port, &port->c_dest, DEST_LIST);
+ clear_subscriber_list(client, port, &port->c_src, true);
+ clear_subscriber_list(client, port, &port->c_dest, false);
if (port->private_free)
port->private_free(port->private_data);
@@ -484,85 +485,120 @@ static int match_subs_info(struct snd_se
return 0;
}
+static int check_and_subscribe_port(struct snd_seq_client *client,
+ struct snd_seq_client_port *port,
+ struct snd_seq_subscribers *subs,
+ bool is_src, bool exclusive, bool ack)
+{
+ struct snd_seq_port_subs_info *grp;
+ struct list_head *p;
+ struct snd_seq_subscribers *s;
+ int err;
-/* connect two ports */
-int snd_seq_port_connect(struct snd_seq_client *connector,
- struct snd_seq_client *src_client,
- struct snd_seq_client_port *src_port,
- struct snd_seq_client *dest_client,
- struct snd_seq_client_port *dest_port,
- struct snd_seq_port_subscribe *info)
-{
- struct snd_seq_port_subs_info *src = &src_port->c_src;
- struct snd_seq_port_subs_info *dest = &dest_port->c_dest;
- struct snd_seq_subscribers *subs, *s;
- int err, src_called = 0;
- unsigned long flags;
- int exclusive;
-
- subs = kzalloc(sizeof(*subs), GFP_KERNEL);
- if (! subs)
- return -ENOMEM;
-
- subs->info = *info;
- atomic_set(&subs->ref_count, 2);
-
- down_write(&src->list_mutex);
- down_write_nested(&dest->list_mutex, SINGLE_DEPTH_NESTING);
-
- exclusive = info->flags & SNDRV_SEQ_PORT_SUBS_EXCLUSIVE ? 1 : 0;
+ grp = is_src ? &port->c_src : &port->c_dest;
err = -EBUSY;
+ down_write(&grp->list_mutex);
if (exclusive) {
- if (! list_empty(&src->list_head) || ! list_empty(&dest->list_head))
+ if (!list_empty(&grp->list_head))
goto __error;
} else {
- if (src->exclusive || dest->exclusive)
+ if (grp->exclusive)
goto __error;
/* check whether already exists */
- list_for_each_entry(s, &src->list_head, src_list) {
- if (match_subs_info(info, &s->info))
- goto __error;
- }
- list_for_each_entry(s, &dest->list_head, dest_list) {
- if (match_subs_info(info, &s->info))
+ list_for_each(p, &grp->list_head) {
+ s = get_subscriber(p, is_src);
+ if (match_subs_info(&subs->info, &s->info))
goto __error;
}
}
- if ((err = subscribe_port(src_client, src_port, src, info,
- connector->number != src_client->number)) < 0)
- goto __error;
- src_called = 1;
-
- if ((err = subscribe_port(dest_client, dest_port, dest, info,
- connector->number != dest_client->number)) < 0)
+ err = subscribe_port(client, port, grp, &subs->info, ack);
+ if (err < 0) {
+ grp->exclusive = 0;
goto __error;
+ }
/* add to list */
- write_lock_irqsave(&src->list_lock, flags);
- // write_lock(&dest->list_lock); // no other lock yet
- list_add_tail(&subs->src_list, &src->list_head);
- list_add_tail(&subs->dest_list, &dest->list_head);
- // write_unlock(&dest->list_lock); // no other lock yet
- write_unlock_irqrestore(&src->list_lock, flags);
+ write_lock_irq(&grp->list_lock);
+ if (is_src)
+ list_add_tail(&subs->src_list, &grp->list_head);
+ else
+ list_add_tail(&subs->dest_list, &grp->list_head);
+ grp->exclusive = exclusive;
+ atomic_inc(&subs->ref_count);
+ write_unlock_irq(&grp->list_lock);
+ err = 0;
- src->exclusive = dest->exclusive = exclusive;
+ __error:
+ up_write(&grp->list_mutex);
+ return err;
+}
+
+static void delete_and_unsubscribe_port(struct snd_seq_client *client,
+ struct snd_seq_client_port *port,
+ struct snd_seq_subscribers *subs,
+ bool is_src, bool ack)
+{
+ struct snd_seq_port_subs_info *grp;
+
+ grp = is_src ? &port->c_src : &port->c_dest;
+ down_write(&grp->list_mutex);
+ write_lock_irq(&grp->list_lock);
+ if (is_src)
+ list_del(&subs->src_list);
+ else
+ list_del(&subs->dest_list);
+ grp->exclusive = 0;
+ write_unlock_irq(&grp->list_lock);
+ up_write(&grp->list_mutex);
+
+ unsubscribe_port(client, port, grp, &subs->info, ack);
+}
+
+/* connect two ports */
+int snd_seq_port_connect(struct snd_seq_client *connector,
+ struct snd_seq_client *src_client,
+ struct snd_seq_client_port *src_port,
+ struct snd_seq_client *dest_client,
+ struct snd_seq_client_port *dest_port,
+ struct snd_seq_port_subscribe *info)
+{
+ struct snd_seq_subscribers *subs;
+ bool exclusive;
+ int err;
+
+ subs = kzalloc(sizeof(*subs), GFP_KERNEL);
+ if (!subs)
+ return -ENOMEM;
+
+ subs->info = *info;
+ atomic_set(&subs->ref_count, 0);
+ INIT_LIST_HEAD(&subs->src_list);
+ INIT_LIST_HEAD(&subs->dest_list);
+
+ exclusive = !!(info->flags & SNDRV_SEQ_PORT_SUBS_EXCLUSIVE);
+
+ err = check_and_subscribe_port(src_client, src_port, subs, true,
+ exclusive,
+ connector->number != src_client->number);
+ if (err < 0)
+ goto error;
+ err = check_and_subscribe_port(dest_client, dest_port, subs, false,
+ exclusive,
+ connector->number != dest_client->number);
+ if (err < 0)
+ goto error_dest;
- up_write(&dest->list_mutex);
- up_write(&src->list_mutex);
return 0;
- __error:
- if (src_called)
- unsubscribe_port(src_client, src_port, src, info,
- connector->number != src_client->number);
+ error_dest:
+ delete_and_unsubscribe_port(src_client, src_port, subs, true,
+ connector->number != src_client->number);
+ error:
kfree(subs);
- up_write(&dest->list_mutex);
- up_write(&src->list_mutex);
return err;
}
-
/* remove the connection */
int snd_seq_port_disconnect(struct snd_seq_client *connector,
struct snd_seq_client *src_client,
@@ -572,37 +608,28 @@ int snd_seq_port_disconnect(struct snd_s
struct snd_seq_port_subscribe *info)
{
struct snd_seq_port_subs_info *src = &src_port->c_src;
- struct snd_seq_port_subs_info *dest = &dest_port->c_dest;
struct snd_seq_subscribers *subs;
int err = -ENOENT;
- unsigned long flags;
down_write(&src->list_mutex);
- down_write_nested(&dest->list_mutex, SINGLE_DEPTH_NESTING);
-
/* look for the connection */
list_for_each_entry(subs, &src->list_head, src_list) {
if (match_subs_info(info, &subs->info)) {
- write_lock_irqsave(&src->list_lock, flags);
- // write_lock(&dest->list_lock); // no lock yet
- list_del(&subs->src_list);
- list_del(&subs->dest_list);
- // write_unlock(&dest->list_lock);
- write_unlock_irqrestore(&src->list_lock, flags);
- src->exclusive = dest->exclusive = 0;
- unsubscribe_port(src_client, src_port, src, info,
- connector->number != src_client->number);
- unsubscribe_port(dest_client, dest_port, dest, info,
- connector->number != dest_client->number);
- kfree(subs);
+ atomic_dec(&subs->ref_count); /* mark as not ready */
err = 0;
break;
}
}
-
- up_write(&dest->list_mutex);
up_write(&src->list_mutex);
- return err;
+ if (err < 0)
+ return err;
+
+ delete_and_unsubscribe_port(src_client, src_port, subs, true,
+ connector->number != src_client->number);
+ delete_and_unsubscribe_port(dest_client, dest_port, subs, false,
+ connector->number != dest_client->number);
+ kfree(subs);
+ return 0;
}
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 3.10 00/64] 3.10.97-stable review Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 03/64] crypto: algif_hash - Only export and import on sockets with data Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 31/64] ALSA: rawmidi: Remove kernel WARNING for NULL user-space buffer check Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 55/64] crypto: af_alg - Disallow bind/setkey/... after accept(2) Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 25/64] fix sysvfs symlinks Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 01/64] recordmcount: Fix endianness handling bug for nop_mcount Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 37/64] ALSA: seq: Fix lockdep warnings due to double mutex locks Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 60/64] crypto: algif_hash - wait for crypto_ahash_init() to complete Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 59/64] ahci: Intel DNV device IDs SATA Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 07/64] kernel/signal.c: unexport sigsuspend() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 24/64] [media] media: vb2 dma-contig: Fully cache synchronise buffers in prepare and finish Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 39/64] ALSA: timer: Fix leftover link at closing Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 28/64] ALSA: compress: Disable GET_CODEC_CAPS ioctl for some architectures Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 23/64] [media] v4l2-compat-ioctl32: fix alignment for ARM64 Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 05/64] HID: usbhid: fix recursive deadlock Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 57/64] AHCI: Fix softreset failed issue of Port Multiplier Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 04/64] dm btree: fix leak of bufio-backed block in btree_split_sibling error path Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:40 +0100
[PATCH 3.10 16/64] vTPM: fix memory allocation flag for rtce buffer at kernel boot Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:50 +0100
[PATCH 3.10 14/64] wlcore/wl12xx: spi: fix oops on firmware load Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:50 +0100
[PATCH 3.10 15/64] wlcore/wl12xx: spi: fix NULL pointer dereference (Oops) Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:50 +0100
[PATCH 3.10 22/64] parisc: Fix __ARCH_SI_PREAMBLE_SIZE Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:50 +0100
[PATCH 3.10 19/64] [PATCH] fix calculation of meta_bg descriptor backups Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:50 +0100
[PATCH 3.10 18/64] jbd2: Fix unreclaimed pages after truncate in data=journal mode Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:50 +0100
[PATCH 3.10 17/64] mtd: mtdpart: fix add_mtd_partitions error path Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:50 +0100
[PATCH 3.10 12/64] spi: atmel: Fix DMA-setup for transfers with more than 8 bits per word Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:50 +0100
[PATCH 3.10 21/64] parisc: Fix syscall restarts Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:50 +0100
[PATCH 3.10 11/64] Revert "dm mpath: fix stalls when handling invalid ioctls" Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:50 +0100
[PATCH 3.10 13/64] spi: fix parent-device reference leak Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:50 +0100
[PATCH 3.10 02/64] xhci: fix placement of call to usb_disabled() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 00:50 +0100
Re: [PATCH 3.10 00/64] 3.10.97-stable review Guenter Roeck <linux@roeck-us.net> - 2016-02-15 06:30 +0100
Re: [PATCH 3.10 00/64] 3.10.97-stable review Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-15 19:20 +0100
Re: [PATCH 3.10 00/64] 3.10.97-stable review Guenter Roeck <linux@roeck-us.net> - 2016-02-15 16:50 +0100
Re: [PATCH 3.10 00/64] 3.10.97-stable review Shuah Khan <shuahkh@osg.samsung.com> - 2016-02-15 18:20 +0100
Re: [PATCH 3.10 00/64] 3.10.97-stable review Shuah Khan <shuahkh@osg.samsung.com> - 2016-02-17 21:40 +0100
Re: [PATCH 3.10 00/64] 3.10.97-stable review Willy Tarreau <w@1wt.eu> - 2016-02-18 00:30 +0100
Re: [PATCH 3.10 00/64] 3.10.97-stable review Shuah Khan <shuahkh@osg.samsung.com> - 2016-02-18 00:40 +0100
Re: [PATCH 3.10 00/64] 3.10.97-stable review Shuah Khan <shuahkh@osg.samsung.com> - 2016-02-18 04:30 +0100
Re: [PATCH 3.10 00/64] 3.10.97-stable review Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-19 23:30 +0100
Re: [PATCH 3.10 00/64] 3.10.97-stable review Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-17 21:40 +0100
csiph-web