All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Will Drewry <wad@chromium.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>, Indan Zupancic <indan@nul.nu>,
	Roland McGrath <mcgrathr@google.com>,
	Eric Paris <netdev@parisplace.org>,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	kernel-hardening@lists.openwall.com, mingo@redhat.com,
	oleg@redhat.com, peterz@infradead.org, rdunlap@xenotime.net,
	tglx@linutronix.de, luto@mit.edu, eparis@redhat.com,
	serge.hallyn@canonical.com, pmoore@redhat.com,
	akpm@linux-foundation.org, corbet@lwn.net,
	eric.dumazet@gmail.com, markus@chromium.org,
	coreyb@linux.vnet.ibm.com, keescook@chromium.org
Subject: [kernel-hardening] Re: seccomp and ptrace. what is the correct order?
Date: Tue, 22 May 2012 22:07:05 +0100	[thread overview]
Message-ID: <20120522210704.GK11775@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CABqD9haSh=Tof9n2m8PHBaoqac1kcUZq-f5BcjDBv++5APGTCg@mail.gmail.com>

On Tue, May 22, 2012 at 03:48:40PM -0500, Will Drewry wrote:
> On Tue, May 22, 2012 at 3:34 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> > The proposed patch seems to duplicate the functionality in
> > <asm/syscall.h>. ?Those macros also try to do the right thing in the
> > presence of compat.
> 
> That was my first thought too, so I ran a few simple tests.  gcc isn't
> smart enough to not add ~344 bytes of code to get the number and
> arguments for the x86/kernel/ptrace.c case I included (in the
> naive-est of integrations).  But I don't know that it justifies the
> extra patchwork or enforcing shared code across arches.
> 
> Regardless, the syscall entry + trace code can use some attention
> across the architectures. I don't know that
> one-more-layer-of-abstraction is the right answer (rather than just
> fixing the code). The biggest benefit would be having one-true
> syscall_trace_entry slow path. That said, the fast paths will be
> forever divergent so the opportunity for bugs like the ones pointed
> out will still be there.

FWIW, I'd prefer to have all that done inside __audit_syscall_entry(),
with
        context->arch       = syscall_get_arch(current, regs);
        context->major      = syscall_get_nr(current, regs);
	syscall_get_arguments(current, regs, 0, 4, context->argv);
done instead of initializations from arguments we are doing there now.
I seriously doubt that it would lead to worse code than what we currently
have.  If nothing else, we won't be passing that pile of arguments around.

And yes, asm/syscall.h stuff is probably the right approach here.  For
biarch ones syscall_get_arguments() is saner than doing them one by one...

WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Will Drewry <wad@chromium.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>, Indan Zupancic <indan@nul.nu>,
	Roland McGrath <mcgrathr@google.com>,
	Eric Paris <netdev@parisplace.org>,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	kernel-hardening@lists.openwall.com, mingo@redhat.com,
	oleg@redhat.com, peterz@infradead.org, rdunlap@xenotime.net,
	tglx@linutronix.de, luto@mit.edu, eparis@redhat.com,
	serge.hallyn@canonical.com, pmoore@redhat.com,
	akpm@linux-foundation.org, corbet@lwn.net,
	eric.dumazet@gmail.com, markus@chromium.org,
	coreyb@linux.vnet.ibm.com, keescook@chromium.org
Subject: Re: seccomp and ptrace. what is the correct order?
Date: Tue, 22 May 2012 22:07:05 +0100	[thread overview]
Message-ID: <20120522210704.GK11775@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CABqD9haSh=Tof9n2m8PHBaoqac1kcUZq-f5BcjDBv++5APGTCg@mail.gmail.com>

On Tue, May 22, 2012 at 03:48:40PM -0500, Will Drewry wrote:
> On Tue, May 22, 2012 at 3:34 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> > The proposed patch seems to duplicate the functionality in
> > <asm/syscall.h>. ?Those macros also try to do the right thing in the
> > presence of compat.
> 
> That was my first thought too, so I ran a few simple tests.  gcc isn't
> smart enough to not add ~344 bytes of code to get the number and
> arguments for the x86/kernel/ptrace.c case I included (in the
> naive-est of integrations).  But I don't know that it justifies the
> extra patchwork or enforcing shared code across arches.
> 
> Regardless, the syscall entry + trace code can use some attention
> across the architectures. I don't know that
> one-more-layer-of-abstraction is the right answer (rather than just
> fixing the code). The biggest benefit would be having one-true
> syscall_trace_entry slow path. That said, the fast paths will be
> forever divergent so the opportunity for bugs like the ones pointed
> out will still be there.

FWIW, I'd prefer to have all that done inside __audit_syscall_entry(),
with
        context->arch       = syscall_get_arch(current, regs);
        context->major      = syscall_get_nr(current, regs);
	syscall_get_arguments(current, regs, 0, 4, context->argv);
done instead of initializations from arguments we are doing there now.
I seriously doubt that it would lead to worse code than what we currently
have.  If nothing else, we won't be passing that pile of arguments around.

