[alsa-devel] [PATCH] control_hw: Fix issue when applying seccomp policy

[Takashi Iwai]

On Thu, 16 Aug 2018 11:06:33 +0200,
Hsin-yu Chao wrote:
> 
> The problem happens when I add seccomp filter to whiltelist syscall
> "ioctl" with arguments of type 'U', for SNDRV_CTL_IOCTL_TLV_READ,
> SNDRV_CTL_IOCTL_TLV_WRITE and SNDRV_CTL_IOCTL_TLV_COMMAND.
> I was expecting snd_ctl_hw_elem_tlv() to run without problem, but
> actually seeing it return error because of ioctl() call failure.
> 
> In control_hw.c, when the local int "inum" got passed to ioctl, it is
> converted to "unsigned long".
> Take SNDRV_CTL_IOCTL_TLV_WRITE as example, it will stretch to
> 0xffffffffc008551b.
> 
> This doesn't bring problem to snd function, I think it's because
> snd_pcm_lib_ioctl takes "unsigned int cmd" as argument.
> https://github.com/torvalds/linux/blob/master/sound/core/pcm_lib.c#L1769
> 
> However the seccomp_data struct takes 64 bits argument to check
> against seccomp rules.
> https://github.com/torvalds/linux/blob/master/arch/x86/entry/common.c#L104
> and these unexpected 0xff bytes make the seccomp rule check fail.
> 
> Since ioctl takes "unsigned long" command, I think we should fix this
> in alsa-lib as there is no intention to append the 0xff bytes to
> kernel.

Thanks, that makes sense.

Could you add these background information concisely in the patch
description and resubmit?


Takashi

> 
> Thanks,
> Hsin-yu
> 
> On Wed, Aug 15, 2018 at 5:37 PM Takashi Iwai <tiwai at suse.de> wrote:
> >
> > On Wed, 15 Aug 2018 10:17:50 +0200,
> > Hsin-Yu Chao wrote:
> > >
> > > When seccomp policy is applied to filter ioctl syscall with
> > > SNDRV_CTL_IOCTL_TLV_COMMAND, SNDRV_CTL_IOCTL_TLV_READ and
> > > SNDRV_CTL_IOCTL_TLV_WRITE in whiltelist, alsa-lib still breaks
> > > in at snd_ctl_hw_elem_tlv().
> > > Fix the problem by passing unsigned int to ioctl.
> >
> > Could you explain exactly what breaks and how?
> >
> >
> > thanks,
> >
> > Takashi
> >
> > >
> > > Signed-off-by: Hsin-Yu Chao <hychao at chromium.org>
> > > ---
> > >  src/control/control_hw.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/src/control/control_hw.c b/src/control/control_hw.c
> > > index 68eca522..b54d65f2 100644
> > > --- a/src/control/control_hw.c
> > > +++ b/src/control/control_hw.c
> > > @@ -215,7 +215,7 @@ static int snd_ctl_hw_elem_tlv(snd_ctl_t *handle, int op_flag,
> > >                              unsigned int numid,
> > >                              unsigned int *tlv, unsigned int tlv_size)
> > >  {
> > > -     int inum;
> > > +     unsigned int inum;
> > >       snd_ctl_hw_t *hw = handle->private_data;
> > >       struct snd_ctl_tlv *xtlv;
> > >
> > > --
> > > 2.18.0.865.gffc8e1a3cd6-goog
> > >
> > > _______________________________________________
> > > Alsa-devel mailing list
> > > Alsa-devel at alsa-project.org
> > > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> > >
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>