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

2012-08-02 Thread Andy Lutomirski
On Thu, Aug 2, 2012 at 7:32 AM, Will Drewry  wrote:
> On Thu, Jul 26, 2012 at 10:41 AM, Andy Lutomirski  wrote:
>> On Tue, Jul 17, 2012 at 4:19 PM, Andy Lutomirski  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 
>>> Cc: Will Drewry 
>>> ---
>>>  arch/x86/include/asm/syscall.h |   11 +++
>>>  kernel/seccomp.c   |   15 +++
>>>  2 files changed, 26 insertions(+), 0 deletions(-)
>>
>> Will, can you pick this, or some version of it, up in your
>> seccomp-for-ARM tree or wherever your development is?
>
> I'm still not sure about this change though the end result is nice.
> Regardless, I'll explore it when I can -- my family has just increased
> in size, so I'm going to be a bit delayed!

I don't think there's any particular rush here.  It's a very minor ABI
change, but I had to write code to explicitly look for it to detect
any difference at all.

--Andy


>
> cheers!
> will



-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-02 Thread Will Drewry
On Thu, Jul 26, 2012 at 10:41 AM, Andy Lutomirski  wrote:
> On Tue, Jul 17, 2012 at 4:19 PM, Andy Lutomirski  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 
>> Cc: Will Drewry 
>> ---
>>  arch/x86/include/asm/syscall.h |   11 +++
>>  kernel/seccomp.c   |   15 +++
>>  2 files changed, 26 insertions(+), 0 deletions(-)
>
> Will, can you pick this, or some version of it, up in your
> seccomp-for-ARM tree or wherever your development is?

I'm still not sure about this change though the end result is nice.
Regardless, I'll explore it when I can -- my family has just increased
in size, so I'm going to be a bit delayed!

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


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

2012-08-02 Thread Will Drewry
On Thu, Jul 26, 2012 at 10:41 AM, Andy Lutomirski l...@amacapital.net wrote:
 On Tue, Jul 17, 2012 at 4:19 PM, Andy Lutomirski l...@amacapital.net 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 l...@amacapital.net
 Cc: Will Drewry w...@chromium.org
 ---
  arch/x86/include/asm/syscall.h |   11 +++
  kernel/seccomp.c   |   15 +++
  2 files changed, 26 insertions(+), 0 deletions(-)

 Will, can you pick this, or some version of it, up in your
 seccomp-for-ARM tree or wherever your development is?

I'm still not sure about this change though the end result is nice.
Regardless, I'll explore it when I can -- my family has just increased
in size, so I'm going to be a bit delayed!

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


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

2012-08-02 Thread Andy Lutomirski
On Thu, Aug 2, 2012 at 7:32 AM, Will Drewry w...@chromium.org wrote:
 On Thu, Jul 26, 2012 at 10:41 AM, Andy Lutomirski l...@amacapital.net wrote:
 On Tue, Jul 17, 2012 at 4:19 PM, Andy Lutomirski l...@amacapital.net 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 l...@amacapital.net
 Cc: Will Drewry w...@chromium.org
 ---
  arch/x86/include/asm/syscall.h |   11 +++
  kernel/seccomp.c   |   15 +++
  2 files changed, 26 insertions(+), 0 deletions(-)

 Will, can you pick this, or some version of it, up in your
 seccomp-for-ARM tree or wherever your development is?

 I'm still not sure about this change though the end result is nice.
 Regardless, I'll explore it when I can -- my family has just increased
 in size, so I'm going to be a bit delayed!

I don't think there's any particular rush here.  It's a very minor ABI
change, but I had to write code to explicitly look for it to detect
any difference at all.

--Andy



 cheers!
 will



-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-07-26 Thread Andy Lutomirski
On Tue, Jul 17, 2012 at 4:19 PM, Andy Lutomirski  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 
> Cc: Will Drewry 
> ---
>  arch/x86/include/asm/syscall.h |   11 +++
>  kernel/seccomp.c   |   15 +++
>  2 files changed, 26 insertions(+), 0 deletions(-)

Will, can you pick this, or some version of it, up in your
seccomp-for-ARM tree or wherever your development is?

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


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

2012-07-26 Thread Andy Lutomirski
On Tue, Jul 17, 2012 at 4:19 PM, Andy Lutomirski l...@amacapital.net 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 l...@amacapital.net
 Cc: Will Drewry w...@chromium.org
 ---
  arch/x86/include/asm/syscall.h |   11 +++
  kernel/seccomp.c   |   15 +++
  2 files changed, 26 insertions(+), 0 deletions(-)

