Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

Lars-Peter Clausen and committed by
Takashi Iwai
07f4d9d7 7171511e

+28 -6
+2
include/sound/core.h
··· 116 116 int user_ctl_count; /* count of all user controls */ 117 117 struct list_head controls; /* all controls for this card */ 118 118 struct list_head ctl_files; /* active control files */ 119 + struct mutex user_ctl_lock; /* protects user controls against 120 + concurrent access */ 119 121 120 122 struct snd_info_entry *proc_root; /* root for soundcard specific files */ 121 123 struct snd_info_entry *proc_id; /* the card id */
+25 -6
sound/core/control.c
··· 991 991 992 992 struct user_element { 993 993 struct snd_ctl_elem_info info; 994 + struct snd_card *card; 994 995 void *elem_data; /* element data */ 995 996 unsigned long elem_data_size; /* size of element data in bytes */ 996 997 void *tlv_data; /* TLV data */ ··· 1035 1034 { 1036 1035 struct user_element *ue = kcontrol->private_data; 1037 1036 1037 + mutex_lock(&ue->card->user_ctl_lock); 1038 1038 memcpy(&ucontrol->value, ue->elem_data, ue->elem_data_size); 1039 + mutex_unlock(&ue->card->user_ctl_lock); 1039 1040 return 0; 1040 1041 } 1041 1042 ··· 1046 1043 { 1047 1044 int change; 1048 1045 struct user_element *ue = kcontrol->private_data; 1049 - 1046 + 1047 + mutex_lock(&ue->card->user_ctl_lock); 1050 1048 change = memcmp(&ucontrol->value, ue->elem_data, ue->elem_data_size) != 0; 1051 1049 if (change) 1052 1050 memcpy(ue->elem_data, &ucontrol->value, ue->elem_data_size); 1051 + mutex_unlock(&ue->card->user_ctl_lock); 1053 1052 return change; 1054 1053 } 1055 1054 ··· 1071 1066 new_data = memdup_user(tlv, size); 1072 1067 if (IS_ERR(new_data)) 1073 1068 return PTR_ERR(new_data); 1069 + mutex_lock(&ue->card->user_ctl_lock); 1074 1070 change = ue->tlv_data_size != size; 1075 1071 if (!change) 1076 1072 change = memcmp(ue->tlv_data, new_data, size); 1077 1073 kfree(ue->tlv_data); 1078 1074 ue->tlv_data = new_data; 1079 1075 ue->tlv_data_size = size; 1076 + mutex_unlock(&ue->card->user_ctl_lock); 1080 1077 } else { 1081 - if (! ue->tlv_data_size || ! ue->tlv_data) 1082 - return -ENXIO; 1083 - if (size < ue->tlv_data_size) 1084 - return -ENOSPC; 1078 + int ret = 0; 1079 + 1080 + mutex_lock(&ue->card->user_ctl_lock); 1081 + if (!ue->tlv_data_size || !ue->tlv_data) { 1082 + ret = -ENXIO; 1083 + goto err_unlock; 1084 + } 1085 + if (size < ue->tlv_data_size) { 1086 + ret = -ENOSPC; 1087 + goto err_unlock; 1088 + } 1085 1089 if (copy_to_user(tlv, ue->tlv_data, ue->tlv_data_size)) 1086 - return -EFAULT; 1090 + ret = -EFAULT; 1091 + err_unlock: 1092 + mutex_unlock(&ue->card->user_ctl_lock); 1093 + if (ret) 1094 + return ret; 1087 1095 } 1088 1096 return change; 1089 1097 } ··· 1228 1210 ue = kzalloc(sizeof(struct user_element) + private_size, GFP_KERNEL); 1229 1211 if (ue == NULL) 1230 1212 return -ENOMEM; 1213 + ue->card = card; 1231 1214 ue->info = *info; 1232 1215 ue->info.access = 0; 1233 1216 ue->elem_data = (char *)ue + sizeof(*ue);
+1
sound/core/init.c
··· 232 232 INIT_LIST_HEAD(&card->devices); 233 233 init_rwsem(&card->controls_rwsem); 234 234 rwlock_init(&card->ctl_files_rwlock); 235 + mutex_init(&card->user_ctl_lock); 235 236 INIT_LIST_HEAD(&card->controls); 236 237 INIT_LIST_HEAD(&card->ctl_files); 237 238 spin_lock_init(&card->files_lock);