[Qemu-devel] [PATCH] linux-user: return EINVAL from prctl(PR_*_SECCOMP)

[Qemu-devel] [PATCH] linux-user: return EINVAL from prctl(PR_*_SECCOMP)

[James Cowgill]

If an application tries to install a seccomp filter using
prctl(PR_SET_SECCOMP), the filter is likely for the target instead of the host
architecture. This will probably cause qemu to be immediately killed when it
executes another syscall.

Prevent this from happening by returning EINVAL from both seccomp prctl
calls. This is the error returned by the kernel when seccomp support is
disabled.

Fixes: https://bugs.launchpad.net/qemu/+bug/1726394
Signed-off-by: James Cowgill <james.cowgill@mips.com>
---
 linux-user/syscall.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d4497dec5d..43cd5fb2bb 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10482,6 +10482,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             break;
         }
 #endif
+        case PR_GET_SECCOMP:
+        case PR_SET_SECCOMP:
+            ret = -TARGET_EINVAL;
+            break;
         default:
             /* Most prctl options have no pointer arguments */
             ret = get_errno(prctl(arg1, arg2, arg3, arg4, arg5));
-- 
2.15.0

Re: [Qemu-devel] [PATCH] linux-user: return EINVAL from prctl(PR_*_SECCOMP)

[Laurent Vivier]

Le 03/11/2017 à 13:07, James Cowgill a écrit :
> If an application tries to install a seccomp filter using
> prctl(PR_SET_SECCOMP), the filter is likely for the target instead of the host
> architecture. This will probably cause qemu to be immediately killed when it
> executes another syscall.
> 
> Prevent this from happening by returning EINVAL from both seccomp prctl
> calls. This is the error returned by the kernel when seccomp support is
> disabled.
> 
> Fixes: https://bugs.launchpad.net/qemu/+bug/1726394
> Signed-off-by: James Cowgill <james.cowgill@mips.com>
> ---
>  linux-user/syscall.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index d4497dec5d..43cd5fb2bb 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -10482,6 +10482,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              break;
>          }
>  #endif
> +        case PR_GET_SECCOMP:
> +        case PR_SET_SECCOMP:
> +            ret = -TARGET_EINVAL;
> +            break;
>          default:
>              /* Most prctl options have no pointer arguments */
>              ret = get_errno(prctl(arg1, arg2, arg3, arg4, arg5));
> 

I think we should allow PR_GET_SECCOMP, and at least all the modes
except SECCOMP_MODE_FILTER for PR_SET_SECCOMP.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH] linux-user: return EINVAL from prctl(PR_*_SECCOMP)

[Peter Maydell]

On 3 November 2017 at 12:07, James Cowgill <james.cowgill@mips.com> wrote:
> If an application tries to install a seccomp filter using
> prctl(PR_SET_SECCOMP), the filter is likely for the target instead of the host
> architecture. This will probably cause qemu to be immediately killed when it
> executes another syscall.
>
> Prevent this from happening by returning EINVAL from both seccomp prctl
> calls. This is the error returned by the kernel when seccomp support is
> disabled.
>
> Fixes: https://bugs.launchpad.net/qemu/+bug/1726394
> Signed-off-by: James Cowgill <james.cowgill@mips.com>
> ---
>  linux-user/syscall.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index d4497dec5d..43cd5fb2bb 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -10482,6 +10482,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              break;
>          }
>  #endif
> +        case PR_GET_SECCOMP:
> +        case PR_SET_SECCOMP:
> +            ret = -TARGET_EINVAL;
> +            break;
>          default:
>              /* Most prctl options have no pointer arguments */
>              ret = get_errno(prctl(arg1, arg2, arg3, arg4, arg5));
>

Seems like a good idea -- can we have the "why are we failing this"
in a comment as well as in the commit message, please?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] linux-user: return EINVAL from prctl(PR_*_SECCOMP)

