Re: linux-next: manual merge of the tip tree with the audit tree

2014-09-27 Thread Stephen Rothwell
Hi Andy,

On Fri, 26 Sep 2014 11:02:27 -0700 Andy Lutomirski  wrote:
>
> I don't think that more cleanup is possible after all.
> do_audit_syscall_entry may not need to pass the arch parameter to the
> audit code, but it still needs it to choose the set of registers to
> use.

Yes, indeed, I did not look clearly enough :-(

-- 
Cheers,
Stephen Rothwells...@canb.auug.org.au


signature.asc
Description: PGP signature


Re: linux-next: manual merge of the tip tree with the audit tree

2014-09-26 Thread Andy Lutomirski
On Tue, Sep 23, 2014 at 10:47 PM, Stephen Rothwell  
wrote:
> Hi all,
>
> Today's linux-next merge of the tip tree got a conflict in
> arch/x86/kernel/ptrace.c between commit 91397401bb50 ("ARCH: AUDIT:
> audit_syscall_entry() should not require the arch") from the audit tree
> and commit e0ffbaabc46d ("x86: Split syscall_trace_enter into two
> phases") from the tip tree.
>
> I fixed it up (see below - there is more cleanup possible since
> do_audit_syscall_entry() no longer needs its "arch" argument) and can
> carry the fix as necessary (no action is required).

I don't think that more cleanup is possible after all.
do_audit_syscall_entry may not need to pass the arch parameter to the
audit code, but it still needs it to choose the set of registers to
use.

--Andy

>
> --
> Cheers,
> Stephen Rothwells...@canb.auug.org.au
>
> diff --cc arch/x86/kernel/ptrace.c
> index eb1c87f0b03b,29576c244699..
> --- a/arch/x86/kernel/ptrace.c
> +++ b/arch/x86/kernel/ptrace.c
> @@@ -1441,24 -1441,126 +1441,126 @@@ void send_sigtrap(struct task_struct *t
> force_sig_info(SIGTRAP, &info, tsk);
>   }
>
> -
> - #ifdef CONFIG_X86_32
> - # define IS_IA32  1
> - #elif defined CONFIG_IA32_EMULATION
> - # define IS_IA32  is_compat_task()
> - #else
> - # define IS_IA32  0
> + static void do_audit_syscall_entry(struct pt_regs *regs, u32 arch)
> + {
> + #ifdef CONFIG_X86_64
> +   if (arch == AUDIT_ARCH_X86_64) {
>  -  audit_syscall_entry(arch, regs->orig_ax, regs->di,
> ++  audit_syscall_entry(regs->orig_ax, regs->di,
> +   regs->si, regs->dx, regs->r10);
> +   } else
>   #endif
> +   {
>  -  audit_syscall_entry(arch, regs->orig_ax, regs->bx,
> ++  audit_syscall_entry(regs->orig_ax, regs->bx,
> +   regs->cx, regs->dx, regs->si);
> +   }
> + }
>
>   /*
> -  * We must return the syscall number to actually look up in the table.
> -  * This can be -1L to skip running any syscall at all.
> +  * We can return 0 to resume the syscall or anything else to go to phase
> +  * 2.  If we resume the syscall, we need to put something appropriate in
> +  * regs->orig_ax.
> +  *
> +  * NB: We don't have full pt_regs here, but regs->orig_ax and regs->ax
> +  * are fully functional.
> +  *
> +  * For phase 2's benefit, our return value is:
> +  * 0: resume the syscall
> +  * 1: go to phase 2; no seccomp phase 2 needed
> +  * anything else: go to phase 2; pass return value to seccomp
>*/
> - long syscall_trace_enter(struct pt_regs *regs)
> + unsigned long syscall_trace_enter_phase1(struct pt_regs *regs, u32 arch)
> + {
> +   unsigned long ret = 0;
> +   u32 work;
> +
> +   BUG_ON(regs != task_pt_regs(current));
> +
> +   work = ACCESS_ONCE(current_thread_info()->flags) &
> +   _TIF_WORK_SYSCALL_ENTRY;
> +
> +   /*
> +* If TIF_NOHZ is set, we are required to call user_exit() before
> +* doing anything that could touch RCU.
> +*/
> +   if (work & _TIF_NOHZ) {
> +   user_exit();
> +   work &= ~TIF_NOHZ;
> +   }
> +
> + #ifdef CONFIG_SECCOMP
> +   /*
> +* Do seccomp first -- it should minimize exposure of other
> +* code, and keeping seccomp fast is probably more valuable
> +* than the rest of this.
> +*/
> +   if (work & _TIF_SECCOMP) {
> +   struct seccomp_data sd;
> +
> +   sd.arch = arch;
> +   sd.nr = regs->orig_ax;
> +   sd.instruction_pointer = regs->ip;
> + #ifdef CONFIG_X86_64
> +   if (arch == AUDIT_ARCH_X86_64) {
> +   sd.args[0] = regs->di;
> +   sd.args[1] = regs->si;
> +   sd.args[2] = regs->dx;
> +   sd.args[3] = regs->r10;
> +   sd.args[4] = regs->r8;
> +   sd.args[5] = regs->r9;
> +   } else
> + #endif
> +   {
> +   sd.args[0] = regs->bx;
> +   sd.args[1] = regs->cx;
> +   sd.args[2] = regs->dx;
> +   sd.args[3] = regs->si;
> +   sd.args[4] = regs->di;
> +   sd.args[5] = regs->bp;
> +   }
> +
> +   BUILD_BUG_ON(SECCOMP_PHASE1_OK != 0);
> +   BUILD_BUG_ON(SECCOMP_PHASE1_SKIP != 1);
> +
> +   ret = seccomp_phase1(&sd);
> +   if (ret == SECCOMP_PHASE1_SKIP) {
> +   regs->orig_ax = -1;
> +   ret = 0;
> +   } else if (ret != SECCOMP_PHASE1_OK) {
> +   return ret;  /* Go directly to phase 2 */
> +   }
> +
> +   work &= ~_TIF_SECCOMP;
> +   }
> + #endif
> +
> +   /* Do our best to finish without phase 2. */
> +   if (work =

Re: linux-next: manual merge of the tip tree with the audit tree

2014-09-26 Thread Richard Guy Briggs
On 14/09/24, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the tip tree got a conflict in
> arch/x86/kernel/entry_64.S between commit b4f0d3755c5e ("audit: x86:
> drop arch from __audit_syscall_entry() interface") from the audit tree
> and commit 1dcf74f6edfc ("x86_64, entry: Use split-phase
> syscall_trace_enter for 64-bit syscalls") from the tip tree.
> 
> I fixed it up (probably incorrectly - I removed the auditsys section
> as that is what the latter did) and can carry the fix as necessary (no
> action is required).

The fix looks fine to me.  The auditsys section was no longer needed
since it was taken care of via the phase1 and phase2 calls.  It was the
part in ptrace.c that was important (it would not have compiled) and you
updated that fine to remove the arch arg.

Thanks, Stephen.

> Stephen Rothwells...@canb.auug.org.au



- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
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: linux-next: manual merge of the tip tree with the audit tree

2014-09-24 Thread Andy Lutomirski
On Wed, Sep 24, 2014 at 11:12 AM, Andy Lutomirski  wrote:
> On Tue, Sep 23, 2014 at 10:47 PM, Stephen Rothwell  
> wrote:
>> Hi all,
>>
>> Today's linux-next merge of the tip tree got a conflict in
>> arch/x86/kernel/ptrace.c between commit 91397401bb50 ("ARCH: AUDIT:
>> audit_syscall_entry() should not require the arch") from the audit tree
>> and commit e0ffbaabc46d ("x86: Split syscall_trace_enter into two
>> phases") from the tip tree.
>>
>> I fixed it up (see below - there is more cleanup possible since
>> do_audit_syscall_entry() no longer needs its "arch" argument) and can
>> carry the fix as necessary (no action is required).
>
> [mainly for Eric]
>
> This appears to re-introduce the bug fixed in:
>
> commit 81f49a8fd7088cfcb588d182eeede862c0e3303e
> Author: Andy Lutomirski 
> Date:   Fri Sep 5 15:13:52 2014 -0700
>
> x86, x32, audit: Fix x32's AUDIT_ARCH wrt audit
>
> This bug is not currently observable because enabling x32 disables
> syscall auditing.
>
> Eric, do you want to fix this or should I?

Never mind, -ETOOEARLY.  There's no bug.  TS_COMPAT != is_compat_task.

--Andy

>
> --Andy



-- 
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: linux-next: manual merge of the tip tree with the audit tree

2014-09-24 Thread Andy Lutomirski
On Tue, Sep 23, 2014 at 10:47 PM, Stephen Rothwell  
wrote:
> Hi all,
>
> Today's linux-next merge of the tip tree got a conflict in
> arch/x86/kernel/ptrace.c between commit 91397401bb50 ("ARCH: AUDIT:
> audit_syscall_entry() should not require the arch") from the audit tree
> and commit e0ffbaabc46d ("x86: Split syscall_trace_enter into two
> phases") from the tip tree.
>
> I fixed it up (see below - there is more cleanup possible since
> do_audit_syscall_entry() no longer needs its "arch" argument) and can
> carry the fix as necessary (no action is required).

[mainly for Eric]

This appears to re-introduce the bug fixed in:

commit 81f49a8fd7088cfcb588d182eeede862c0e3303e
Author: Andy Lutomirski 
Date:   Fri Sep 5 15:13:52 2014 -0700

x86, x32, audit: Fix x32's AUDIT_ARCH wrt audit

This bug is not currently observable because enabling x32 disables
syscall auditing.

Eric, do you want to fix this or should I?

--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/