CVE-2014-4652: Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition') (CWE-362)
value: Race condition in the tlv handler functionality in the snd_ctl_elem_user_tlv function in sound/core/control.c in the ALSA control implementation in the Linux kernel before 3.15.2 allows local users to obtain sensitive information from kernel memory by leveraging /dev/snd/controlCX access.
CWE description: The product contains a code sequence that can run concurrently with other code, and the code sequence requires temporary, exclusive access to a shared resource, but a timing window exists in which the shared resource can be modified by another code sequence that is operating concurrently.
Published: 2014-07-03T04:22Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: include/sound/core.h, sound/core/control.c, sound/core/init.c
Commit date: 2014-06-18 15:12:33 +0200
Commit message: ALSA: control: Protect user controls against concurrent access

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>

Diff: diff --git a/include/sound/core.h b/include/sound/core.h
index eedda2cdfe5796..1df3f2fe5350f8 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -114,10 +114,12 @@ struct snd_card {
 	rwlock_t ctl_files_rwlock;	/* ctl_files list lock */
 	int controls_count;		/* count of all controls */
 	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 */
 	struct proc_dir_entry *proc_root_link;	/* number link to real id */
 
diff --git a/sound/core/control.c b/sound/core/control.c
index f038f5afafe2bf..00ab034f5fcbee 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -989,10 +989,11 @@ static int snd_ctl_elem_unlock(struct snd_ctl_file *file,
 	return result;
 }
 
 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 */
 	unsigned long tlv_data_size;	/* TLV data size */
 	void *priv_data;		/* private data (like strings for enumerated type) */
@@ -1032,23 +1033,27 @@ static int snd_ctl_elem_user_enum_info(struct snd_kcontrol *kcontrol,
 static int snd_ctl_elem_user_get(struct snd_kcontrol *kcontrol,
 				 struct snd_ctl_elem_value *ucontrol)
 {
 	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;
 }
 
 static int snd_ctl_elem_user_put(struct snd_kcontrol *kcontrol,
 				 struct snd_ctl_elem_value *ucontrol)
 {
 	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;
 }
 
 static int snd_ctl_elem_user_tlv(struct snd_kcontrol *kcontrol,
 				 int op_flag,
@@ -1064,23 +1069,36 @@ static int snd_ctl_elem_user_tlv(struct snd_kcontrol *kcontrol,
 			return -EINVAL;
 
 		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;
 }
 
 static int snd_ctl_elem_init_enum_names(struct user_element *ue)
@@ -1208,10 +1226,11 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file,
 	}
 	private_size *= info->count;
 	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);
 	ue->elem_data_size = private_size;
 	if (ue->info.type == SNDRV_CTL_ELEM_TYPE_ENUMERATED) {
diff --git a/sound/core/init.c b/sound/core/init.c
index 5ee83845c5de57..7bdfd19e24a80a 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -230,10 +230,11 @@ int snd_card_new(struct device *parent, int idx, const char *xid,
 	card->number = idx;
 	card->module = module;
 	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);
 	INIT_LIST_HEAD(&card->files_list);
 #ifdef CONFIG_PM

Vulnerable code: 	
		if (! ue->tlv_data_size || ! ue->tlv_data)
			return -ENXIO;
		if (size < ue->tlv_data_size)
			return -ENOSPC;
			return -EFAULT;
Fixed code: 	struct mutex user_ctl_lock;	/* protects user controls against
					   concurrent access */
	struct snd_card *card;
	mutex_lock(&ue->card->user_ctl_lock);
	mutex_unlock(&ue->card->user_ctl_lock);

	mutex_lock(&ue->card->user_ctl_lock);
	mutex_unlock(&ue->card->user_ctl_lock);
		mutex_lock(&ue->card->user_ctl_lock);
		mutex_unlock(&ue->card->user_ctl_lock);
		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;
		}
			ret = -EFAULT;
err_unlock:
		mutex_unlock(&ue->card->user_ctl_lock);
		if (ret)
			return ret;
	ue->card = card;
	mutex_init(&card->user_ctl_lock);