Will, can you pick this, or some version of it, up in your
seccomp-for-ARM tree or wherever your development is?

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


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

2012-07-18 Thread Andy Lutomirski
On Wed, Jul 18, 2012 at 11:35 AM, Will Drewry  wrote:
> On Tue, Jul 17, 2012 at 9:13 PM, Will Drewry  wrote:
>> On Tue, Jul 17, 2012 at 6:19 PM, Andy Lutomirski  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 
>>> Cc: Will Drewry 
>>> ---
>>>  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
>> :).
>>
>
> Since x86 always returns -ENOSYS with an invalid syscall and only x86
> supports seccomp filter in 3.5, I'd propose pushing this off for 3.6+
> to get more feedback from the relevant parties.  Not doing it now
> doesn't expose any users of 3.5 to any sort of changing ABI.

It's (barely) visible.  See VSYS.trace_changenr_high here:

https://github.com/amluto/seccomp

>
> (Otherwise, it seems fine but may make adding new arches slightly more
> onerous, but I suspect ftrace needs this sort of info too as it
> spreads to other arches!)

Are there any arches for which the return value isn't its own
register?  If so, this is necessary.

Agreed, though: let's wait for 3.6.

-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-07-18 Thread Will Drewry
On Tue, Jul 17, 2012 at 9:13 PM, Will Drewry  wrote:
> On Tue, Jul 17, 2012 at 6:19 PM, Andy Lutomirski  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 
>> Cc: Will Drewry 
>> ---
>>  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
> :).
>

Since x86 always returns -ENOSYS with an invalid syscall and only x86
supports seccomp filter in 3.5, I'd propose pushing this off for 3.6+
to get more feedback from the relevant parties.  Not doing it now
doesn't expose any users of 3.5 to any sort of changing ABI.

(Otherwise, it seems fine but may make adding new arches slightly more
onerous, but I suspect ftrace needs this sort of info too as it
spreads to other arches!)

>> +}
>> +
>>  #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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-07-18 Thread Will Drewry
On Tue, Jul 17, 2012 at 9:13 PM, Will Drewry w...@chromium.org wrote:
 On Tue, Jul 17, 2012 at 6:19 PM, Andy Lutomirski l...@amacapital.net 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 l...@amacapital.net
 Cc: Will Drewry w...@chromium.org
 ---
  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
 :).


Since x86 always returns -ENOSYS with an invalid syscall and only x86
supports seccomp filter in 3.5, I'd propose pushing this off for 3.6+
to get more feedback from the relevant parties.  Not doing it now
doesn't expose any users of 3.5 to any sort of changing ABI.

(Otherwise, it seems fine but may make adding new arches slightly more
onerous, but I suspect ftrace needs this sort of info too as it
spreads to other arches!)

 +}
 +
  #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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-07-18 Thread Andy Lutomirski
On Wed, Jul 18, 2012 at 11:35 AM, Will Drewry w...@chromium.org wrote:
 On Tue, Jul 17, 2012 at 9:13 PM, Will Drewry w...@chromium.org wrote:
 On Tue, Jul 17, 2012 at 6:19 PM, Andy Lutomirski l...@amacapital.net 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 l...@amacapital.net
 Cc: Will Drewry w...@chromium.org
 ---
  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
 :).


 Since x86 always returns -ENOSYS with an invalid syscall and only x86
 supports seccomp filter in 3.5, I'd propose pushing this off for 3.6+
 to get more feedback from the relevant parties.  Not doing it now
 doesn't expose any users of 3.5 to any sort of changing ABI.

It's (barely) visible.  See VSYS.trace_changenr_high here:

https://github.com/amluto/seccomp


 (Otherwise, it seems fine but may make adding new arches slightly more
 onerous, but I suspect ftrace needs this sort of info too as it
 spreads to other arches!)

Are there any arches for which the return value isn't its own
register?  If so, this is necessary.

Agreed, though: let's wait for 3.6.

-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-07-17 Thread Will Drewry
On Tue, Jul 17, 2012 at 6:19 PM, Andy Lutomirski  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 
> Cc: Will Drewry 
> ---
>  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-07-17 Thread Will Drewry
On Tue, Jul 17, 2012 at 6:19 PM, Andy Lutomirski l...@amacapital.net 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 l...@amacapital.net
 Cc: Will Drewry w...@chromium.org
 ---
  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/