[James Cowgill]

Hi,

On 03/11/17 14:29, Laurent Vivier wrote:
> Le 03/11/2017 à 13:07, James Cowgill a écrit :
>> If an application tries to install a seccomp filter using
>> prctl(PR_SET_SECCOMP), the filter is likely for the target instead of the host
>> architecture. This will probably cause qemu to be immediately killed when it
>> executes another syscall.
>>
>> Prevent this from happening by returning EINVAL from both seccomp prctl
>> calls. This is the error returned by the kernel when seccomp support is
>> disabled.
>>
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1726394
>> Signed-off-by: James Cowgill <james.cowgill@mips.com>
>> ---
>>  linux-user/syscall.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index d4497dec5d..43cd5fb2bb 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -10482,6 +10482,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>>              break;
>>          }
>>  #endif
>> +        case PR_GET_SECCOMP:
>> +        case PR_SET_SECCOMP:
>> +            ret = -TARGET_EINVAL;
>> +            break;
>>          default:
>>              /* Most prctl options have no pointer arguments */
>>              ret = get_errno(prctl(arg1, arg2, arg3, arg4, arg5));
>>
> 
> I think we should allow PR_GET_SECCOMP, and at least all the modes
> except SECCOMP_MODE_FILTER for PR_SET_SECCOMP.

I tried allowing SECCOMP_MODE_STRICT and made a small test program. It
seems that qemu is SIGKILLed when _exit is used because qemu itself
tries to call rt_sigprocmask which is not an allowed syscall.

PR_GET_SECCOMP can probably be allowed. My reasoning for disabling it
was to match the kernel behavior for when seccomp is disabled by the
Kconfig option.

Thanks,
James

Re: [Qemu-devel] [PATCH] linux-user: return EINVAL from prctl(PR_*_SECCOMP)

[Peter Maydell]

On 3 November 2017 at 15:52, James Cowgill <James.Cowgill@mips.com> wrote:
> On 03/11/17 14:29, Laurent Vivier wrote:
>> I think we should allow PR_GET_SECCOMP, and at least all the modes
>> except SECCOMP_MODE_FILTER for PR_SET_SECCOMP.
>
> I tried allowing SECCOMP_MODE_STRICT and made a small test program. It
> seems that qemu is SIGKILLed when _exit is used because qemu itself
> tries to call rt_sigprocmask which is not an allowed syscall.
>
> PR_GET_SECCOMP can probably be allowed. My reasoning for disabling it
> was to match the kernel behavior for when seccomp is disabled by the
> Kconfig option.

I think I like "behave like a kernel with seccomp disabled" --
it's a clean line that we can assume won't confuse guest programs.
Laurent?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] linux-user: return EINVAL from prctl(PR_*_SECCOMP)

[Laurent Vivier]

Le 03/11/2017 à 17:01, Peter Maydell a écrit :
> On 3 November 2017 at 15:52, James Cowgill <James.Cowgill@mips.com> wrote:
>> On 03/11/17 14:29, Laurent Vivier wrote:
>>> I think we should allow PR_GET_SECCOMP, and at least all the modes
>>> except SECCOMP_MODE_FILTER for PR_SET_SECCOMP.
>>
>> I tried allowing SECCOMP_MODE_STRICT and made a small test program. It
>> seems that qemu is SIGKILLed when _exit is used because qemu itself
>> tries to call rt_sigprocmask which is not an allowed syscall.
>>
>> PR_GET_SECCOMP can probably be allowed. My reasoning for disabling it
>> was to match the kernel behavior for when seccomp is disabled by the
>> Kconfig option.
> 
> I think I like "behave like a kernel with seccomp disabled" --
> it's a clean line that we can assume won't confuse guest programs.
> Laurent?

I agree. I didn't think about all the syscalls QEMU can do aside of the
executed program. So your solution is the good one.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Thanks,
Laurent

[Qemu-devel] [PATCH v2] linux-user: return EINVAL from prctl(PR_*_SECCOMP)

[James Cowgill]

If an application tries to install a seccomp filter using
prctl(PR_SET_SECCOMP), the filter is likely for the target instead of the host
architecture. This will probably cause qemu to be immediately killed when it
executes another syscall.

Prevent this from happening by returning EINVAL from both seccomp prctl
calls. This is the error returned by the kernel when seccomp support is
disabled.

Fixes: https://bugs.launchpad.net/qemu/+bug/1726394
Signed-off-by: James Cowgill <james.cowgill@mips.com>
---
Changes from v1:
- add comment

 linux-user/syscall.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d4497dec5d..419991e834 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10482,6 +10482,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             break;
         }
 #endif
+        case PR_GET_SECCOMP:
+        case PR_SET_SECCOMP:
+            /* Disable seccomp to prevent the target disabling syscalls we
+             * need. */
+            ret = -TARGET_EINVAL;
+            break;
         default:
             /* Most prctl options have no pointer arguments */
             ret = get_errno(prctl(arg1, arg2, arg3, arg4, arg5));
-- 
2.15.0

Re: [Qemu-devel] [PATCH v2] linux-user: return EINVAL from prctl(PR_*_SECCOMP)

[Laurent Vivier]

Le 06/11/2017 à 19:03, James Cowgill a écrit :
> If an application tries to install a seccomp filter using
> prctl(PR_SET_SECCOMP), the filter is likely for the target instead of the host
> architecture. This will probably cause qemu to be immediately killed when it
> executes another syscall.
> 
> Prevent this from happening by returning EINVAL from both seccomp prctl
> calls. This is the error returned by the kernel when seccomp support is
> disabled.
> 
> Fixes: https://bugs.launchpad.net/qemu/+bug/1726394
> Signed-off-by: James Cowgill <james.cowgill@mips.com>
> ---
> Changes from v1:
> - add comment
> 
>  linux-user/syscall.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index d4497dec5d..419991e834 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -10482,6 +10482,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              break;
>          }
>  #endif
> +        case PR_GET_SECCOMP:
> +        case PR_SET_SECCOMP:
> +            /* Disable seccomp to prevent the target disabling syscalls we
> +             * need. */
> +            ret = -TARGET_EINVAL;
> +            break;
>          default:
>              /* Most prctl options have no pointer arguments */
>              ret = get_errno(prctl(arg1, arg2, arg3, arg4, arg5));
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [Qemu-devel] [PATCH v2] linux-user: return EINVAL from prctl(PR_*_SECCOMP)

[Riku Voipio]

On Mon, Nov 06, 2017 at 06:03:51PM +0000, James Cowgill wrote:
> If an application tries to install a seccomp filter using
> prctl(PR_SET_SECCOMP), the filter is likely for the target instead of the host
> architecture. This will probably cause qemu to be immediately killed when it
> executes another syscall.
> 
> Prevent this from happening by returning EINVAL from both seccomp prctl
> calls. This is the error returned by the kernel when seccomp support is
> disabled.

Thanks, applied to linux-user
 
> Fixes: https://bugs.launchpad.net/qemu/+bug/1726394
> Signed-off-by: James Cowgill <james.cowgill@mips.com>
> ---
> Changes from v1:
> - add comment
> 
>  linux-user/syscall.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index d4497dec5d..419991e834 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -10482,6 +10482,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              break;
>          }
>  #endif
> +        case PR_GET_SECCOMP:
> +        case PR_SET_SECCOMP:
> +            /* Disable seccomp to prevent the target disabling syscalls we
> +             * need. */
> +            ret = -TARGET_EINVAL;
> +            break;
>          default:
>              /* Most prctl options have no pointer arguments */
>              ret = get_errno(prctl(arg1, arg2, arg3, arg4, arg5));
> -- 
> 2.15.0
> 
>