And yes, asm/syscall.h stuff is probably the right approach here.  For
biarch ones syscall_get_arguments() is saner than doing them one by one...

  reply	other threads:[~2012-05-22 21:07 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-21 18:21 [kernel-hardening] seccomp and ptrace. what is the correct order? Eric Paris
2012-05-21 18:21 ` Eric Paris
2012-05-21 18:25 ` [kernel-hardening] " Roland McGrath
2012-05-21 18:25   ` Roland McGrath
2012-05-21 18:40   ` [kernel-hardening] " Andrew Lutomirski
2012-05-21 19:20   ` Indan Zupancic
2012-05-21 19:20     ` Indan Zupancic
2012-05-22 16:23     ` [kernel-hardening] " Will Drewry
2012-05-22 16:23       ` Will Drewry
2012-05-22 16:26       ` [kernel-hardening] " Will Drewry
2012-05-22 16:26         ` Will Drewry
2012-05-22 17:39       ` [kernel-hardening] " Al Viro
2012-05-22 17:39         ` Al Viro
2012-05-22 20:26         ` [kernel-hardening] " Will Drewry
2012-05-22 20:26           ` Will Drewry
2012-05-22 20:34           ` [kernel-hardening] " H. Peter Anvin
2012-05-22 20:34             ` H. Peter Anvin
2012-05-22 20:48             ` [kernel-hardening] " Will Drewry
2012-05-22 20:48               ` Will Drewry
2012-05-22 21:07               ` Al Viro [this message]
2012-05-22 21:07                 ` Al Viro
2012-05-22 21:17                 ` [kernel-hardening] " Roland McGrath
2012-05-22 21:17                   ` Roland McGrath
2012-05-22 21:18                   ` [kernel-hardening] " H. Peter Anvin
2012-05-22 21:18                     ` H. Peter Anvin
2012-05-22 22:20                   ` [kernel-hardening] " Al Viro
2012-05-22 22:20                     ` Al Viro
2012-05-22 21:09               ` [kernel-hardening] " H. Peter Anvin
2012-05-22 21:09                 ` H. Peter Anvin
2012-05-22 21:14                 ` [kernel-hardening] " Will Drewry
2012-05-22 21:14                   ` Will Drewry
2012-05-22 21:37                   ` [kernel-hardening] " H. Peter Anvin
2012-05-22 21:37                     ` H. Peter Anvin
2012-05-24 16:07         ` [kernel-hardening] [RFC PATCH 0/3] move the secure_computing call Will Drewry
2012-05-24 16:07           ` Will Drewry
2012-05-24 16:07           ` [kernel-hardening] [RFC PATCH 1/3] seccomp: Don't allow tracers to abuse RET_TRACE Will Drewry
2012-05-24 16:07             ` Will Drewry
2012-05-24 17:54             ` [kernel-hardening] " Indan Zupancic
2012-05-24 17:54               ` Indan Zupancic
2012-05-24 18:24               ` [kernel-hardening] " Will Drewry
2012-05-24 18:24                 ` Will Drewry
2012-05-24 20:17                 ` [kernel-hardening] " Indan Zupancic
2012-05-24 20:17                   ` Indan Zupancic
2012-05-24 16:08           ` [kernel-hardening] [RFC PATCH 2/3] arch/x86: move secure_computing after ptrace Will Drewry
2012-05-24 16:08             ` Will Drewry
2012-05-24 16:08           ` [kernel-hardening] [RFC PATCH 3/3] arch/*: move secure_computing after trace Will Drewry
2012-05-24 16:08             ` Will Drewry
2012-05-24 16:13           ` [kernel-hardening] Re: [RFC PATCH 0/3] move the secure_computing call H. Peter Anvin
2012-05-24 16:13             ` H. Peter Anvin
2012-05-24 18:07             ` [kernel-hardening] " Roland McGrath
2012-05-24 18:07               ` Roland McGrath
2012-05-24 18:27               ` [kernel-hardening] " Indan Zupancic
2012-05-24 18:27                 ` Indan Zupancic
2012-05-24 18:45                 ` [kernel-hardening] " H. Peter Anvin
2012-05-24 18:45                   ` H. Peter Anvin
2012-05-24 19:39                   ` [kernel-hardening] " Indan Zupancic
2012-05-24 19:39                     ` Indan Zupancic
2012-05-24 22:00           ` [kernel-hardening] " Andrew Morton
2012-05-24 22:00             ` Andrew Morton
2012-05-25  1:55             ` [kernel-hardening] " Will Drewry
2012-05-25  1:55               ` Will Drewry
2012-05-24 23:40           ` [kernel-hardening] " James Morris
2012-05-24 23:40             ` James Morris
2012-05-24 23:43             ` [kernel-hardening] " Andrew Lutomirski
2012-05-24 23:43               ` Andrew Lutomirski
2012-05-24 23:56               ` [kernel-hardening] " H. Peter Anvin
2012-05-24 23:56                 ` H. Peter Anvin
2012-05-25  0:26                 ` [kernel-hardening] " Andrew Lutomirski
2012-05-25  0:26                   ` Andrew Lutomirski
2012-05-25  0:38                   ` [kernel-hardening] " H. Peter Anvin
2012-05-25  0:38                     ` H. Peter Anvin
2012-05-25  0:55                     ` [kernel-hardening] " Andrew Lutomirski
2012-05-25  0:55                       ` Andrew Lutomirski
2012-05-21 18:47 ` [kernel-hardening] Re: seccomp and ptrace. what is the correct order? richard -rw- weinberger
2012-05-21 18:47   ` richard -rw- weinberger
2012-05-21 19:13   ` [kernel-hardening] " H. Peter Anvin
2012-05-21 19:13     ` H. Peter Anvin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120522210704.GK11775@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=coreyb@linux.vnet.ibm.com \
    --cc=eparis@redhat.com \
    --cc=eric.dumazet@gmail.com \
    --cc=hpa@zytor.com \
    --cc=indan@nul.nu \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@mit.edu \
    --cc=markus@chromium.org \
    --cc=mcgrathr@google.com \
    --cc=mingo@redhat.com \
    --cc=netdev@parisplace.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=pmoore@redhat.com \
    --cc=rdunlap@xenotime.net \
    --cc=serge.hallyn@canonical.com \
    --cc=tglx@linutronix.de \
    --cc=wad@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.