[alsa-devel] [PATCH 2/3] ALSA: control: add dimension validator for userspace element

[Takashi Sakamoto]

On Jul 1 2016 20:10, Takashi Sakamoto wrote:
> The 'dimen' field in struct snd_ctl_elem_info is used to compose all of
> members in the element as multi-dimensional matrix. The field has four
> members. Each member represents the width in each dimension level by
> element member unit. For example, if the members consist of typical
> two dimensional matrix, the dimen[0] represents the number of rows
> and dimen[1] represents the number of columns (or vise-versa).
> 
> The total members in the matrix should be within the number of members in
> the element, while current implementation has no validator of this
> information. In a view of userspace applications, the information must be
> valid so that it cannot cause any bugs such as buffer-over-run.
> 
> This commit adds a validator of dimension information for userspace
> applications which add new element sets. When they add the element sets
> with wrong dimension information, they receive -EINVAL.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi at sakamocchi.jp>
> ---
>  sound/core/control.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/sound/core/control.c b/sound/core/control.c
> index a85d455..54da910 100644
> --- a/sound/core/control.c
> +++ b/sound/core/control.c
> @@ -805,6 +805,34 @@ static int snd_ctl_elem_list(struct snd_card *card,
>  	return 0;
>  }
>  
> +static bool validate_element_member_dimension(struct snd_ctl_elem_info *info)
> +{
> +	unsigned int members;
> +	unsigned int i = 0;
> +
> +	if (info->dimen.d[0] == 0)
> +		return true;
> +
> +	members = 1;
> +	for (i = 0; i < ARRAY_SIZE(info->dimen.d); ++i) {
> +		if (info->dimen.d[i] < 0)
> +			return false;

Mmm... info->dimen.d is declared with 'unsigned short' type. Thus,
negative value check is needless...

struct snd_ctl_elem_info {
    ...
    union {
        unsigned short d[4];
        ..
    };
    ...
};

Please abandon this patchset. I'll post new one tomorrow.

> +		if (info->dimen.d[i] == 0)
> +			break;
> +
> +		members *= info->dimen.d[i];
> +		if (members > info->count)
> +			return false;
> +	}
> +
> +	for (++i; i < ARRAY_SIZE(info->dimen.d); ++i) {
> +		if (info->dimen.d[i] != 0)
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
>  static int snd_ctl_elem_info(struct snd_ctl_file *ctl,
>  			     struct snd_ctl_elem_info *info)
>  {
> @@ -1272,6 +1300,8 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file,
>  	if (info->count < 1 ||
>  	    info->count > max_value_counts[info->type])
>  		return -EINVAL;
> +	if (!validate_element_member_dimension(info))
> +		return -EINVAL;
>  	private_size = value_sizes[info->type] * info->count;
>  
>  	/*
>