1. Patchew
  2. QEMU
  3. View series

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

James Cowgill posted 1 patch 7 years, 12 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171103120703.15749-1-james.cowgill@mips.com
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
There is a newer version of this series
linux-user/syscall.c | 4 ++++
1 file changed, 4 insertions(+)
[Qemu-devel] [PATCH] linux-user: return EINVAL from prctl(PR_*_SECCOMP)
Posted by James Cowgill 7 years, 12 months ago 
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)
Posted by Laurent Vivier 7 years, 12 months ago 
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)
Posted by James Cowgill 7 years, 12 months ago 
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)
Posted by Peter Maydell 7 years, 12 months ago 
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)
Posted by Laurent Vivier 7 years, 12 months ago 
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
Re: [Qemu-devel] [PATCH] linux-user: return EINVAL from prctl(PR_*_SECCOMP)
Posted by Peter Maydell 7 years, 12 months ago 
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
[Qemu-devel] [PATCH v2] linux-user: return EINVAL from prctl(PR_*_SECCOMP)
Posted by James Cowgill 7 years, 11 months ago 
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)
Posted by Laurent Vivier 7 years, 11 months ago 
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)
Posted by Riku Voipio 7 years, 11 months ago 
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
> 
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.