ALSA: control: Protect user controls against concurrent access

[ Upstream commit 07f4d9d74a04aa7c72c5dae0ef97565f28f17b92 ]

The user-control put and get handlers as well as the tlv do not protect against
concurrent access from multiple threads. Since the state of the control is not
updated atomically it is possible that either two write operations or a write
and a read operation race against each other. Both can lead to arbitrary memory
disclosure. This patch introduces a new lock that protects user-controls from
concurrent access. Since applications typically access controls sequentially
than in parallel a single lock per card should be fine.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jaroslav Kysela <perex@perex.cz>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
[wt: fixes CVE-2014-4652]
Signed-off-by: Willy Tarreau <w@1wt.eu>

diff --git a/include/sound/core.h b/include/sound/core.h
index a61499c22b0b96044314048791b3152e5249d98e..3ad641cdc7356b6aa2fc0a92ee47d0a337a695dc 100644 (file)
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -120,6 +120,8 @@ struct snd_card {
        int user_ctl_count;             /* count of all user controls */
        struct list_head controls;      /* all controls for this card */
        struct list_head ctl_files;     /* active control files */
+       struct mutex user_ctl_lock;     /* protects user controls against
+                                          concurrent access */
 
        struct snd_info_entry *proc_root;       /* root for soundcard specific files */
        struct snd_info_entry *proc_id; /* the card id */

diff --git a/sound/core/control.c b/sound/core/control.c
index d7f4644b86023825562934fe88be0e9d788fe579..8bf3a6d1ebe241bba1c09553cda92fe670ce44a6 100644 (file)
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -880,6 +880,7 @@ static int snd_ctl_elem_unlock(struct snd_ctl_file *file,
 
 struct user_element {
        struct snd_ctl_elem_info info;
+       struct snd_card *card;
        void *elem_data;                /* element data */
        unsigned long elem_data_size;   /* size of element data in bytes */
        void *tlv_data;                 /* TLV data */
@@ -902,7 +903,9 @@ static int snd_ctl_elem_user_get(struct snd_kcontrol *kcontrol,
 {
        struct user_element *ue = kcontrol->private_data;
 
+       mutex_lock(&ue->card->user_ctl_lock);
        memcpy(&ucontrol->value, ue->elem_data, ue->elem_data_size);
+       mutex_unlock(&ue->card->user_ctl_lock);
        return 0;
 }
 
@@ -911,10 +914,12 @@ static int snd_ctl_elem_user_put(struct snd_kcontrol *kcontrol,
 {
        int change;
        struct user_element *ue = kcontrol->private_data;
-       
+
+       mutex_lock(&ue->card->user_ctl_lock);
        change = memcmp(&ucontrol->value, ue->elem_data, ue->elem_data_size) != 0;
        if (change)
                memcpy(ue->elem_data, &ucontrol->value, ue->elem_data_size);
+       mutex_unlock(&ue->card->user_ctl_lock);
        return change;
 }
 
@@ -934,19 +939,32 @@ static int snd_ctl_elem_user_tlv(struct snd_kcontrol *kcontrol,
                new_data = memdup_user(tlv, size);
                if (IS_ERR(new_data))
                        return PTR_ERR(new_data);
+               mutex_lock(&ue->card->user_ctl_lock);
                change = ue->tlv_data_size != size;
                if (!change)
                        change = memcmp(ue->tlv_data, new_data, size);
                kfree(ue->tlv_data);
                ue->tlv_data = new_data;
                ue->tlv_data_size = size;
+               mutex_unlock(&ue->card->user_ctl_lock);
        } else {
-               if (! ue->tlv_data_size || ! ue->tlv_data)
-                       return -ENXIO;
-               if (size < ue->tlv_data_size)
-                       return -ENOSPC;
+               int ret = 0;
+
+               mutex_lock(&ue->card->user_ctl_lock);
+               if (!ue->tlv_data_size || !ue->tlv_data) {
+                       ret = -ENXIO;
+                       goto err_unlock;
+               }
+               if (size < ue->tlv_data_size) {
+                       ret = -ENOSPC;
+                       goto err_unlock;
+               }
                if (copy_to_user(tlv, ue->tlv_data, ue->tlv_data_size))
-                       return -EFAULT;
+                       ret = -EFAULT;
+err_unlock:
+               mutex_unlock(&ue->card->user_ctl_lock);
+               if (ret)
+                       return ret;
        }
        return change;
 }
@@ -1035,6 +1053,7 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file,
        ue = kzalloc(sizeof(struct user_element) + private_size, GFP_KERNEL);
        if (ue == NULL)
                return -ENOMEM;
+       ue->card = card;
        ue->info = *info;
        ue->info.access = 0;
        ue->elem_data = (char *)ue + sizeof(*ue);

diff --git a/sound/core/init.c b/sound/core/init.c
index 82f350e00cbe2e00f8d4ca58845f2394dc6a3638..fd8590fefbd9a0ca32b5c14372c6fc64f32dc687 100644 (file)
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -206,6 +206,7 @@ int snd_card_create(int idx, const char *xid,
        INIT_LIST_HEAD(&card->devices);
        init_rwsem(&card->controls_rwsem);
        rwlock_init(&card->ctl_files_rwlock);
+       mutex_init(&card->user_ctl_lock);
        INIT_LIST_HEAD(&card->controls);
        INIT_LIST_HEAD(&card->ctl_files);
        spin_lock_init(&card->files_lock);