mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Chris Metcalf <cmetcalf@mellanox.com>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Christoph Lameter <cl@linux.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Ingo Molnar <mingo@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>, Tejun Heo <tj@kernel.org>,
	Gilad Ben Yossef <giladb@ezchip.com>,
	Will Deacon <will.deacon@arm.com>, Rik van Riel <riel@redhat.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	X86 ML <x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH v10 09/12] arch/x86: enable task isolation functionality
Date: Mon, 7 Mar 2016 15:51:21 -0500	[thread overview]
Message-ID: <56DDE9C9.5060900@mellanox.com> (raw)
In-Reply-To: <CALCETrUrc_LJyLJLHefSDYagCrNqqzKuknr6uLgVXnPW8PmZKw@mail.gmail.com>

On 03/03/2016 06:46 PM, Andy Lutomirski wrote:
> On Thu, Mar 3, 2016 at 11:52 AM, Chris Metcalf <cmetcalf@mellanox.com> wrote:
>> On 03/02/2016 07:36 PM, Andy Lutomirski wrote:
>>> On Mar 2, 2016 12:10 PM, "Chris Metcalf" <cmetcalf@ezchip.com> wrote:
>>>> In prepare_exit_to_usermode(), call task_isolation_ready()
>>>> when we are checking the thread-info flags, and after we've handled
>>>> the other work, call task_isolation_enter() unconditionally.
>>>>
>>>> In syscall_trace_enter_phase1(), we add the necessary support for
>>>> strict-mode detection of syscalls.
>>>> [...]
>>>> @@ -91,6 +92,10 @@ unsigned long syscall_trace_enter_phase1(struct
>>>> pt_regs *regs, u32 arch)
>>>>            */
>>>>           if (work & _TIF_NOHZ) {
>>>>                   enter_from_user_mode();
>>>> +               if (task_isolation_check_syscall(regs->orig_ax)) {
>>>> +                       regs->orig_ax = -1;
>>>> +                       return 0;
>>>> +               }
>>> This needs a comment indicating the intended semantics.
>>> And I've still heard no explanation of why this part can't use seccomp.
>>
>> Here's an excerpt from my earlier reply to you from:
>>
>>    https://lkml.kernel.org/r/55AE9EAC.4010202@ezchip.com
>>
>> Admittedly this patch series has been moving very slowly through
>> review, so it's not surprising we have to revisit some things!
>>
>> On 07/21/2015 03:34 PM, Chris Metcalf wrote:
>>> On 07/13/2015 05:47 PM, Andy Lutomirski wrote:
>>>> If a user wants a syscall to kill them, use
>>>> seccomp.  The kernel isn't at fault if the user does a syscall when it
>>>> didn't want to enter the kernel.
>>>
>>> Interesting!  I didn't realize how close SECCOMP_SET_MODE_STRICT
>>> was to what I wanted here.  One concern is that there doesn't seem
>>> to be a way to "escape" from seccomp strict mode, i.e. you can't
>>> call seccomp() again to turn it off - which makes sense for seccomp
>>> since it's a security issue, but not so much sense with cpu_isolated.
>>>
>>> So, do you think there's a good role for the seccomp() API to play
>>> in achieving this goal?  It's certainly not a question of "the kernel at
>>> fault" but rather "asking the kernel to help catch user mistakes"
>>> (typically third-party libraries in our customers' experience).  You
>>> could imagine a SECCOMP_SET_MODE_ISOLATED or something.
>>>
>>> Alternatively, we could stick with the API proposed in my patch
>>> series, or something similar, and just try to piggy-back on the seccomp
>>> internals to make it happen.  It would require Kconfig to ensure
>>> that SECCOMP was enabled though, which obviously isn't currently
>>> required to do cpu isolation.
>>
>> On looking at this again just now, one thing that strikes me is that
>> it may not be necessary to forbid the syscall like seccomp does.
>> It may be sufficient just to trigger the task isolation strict signal
>> and then allow the syscall to complete.  After all, we don't "fail"
>> any of the other things that upset strict mode, like page faults; we
>> let them complete, but add a signal.  So for consistency, I think it
>> may in fact make sense to simply trigger the signal but let the
>> syscall do its thing.  After all, perhaps the signal is handled
>> and logged and we don't mind having the application continue; the
>> signal handler can certainly choose to fail hard, or in the usual
>> case of no signal handler, that kills the task just fine too.
>> Allowing the syscall to complete is really kind of incidental.
> No, don't do that.  First, if you have a signal pending, a lot of
> syscalls will abort with -EINTR.  Second, if you fire a signal on
> entry via sigreturn, you're not going to like the results.

OK, you've convinced me to stick with the previous model of just
forbidding the syscall in this case.

> Let task isolation users who want to detect when they screw up and do
> a syscall do it with seccomp.

Can you give me more details on what you're imagining here?  Remember
that a key use case is that these applications can remove the syscall
prohibition voluntarily; it's only there to prevent unintended uses
(by third party libraries or just straight-up programming bugs).
As far as I can tell, seccomp does not allow you to go from "less
permissive" to "more permissive" settings at all, which means that as
it exists, it's not a good solution for this use case.

Or were you thinking about a new seccomp API that allows this?

Or were you thinking that I could just use seccomp internals, i.e.
allow the prctl() to set a special SECCOMP_MODE_TASK_ISOLATION
and handle it appropriately in seccomp_phase1(), maybe?  But, not
touch the actual seccomp() API?

I'm happy to spec something out, but I'd definitely benefit from some
sense from you as to what you think is the better approach.

-- 
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

  reply	other threads:[~2016-03-07 20:51 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-02 20:09 [PATCH v10 00/12] support "task_isolation" mode Chris Metcalf
2016-03-02 20:09 ` [PATCH v10 01/12] vmstat: add quiet_vmstat_sync function Chris Metcalf
2016-03-02 20:09 ` [PATCH v10 02/12] vmstat: add vmstat_idle function Chris Metcalf
2016-03-02 20:09 ` [PATCH v10 03/12] lru_add_drain_all: factor out lru_add_drain_needed Chris Metcalf
2016-03-02 20:09 ` [PATCH v10 04/12] task_isolation: add initial support Chris Metcalf
2016-03-02 20:09 ` [PATCH v10 05/12] task_isolation: support CONFIG_TASK_ISOLATION_ALL Chris Metcalf
2016-03-03 18:34   ` Andi Kleen
2016-03-03 19:40     ` Chris Metcalf
2016-03-03 20:04       ` Andi Kleen
2016-03-05 12:31       ` Ingo Molnar
2016-03-02 20:09 ` [PATCH v10 06/12] task_isolation: support PR_TASK_ISOLATION_STRICT mode Chris Metcalf
2016-03-02 20:09 ` [PATCH v10 07/12] task_isolation: add debug boot flag Chris Metcalf
2016-03-02 20:37   ` Peter Zijlstra
2016-03-02 20:56     ` Chris Metcalf
2016-03-02 20:09 ` [PATCH v10 08/12] arm, tile: turn off timer tick for oneshot_stopped state Chris Metcalf
2016-03-02 20:09 ` [PATCH v10 09/12] arch/x86: enable task isolation functionality Chris Metcalf
2016-03-03  0:36   ` Andy Lutomirski
2016-03-03 19:52     ` Chris Metcalf
2016-03-03 23:46       ` Andy Lutomirski
2016-03-07 20:51         ` Chris Metcalf [this message]
2016-03-07 20:55           ` Andy Lutomirski
2016-03-08 20:40             ` Chris Metcalf
2016-03-09 20:58               ` Andy Lutomirski
2016-03-09 21:05                 ` Chris Metcalf
2016-03-09 21:07                   ` Andy Lutomirski
2016-03-09 21:13                     ` Chris Metcalf
2016-03-09 21:10                 ` Kees Cook
2016-03-09 21:18                   ` Andy Lutomirski
2016-03-09 21:25                     ` Kees Cook
2016-03-09 21:57                       ` Andy Lutomirski
2016-03-02 20:09 ` [PATCH v10 10/12] arch/tile: " Chris Metcalf
2016-03-02 20:09 ` [PATCH v10 11/12] arm64: factor work_pending state machine to C Chris Metcalf
2016-03-04 16:38   ` Will Deacon
2016-03-04 20:02     ` Chris Metcalf
2016-03-14 10:29       ` Mark Rutland
2016-03-02 20:09 ` [PATCH v10 12/12] arch/arm64: enable task isolation functionality Chris Metcalf

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=56DDE9C9.5060900@mellanox.com \
    --to=cmetcalf@mellanox.com \
    --cc=akpm@linux-foundation.org \
    --cc=catalin.marinas@arm.com \
    --cc=cl@linux.com \
    --cc=fweisbec@gmail.com \
    --cc=giladb@ezchip.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=viresh.kumar@linaro.org \
    --cc=will.deacon@arm.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome