| Message ID | 1428741665-22262-4-git-send-email-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | cab2ed7474bffafd2a68a885e03b85526194abcd |
| Headers | show |
At Sat, 11 Apr 2015 17:41:04 +0900, Takashi Sakamoto wrote: > > currently some members related identical information are not fiiled > in returned parameter of SNDRV_CTL_IOCTL_ELEM_ADD. This is not better > for userspace application. > > This commit copies information to returned value. When failing to copy > into userspace, the added elements are going to be removed. Then, no > applications can lock these elements between adding and removing because > these are already locked. > > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> Applied, thanks. Takashi > --- > sound/core/control.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/sound/core/control.c b/sound/core/control.c > index a750846..ccb1ca2 100644 > --- a/sound/core/control.c > +++ b/sound/core/control.c > @@ -1214,6 +1214,7 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, > unsigned int access; > long private_size; > struct user_element *ue; > + unsigned int offset; > int err; > > if (!*info->id.name) > @@ -1316,6 +1317,15 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, > err = snd_ctl_add(card, kctl); > if (err < 0) > return err; > + offset = snd_ctl_get_ioff(kctl, &info->id); > + snd_ctl_build_ioff(&info->id, kctl, offset); > + /* > + * Here we cannot fill any field for the number of elements added by > + * this operation because there're no specific fields. The usage of > + * 'owner' field for this purpose may cause any bugs to userspace > + * applications because the field originally means PID of a process > + * which locks the element. > + */ > > down_write(&card->controls_rwsem); > card->user_ctl_count++; > @@ -1328,9 +1338,19 @@ static int snd_ctl_elem_add_user(struct snd_ctl_file *file, > struct snd_ctl_elem_info __user *_info, int replace) > { > struct snd_ctl_elem_info info; > + int err; > + > if (copy_from_user(&info, _info, sizeof(info))) > return -EFAULT; > - return snd_ctl_elem_add(file, &info, replace); > + err = snd_ctl_elem_add(file, &info, replace); > + if (err < 0) > + return err; > + if (copy_to_user(_info, &info, sizeof(info))) { > + snd_ctl_remove_user_ctl(file, &info.id); > + return -EFAULT; > + } > + > + return 0; > } > > static int snd_ctl_elem_remove(struct snd_ctl_file *file, > -- > 2.1.0 >
diff --git a/sound/core/control.c b/sound/core/control.c index a750846..ccb1ca2 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1214,6 +1214,7 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, unsigned int access; long private_size; struct user_element *ue; + unsigned int offset; int err; if (!*info->id.name) @@ -1316,6 +1317,15 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, err = snd_ctl_add(card, kctl); if (err < 0) return err; + offset = snd_ctl_get_ioff(kctl, &info->id); + snd_ctl_build_ioff(&info->id, kctl, offset); + /* + * Here we cannot fill any field for the number of elements added by + * this operation because there're no specific fields. The usage of + * 'owner' field for this purpose may cause any bugs to userspace + * applications because the field originally means PID of a process + * which locks the element. + */ down_write(&card->controls_rwsem); card->user_ctl_count++; @@ -1328,9 +1338,19 @@ static int snd_ctl_elem_add_user(struct snd_ctl_file *file, struct snd_ctl_elem_info __user *_info, int replace) { struct snd_ctl_elem_info info; + int err; + if (copy_from_user(&info, _info, sizeof(info))) return -EFAULT; - return snd_ctl_elem_add(file, &info, replace); + err = snd_ctl_elem_add(file, &info, replace); + if (err < 0) + return err; + if (copy_to_user(_info, &info, sizeof(info))) { + snd_ctl_remove_user_ctl(file, &info.id); + return -EFAULT; + } + + return 0; } static int snd_ctl_elem_remove(struct snd_ctl_file *file,
currently some members related identical information are not fiiled in returned parameter of SNDRV_CTL_IOCTL_ELEM_ADD. This is not better for userspace application. This commit copies information to returned value. When failing to copy into userspace, the added elements are going to be removed. Then, no applications can lock these elements between adding and removing because these are already locked. Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> --- sound/core/control.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)