[alsa-devel] [PATCH] control_hw: Fix issue when applying seccomp policy
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.
Signed-off-by: Hsin-Yu Chao hychao@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;
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@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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
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, Hsin-yu
On Wed, Aug 15, 2018 at 5:37 PM Takashi Iwai tiwai@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@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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
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@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@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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (2)
-
Hsin-Yu Chao
-
Takashi Iwai