Message ID | 1405612775-10396-1-git-send-email-luis.henriques@canonical.com |
---|---|
State | New |
Headers | show |
On 07/17/2014 08:59 AM, Luis Henriques wrote: > From: Lars-Peter Clausen <lars@metafoo.de> > > 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> > (cherry picked from commit 07f4d9d74a04aa7c72c5dae0ef97565f28f17b92) > CVE-2014-4652 > BugLink: http://bugs.launchpad.net/bugs/1339294 > Signed-off-by: Luis Henriques <luis.henriques@canonical.com> > --- > include/sound/core.h | 2 ++ > sound/core/control.c | 31 +++++++++++++++++++++++++------ > sound/core/init.c | 1 + > 3 files changed, 28 insertions(+), 6 deletions(-) > > diff --git a/include/sound/core.h b/include/sound/core.h > index a61499c22b0b..3ad641cdc735 100644 > --- 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 7834a5438f75..b5266f7a1a0a 100644 > --- a/sound/core/control.c > +++ b/sound/core/control.c > @@ -873,6 +873,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 */ > @@ -895,7 +896,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; > } > > @@ -904,10 +907,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; > } > > @@ -927,19 +932,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; > } > @@ -1028,6 +1046,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 82f350e00cbe..fd8590fefbd9 100644 > --- 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); >
diff --git a/include/sound/core.h b/include/sound/core.h index a61499c22b0b..3ad641cdc735 100644 --- 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 7834a5438f75..b5266f7a1a0a 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -873,6 +873,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 */ @@ -895,7 +896,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; } @@ -904,10 +907,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; } @@ -927,19 +932,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; } @@ -1028,6 +1046,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 82f350e00cbe..fd8590fefbd9 100644 --- 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);