All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <christian.brauner@ubuntu.com>
To: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>,
	LKML <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Kees Cook <keescook@chromium.org>, Will Drewry <wad@chromium.org>,
	Oleg Nesterov <oleg@redhat.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Parisc List <linux-parisc@vger.kernel.org>,
	linux-s390 <linux-s390@vger.kernel.org>,
	linux-um@lists.infradead.org, X86 ML <x86@kernel.org>
Subject: Re: [PATCH] seccomp: remove unused arg from secure_computing()
Date: Tue, 24 Sep 2019 08:30:36 +0200	[thread overview]
Message-ID: <20190924063035.n3dmryhn6cb52ida@wittgenstein> (raw)
In-Reply-To: <CALCETrU_fs_At-hTpr231kpaAd0z7xJN4ku-DvzhRU6cvcJA_w@mail.gmail.com>

On Mon, Sep 23, 2019 at 11:41:59AM -0700, Andy Lutomirski wrote:
> On Mon, Sep 23, 2019 at 2:49 AM Borislav Petkov <bp@alien8.de> wrote:
> >
> > On Fri, Sep 20, 2019 at 03:19:09PM +0200, Christian Brauner wrote:
> > > While touching seccomp code I realized that the struct seccomp_data
> > > argument to secure_computing() seems to be unused by all current
> > > callers. So let's remove it unless there is some subtlety I missed.
> > > Note, I only tested this on x86.
> >
> > What was amluto thinking in
> >
> > 2f275de5d1ed ("seccomp: Add a seccomp_data parameter secure_computing()")
> 
> IIRC there was a period of time in which x86 used secure_computing()
> for normal syscalls, and it was a good deal faster to have the arch
> code supply seccomp_data.  x86 no longer works like this, and syscalls
> aren't fast anymore ayway :(

I started looking at this and actually had a slightly bigger cleanup in
mind. It seems odd that we have secure_computing() and
__secure_computing(). Especially in the mips and x86 case. From what I
can tell they could both rely on secure_computing() and don't need
__secure_computing().
If I can make those changes, we can make __secure_computing() static and
have only a single function secure_computing() that is used by all
arches which would make this code simpler.
Apparenly mips once switched from secure_computing() to
__secure_computing() because of bpf and tracepoints. The last change to
this was:

commit 3d729deaf287c43e415c5d791c9ac8414dbeff70
Author: James Hogan <jhogan@kernel.org>
Date:   Fri Aug 11 21:56:50 2017 +0100

    MIPS: seccomp: Fix indirect syscall args

which references a broken samples/bpf/tracex5 test. But in the thread to
this last change Kees and others were less than sure that this makes
sense. So I'm not sure. Maybe I should just try and send it out...

Christian

WARNING: multiple messages have this Message-ID (diff)
From: Christian Brauner <christian.brauner@ubuntu.com>
To: Andy Lutomirski <luto@kernel.org>
Cc: linux-s390 <linux-s390@vger.kernel.org>,
	Will Drewry <wad@chromium.org>, Kees Cook <keescook@chromium.org>,
	Parisc List <linux-parisc@vger.kernel.org>,
	X86 ML <x86@kernel.org>,
	linux-um@lists.infradead.org, LKML <linux-kernel@vger.kernel.org>,
	Oleg Nesterov <oleg@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] seccomp: remove unused arg from secure_computing()
Date: Tue, 24 Sep 2019 08:30:36 +0200	[thread overview]
Message-ID: <20190924063035.n3dmryhn6cb52ida@wittgenstein> (raw)
In-Reply-To: <CALCETrU_fs_At-hTpr231kpaAd0z7xJN4ku-DvzhRU6cvcJA_w@mail.gmail.com>

On Mon, Sep 23, 2019 at 11:41:59AM -0700, Andy Lutomirski wrote:
> On Mon, Sep 23, 2019 at 2:49 AM Borislav Petkov <bp@alien8.de> wrote:
> >
> > On Fri, Sep 20, 2019 at 03:19:09PM +0200, Christian Brauner wrote:
> > > While touching seccomp code I realized that the struct seccomp_data
> > > argument to secure_computing() seems to be unused by all current
> > > callers. So let's remove it unless there is some subtlety I missed.
> > > Note, I only tested this on x86.
> >
> > What was amluto thinking in
> >
> > 2f275de5d1ed ("seccomp: Add a seccomp_data parameter secure_computing()")
> 
> IIRC there was a period of time in which x86 used secure_computing()
> for normal syscalls, and it was a good deal faster to have the arch
> code supply seccomp_data.  x86 no longer works like this, and syscalls
> aren't fast anymore ayway :(

I started looking at this and actually had a slightly bigger cleanup in
mind. It seems odd that we have secure_computing() and
__secure_computing(). Especially in the mips and x86 case. From what I
can tell they could both rely on secure_computing() and don't need
__secure_computing().
If I can make those changes, we can make __secure_computing() static and
have only a single function secure_computing() that is used by all
arches which would make this code simpler.
Apparenly mips once switched from secure_computing() to
__secure_computing() because of bpf and tracepoints. The last change to
this was:

commit 3d729deaf287c43e415c5d791c9ac8414dbeff70
Author: James Hogan <jhogan@kernel.org>
Date:   Fri Aug 11 21:56:50 2017 +0100

    MIPS: seccomp: Fix indirect syscall args

which references a broken samples/bpf/tracex5 test. But in the thread to
this last change Kees and others were less than sure that this makes
sense. So I'm not sure. Maybe I should just try and send it out...

Christian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2019-09-24  6:30 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-20 13:19 [PATCH] seccomp: remove unused arg from secure_computing() Christian Brauner
2019-09-20 13:19 ` Christian Brauner
2019-09-23  9:49 ` Borislav Petkov
2019-09-23  9:49   ` Borislav Petkov
2019-09-23 18:41   ` Andy Lutomirski
2019-09-23 18:41     ` Andy Lutomirski
2019-09-23 19:34     ` Borislav Petkov
2019-09-23 19:34       ` Borislav Petkov
2019-09-23 23:51       ` Kees Cook
2019-09-23 23:51         ` Kees Cook
2019-09-24  6:19       ` Christian Brauner
2019-09-24  6:19         ` Christian Brauner
2019-09-24  6:30     ` Christian Brauner [this message]
2019-09-24  6:30       ` Christian Brauner
2019-09-24  6:44 ` [PATCH v1] seccomp: simplify secure_computing() Christian Brauner
2019-09-24  6:44   ` Christian Brauner
2019-09-24  9:51   ` Borislav Petkov
2019-09-24  9:51     ` Borislav Petkov
2019-09-24 17:11   ` Andy Lutomirski
2019-09-24 17:11     ` Andy Lutomirski
2019-10-10 21:53   ` Kees Cook
2019-10-10 21:53     ` Kees Cook
2019-10-11  9:45     ` Christian Brauner
2019-10-11  9:45       ` Christian Brauner

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=20190924063035.n3dmryhn6cb52ida@wittgenstein \
    --to=christian.brauner@ubuntu.com \
    --cc=bp@alien8.de \
    --cc=keescook@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-um@lists.infradead.org \
    --cc=luto@kernel.org \
    --cc=oleg@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=wad@chromium.org \
    --cc=x86@kernel.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.