Skip to content

Commit 07f4d9d

Browse files
larsclausentiwai
authored andcommitted
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>
1 parent 7171511 commit 07f4d9d

File tree

3 files changed

+28
-6
lines changed

3 files changed

+28
-6
lines changed

Diff for: include/sound/core.h

+2
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ struct snd_card {
116116
int user_ctl_count; /* count of all user controls */
117117
struct list_head controls; /* all controls for this card */
118118
struct list_head ctl_files; /* active control files */
119+
struct mutex user_ctl_lock; /* protects user controls against
120+
concurrent access */
119121

120122
struct snd_info_entry *proc_root; /* root for soundcard specific files */
121123
struct snd_info_entry *proc_id; /* the card id */

Diff for: sound/core/control.c

+25-6
Original file line numberDiff line numberDiff line change
@@ -991,6 +991,7 @@ static int snd_ctl_elem_unlock(struct snd_ctl_file *file,
991991

992992
struct user_element {
993993
struct snd_ctl_elem_info info;
994+
struct snd_card *card;
994995
void *elem_data; /* element data */
995996
unsigned long elem_data_size; /* size of element data in bytes */
996997
void *tlv_data; /* TLV data */
@@ -1034,7 +1035,9 @@ static int snd_ctl_elem_user_get(struct snd_kcontrol *kcontrol,
10341035
{
10351036
struct user_element *ue = kcontrol->private_data;
10361037

1038+
mutex_lock(&ue->card->user_ctl_lock);
10371039
memcpy(&ucontrol->value, ue->elem_data, ue->elem_data_size);
1040+
mutex_unlock(&ue->card->user_ctl_lock);
10381041
return 0;
10391042
}
10401043

@@ -1043,10 +1046,12 @@ static int snd_ctl_elem_user_put(struct snd_kcontrol *kcontrol,
10431046
{
10441047
int change;
10451048
struct user_element *ue = kcontrol->private_data;
1046-
1049+
1050+
mutex_lock(&ue->card->user_ctl_lock);
10471051
change = memcmp(&ucontrol->value, ue->elem_data, ue->elem_data_size) != 0;
10481052
if (change)
10491053
memcpy(ue->elem_data, &ucontrol->value, ue->elem_data_size);
1054+
mutex_unlock(&ue->card->user_ctl_lock);
10501055
return change;
10511056
}
10521057

@@ -1066,19 +1071,32 @@ static int snd_ctl_elem_user_tlv(struct snd_kcontrol *kcontrol,
10661071
new_data = memdup_user(tlv, size);
10671072
if (IS_ERR(new_data))
10681073
return PTR_ERR(new_data);
1074+
mutex_lock(&ue->card->user_ctl_lock);
10691075
change = ue->tlv_data_size != size;
10701076
if (!change)
10711077
change = memcmp(ue->tlv_data, new_data, size);
10721078
kfree(ue->tlv_data);
10731079
ue->tlv_data = new_data;
10741080
ue->tlv_data_size = size;
1081+
mutex_unlock(&ue->card->user_ctl_lock);
10751082
} else {
1076-
if (! ue->tlv_data_size || ! ue->tlv_data)
1077-
return -ENXIO;
1078-
if (size < ue->tlv_data_size)
1079-
return -ENOSPC;
1083+
int ret = 0;
1084+
1085+
mutex_lock(&ue->card->user_ctl_lock);
1086+
if (!ue->tlv_data_size || !ue->tlv_data) {
1087+
ret = -ENXIO;
1088+
goto err_unlock;
1089+
}
1090+
if (size < ue->tlv_data_size) {
1091+
ret = -ENOSPC;
1092+
goto err_unlock;
1093+
}
10801094
if (copy_to_user(tlv, ue->tlv_data, ue->tlv_data_size))
1081-
return -EFAULT;
1095+
ret = -EFAULT;
1096+
err_unlock:
1097+
mutex_unlock(&ue->card->user_ctl_lock);
1098+
if (ret)
1099+
return ret;
10821100
}
10831101
return change;
10841102
}
@@ -1210,6 +1228,7 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file,
12101228
ue = kzalloc(sizeof(struct user_element) + private_size, GFP_KERNEL);
12111229
if (ue == NULL)
12121230
return -ENOMEM;
1231+
ue->card = card;
12131232
ue->info = *info;
12141233
ue->info.access = 0;
12151234
ue->elem_data = (char *)ue + sizeof(*ue);

Diff for: sound/core/init.c

+1
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ int snd_card_new(struct device *parent, int idx, const char *xid,
232232
INIT_LIST_HEAD(&card->devices);
233233
init_rwsem(&card->controls_rwsem);
234234
rwlock_init(&card->ctl_files_rwlock);
235+
mutex_init(&card->user_ctl_lock);
235236
INIT_LIST_HEAD(&card->controls);
236237
INIT_LIST_HEAD(&card->ctl_files);
237238
spin_lock_init(&card->files_lock);

0 commit comments

Comments
 (0)