Re: [PATCH 3.5 2/2] seccomp: Future-proof against silly tracers

From: Will Drewry
Date: Tue Jul 17 2012 - 22:13:10 EST


On Tue, Jul 17, 2012 at 6:19 PM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
> Currently, if a tracer changes a syscall nr to __NR_future_enosys,
> behavior will differ between kernels that know about
> __NR_future_enosys (and return -ENOSYS) and older kernels (which
> return the value from pt_regs). This is silly; we should just
> return -ENOSYS.
>
> This is unlikely to ever happen on x86 because the return value in
> pt_regs starts out as -ENOSYS, but a silly tracer can change that.
>
> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
> Cc: Will Drewry <wad@xxxxxxxxxxxx>
> ---
> arch/x86/include/asm/syscall.h | 11 +++++++++++
> kernel/seccomp.c | 15 +++++++++++++++
> 2 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h
> index 1ace47b..8191e057 100644
> --- a/arch/x86/include/asm/syscall.h
> +++ b/arch/x86/include/asm/syscall.h

Since this is used in an arch-wide location, the prototype and comment
should be in asm-generic/syscall.h too.

> @@ -70,6 +70,17 @@ static inline void syscall_set_return_value(struct task_struct *task,
> regs->ax = (long) error ?: val;
> }
>
> +static inline bool syscall_is_nr_future(struct task_struct *task,
> + struct pt_regs *regs)
> +{
> +#ifdef CONFIG_IA32_EMULATION
> + if (task_thread_info(task)->status & TS_COMPAT)
> + return syscall_get_nr(task, regs) > __NR_ia32_syscall_max;
> +#endif
> +
> + return syscall_get_nr(task, regs) > __NR_syscall_max;

I'm not sure how easy this will be to implement on some of the arches
where this data isn't bubbled up. It'd be good if some non-x86 arch
maintainers chimed in (since x86 is easy and already works as expected
:).

> +}
> +
> #ifdef CONFIG_X86_32
>
> static inline void syscall_get_arguments(struct task_struct *task,
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 5af44b5..bd7527d 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -429,6 +429,21 @@ int __secure_computing(int this_syscall)
> */
> if (fatal_signal_pending(current))
> break;
> +
> + if (syscall_is_nr_future(current, regs)) {
> + /*
> + * If the tracer selects a system call that
> + * only future kernels know about, we still need
> + * to execute that syscall by returning
> + * -ENOSYS. (On x86, this only matters if the
> + * tracer changed the return value, which would
> + * be silly, but user code can be silly.)
> + */
> + syscall_set_return_value(current, regs,
> + -ENOSYS, 0);
> + goto skip;
> + }
> +
> if (syscall_get_nr(current, regs) < 0)
> goto skip; /* Explicit request to skip. */
>
> --
> 1.7.7.6
>

thanks!
will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/