On Fri, 01 Jul 2016 14:29:37 +0200,
Takashi Sakamoto wrote:
>
> 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@xxxxxxxxxxxxx>
> > ---
> > 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;
Unnecessary initialization.
> > +
> > + 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.
Indeed, somehow I overlooked it, too.
While we're at it: maybe it's even safer to check more strictly the
result whether it matches with info->count. I don't think there is
any reason to have an unaligned info->count value when the dimen array
is given. It must be a bug.
That is:
> > + 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;
Here will be
return members == info->count;
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
