* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Michael Sammler @ 2018-10-29 21:55 UTC (permalink / raw)
To: Dave Hansen, Jann Horn
Cc: wad, Kees Cook, Linux API, Dave Hansen, linuxram, Andy Lutomirski,
linuxppc-dev
In-Reply-To: <62e09400-0443-8db9-a389-ba4f4201226b@intel.com>
Am 29.10.2018 um 18:29 schrieb Dave Hansen:
> On 10/29/18 9:48 AM, Jann Horn wrote:
>> On Mon, Oct 29, 2018 at 5:37 PM Dave Hansen <dave.hansen@intel.com> wrote:
>>> I'm not sure this is a great use for PKRU. I *think* the basic problem
>>> is that you want to communicate some rights information down into a
>>> filter, and you want to communicate it with PKRU. While it's handy to
>>> have an extra register that nobody (generally) mucks with, I'm not quite
>>> convinced that we want to repurpose it this way.
>> That's not how I understand it; I believe that the context is probably
>> https://arxiv.org/pdf/1801.06822.pdf ?
>> My understanding is that PKRU is used for lightweight in-process
>> sandboxing, and to extend this sandbox protection to the syscall
>> interface, it is necessary to expose PKRU state to seccomp filters.
>> In other words, this isn't using PKRU exclusively for passing rights
>> into a filter, but it has to use PKRU anyway.
> PKRU gives information about rights to various bits of application data.
> From that, a seccomp filter can infer the context, and thus the ability
> for the code to call a given syscall at a certain point in time.
>
> This makes PKRU an opt-in part of the syscall ABI, which is pretty
> interesting. We _could_ do the same kind of thing with any callee-saved
> general purpose register, but PKRU is particularly attractive because
> there is only one instruction that writes to it (well, outside of
> XSAVE*), and random library code is very unlikely at this point to be
> using it.
I agree with you on the points, why PKRU is particularly attractive but
I think the most important point is that PKRU is _not_ a general purpose
register, but is already used to control access to some resource
(memory). This patch would allow to also control access to another
resource (system calls) using the PKRU. This is why it makes sense to
use the PKRU in this patch instead of another callee-saved register.
> PKRU getting reset on signals, and the requirement now that it *can't*
> be changed if you make syscalls probably needs to get thought about very
> carefully before we do this, though.
I am not sure, whether I follow you. Are you saying, that PKRU is
currently not guaranteed to be preserved across system calls?
This would make it very hard to use protection keys if libc does not
save and restore the PKRU before/after systemcalls (and I am not aware
of this).
Or do you mean, that the kernel might want to use the PKRU register for
its own purposes while it is executing?
Then the solution you proposed in another email in this thread would
work: instead of providing the seccomp filter with the current value of
the PKRU (which might be different from what the user space expects) use
the user space value which must have been saved somewhere (otherwise it
would not be possible to restore it).
Or are you afraid, that one part of a user space program installs a
seccomp filter, which blocks system calls based on the PKRU, and another
part of the same program (maybe a library) changes the PKRU in a way,
which the first part did not expect and the program dies because it
tries to do a forbidden system call?
I don't know whether the kernel can (and wants) do anything against
this. This problem also exists without this patch if you replace system
call with memory access.
-- Michael
^ permalink raw reply
* Re: [PATCH v3 0/2] sysctl: handle overflow for file-max
From: Kees Cook @ 2018-10-29 21:44 UTC (permalink / raw)
To: Christian Brauner, Andrew Morton
Cc: LKML, Eric W. Biederman, Luis R. Rodriguez, Joe Lawrence,
Waiman Long, Dominik Brodowski, Al Viro, Alexey Dobriyan,
Linux API
In-Reply-To: <20181029145818.4bqmy25itjnqhodg@brauner.io>
On Mon, Oct 29, 2018 at 7:58 AM, Christian Brauner <christian@brauner.io> wrote:
> On Wed, Oct 17, 2018 at 12:33:20AM +0200, Christian Brauner wrote:
>> Hey,
>>
>> Here is v3 of this patchset. Changelogs are in the individual commits.
>>
>> Currently, when writing
>>
>> echo 18446744073709551616 > /proc/sys/fs/file-max
>>
>> /proc/sys/fs/file-max will overflow and be set to 0. That quickly
>> crashes the system.
>>
>> The first version of this patch intended to detect the overflow and cap
>> at ULONG_MAX. However, we should not do this and rather return EINVAL on
>> overflow. The reasons are:
>> - this aligns with other sysctl handlers that simply reject overflows
>> (cf. [1], [2], and a bunch of others)
>> - we already do a partial fail on overflow right now
>> Namely, when the TMPBUFLEN is exceeded. So we already reject values
>> such as 184467440737095516160 (21 chars) but accept values such as
>> 18446744073709551616 (20 chars) but both are overflows. So we should
>> just always reject 64bit overflows and not special-case this based on
>> the number of chars.
>>
>> (This patchset is in reference to https://lkml.org/lkml/2018/10/11/585.)
>
> Just so that we don't forget, can we make sure that this gets picked
> into linux-next? :)
I was hoping akpm would take this? Andrew, does the v3 look okay to you?
-Kees
--
Kees Cook
^ permalink raw reply
* Re: [RFC 00/20] ns: Introduce Time Namespace
From: Thomas Gleixner @ 2018-10-29 21:36 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andrei Vagin, linux-kselftest@vger.kernel.org, Dmitry Safonov,
linux-api@vger.kernel.org, Jeff Dike, x86@kernel.org,
Dmitry Safonov, linux-kernel@vger.kernel.org, Oleg Nesterov,
criu@openvz.org, Ingo Molnar, Alexey Dobriyan, Andy Lutomirski,
H. Peter Anvin, Cyrill Gorcunov, Christian Brauner,
Pavel Emelianov, Shuah Khan, containers
In-Reply-To: <87y3ag5tze.fsf@xmission.com>
Eric,
On Mon, 29 Oct 2018, Eric W. Biederman wrote:
> Thomas Gleixner <tglx@linutronix.de> writes:
> >
> > I'll try to find some time in the next weeks to look deeper into that, but
> > I can't promise anything before returning from LPC. Btw, LPC would be a
> > great opportunity to discuss that. Are you and the other name space wizards
> > there by any chance?
>
> I will be and there are going to be both container and CRIU
> mini-conferences. So there should at least some of us around.
So let's try to find a slot for a BOF or similar (there might be still
slots for the kernel summit available, i'll ask).
Thanks,
tglx
^ permalink raw reply
* Re: [RFC 00/20] ns: Introduce Time Namespace
From: Eric W. Biederman @ 2018-10-29 21:21 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Andrei Vagin, linux-kselftest@vger.kernel.org, Dmitry Safonov,
linux-api@vger.kernel.org, Jeff Dike, x86@kernel.org,
Dmitry Safonov, linux-kernel@vger.kernel.org, Oleg Nesterov,
criu@openvz.org, Ingo Molnar, Alexey Dobriyan, Andy Lutomirski,
H. Peter Anvin, Cyrill Gorcunov, Christian Brauner,
Pavel Emelianov, Shuah Khan, containers
In-Reply-To: <alpine.DEB.2.21.1810292026040.5984@nanos.tec.linutronix.de>
Thomas Gleixner <tglx@linutronix.de> writes:
> Andrei,
>
> On Sat, 20 Oct 2018, Andrei Vagin wrote:
>> When a container is migrated to another host, we have to restore its
>> monotonic and boottime clocks, but we still expect that the container
>> will continue using the host real-time clock.
>>
>> Before stating this series, I was thinking about this, I decided that
>> these cases can be solved independently. Probably, the full isolation of
>> the time sub-system will have much higher overhead than just offsets for
>> a few clocks. And the idea that isolation of the real-time clock should
>> be optional gives us another hint that offsets for monotonic and
>> boot-time clocks can be implemented independently.
>>
>> Eric and Tomas, what do you think about this? If you agree that these
>> two cases can be implemented separately, what should we do with this
>> series to make it ready to be merged?
>>
>> I know that we need to:
>>
>> * look at device drivers that report timestamps in CLOCK_MONOTONIC base.
>
> and CLOCK_BOOTTIME and that's quite a few.
>
>> * forbid changing offsets after creating timers
>
> There are more things to think about. What about interfaces which expose
> boot time or monotonic time in /proc?
>
> Aside of that (I finally came around to look at the series in more detail)
> I'm really unhappy about the unconditional overhead once the Time namespace
> config switch is enabled. This applies especially to the VDSO. We spent
> quite some time recently to squeeze a few cycles out of those functions and
> it would be a pity to pointlessly waste cycles for the !namespace case.
>
> I can see the urge for this, but please let us think it through properly
> before rushing anything in which we are going to regret once we want to do
> more sophisticated time domain management, e.g. support for isolated clock
> real time. I'm worried, that without a clear plan about the overall
> picture, we end up with duct tape which is hard to distangle after the
> fact.
>
> There have been a few other things brought up versus time management in
> general, like the TSN folks utilizing grand clock masters which expose
> random time instead of proper TAI. Plus some requirements for exposing some
> sort of 'monotonic' clocks which are derived from external synchronization
> mechanisms, but should not affect the regular time keeping clocks.
>
> While different issues, these all fall into the category of separate time
> domains, so taking a step back to the drawing board is probably the best
> thing what we can do now.
>
> There are certainly a few things which can be looked at independently,
> e.g. the VDSO mechanics or general mechanisms to avoid plastering the whole
> kernel with these name space functions applying offsets left and right. I
> rather have dedicated core functionality which replaces/amends existing
> timer functions to become time namespace aware.
>
> I'll try to find some time in the next weeks to look deeper into that, but
> I can't promise anything before returning from LPC. Btw, LPC would be a
> great opportunity to discuss that. Are you and the other name space wizards
> there by any chance?
I will be and there are going to be both container and CRIU
mini-conferences. So there should at least some of us around.
Eric
^ permalink raw reply
* Re: [RFC 00/20] ns: Introduce Time Namespace
From: Thomas Gleixner @ 2018-10-29 20:33 UTC (permalink / raw)
To: Andrei Vagin
Cc: Eric W. Biederman, linux-kselftest@vger.kernel.org,
Dmitry Safonov, linux-api@vger.kernel.org, Jeff Dike,
x86@kernel.org, Dmitry Safonov, linux-kernel@vger.kernel.org,
Oleg Nesterov, criu@openvz.org, Ingo Molnar, Alexey Dobriyan,
Andy Lutomirski, H. Peter Anvin, Cyrill Gorcunov,
Christian Brauner, Pavel Emelianov, Shuah Khan, container
In-Reply-To: <20181021014121.GA23474@gmail.com>
Andrei,
On Sat, 20 Oct 2018, Andrei Vagin wrote:
> When a container is migrated to another host, we have to restore its
> monotonic and boottime clocks, but we still expect that the container
> will continue using the host real-time clock.
>
> Before stating this series, I was thinking about this, I decided that
> these cases can be solved independently. Probably, the full isolation of
> the time sub-system will have much higher overhead than just offsets for
> a few clocks. And the idea that isolation of the real-time clock should
> be optional gives us another hint that offsets for monotonic and
> boot-time clocks can be implemented independently.
>
> Eric and Tomas, what do you think about this? If you agree that these
> two cases can be implemented separately, what should we do with this
> series to make it ready to be merged?
>
> I know that we need to:
>
> * look at device drivers that report timestamps in CLOCK_MONOTONIC base.
and CLOCK_BOOTTIME and that's quite a few.
> * forbid changing offsets after creating timers
There are more things to think about. What about interfaces which expose
boot time or monotonic time in /proc?
Aside of that (I finally came around to look at the series in more detail)
I'm really unhappy about the unconditional overhead once the Time namespace
config switch is enabled. This applies especially to the VDSO. We spent
quite some time recently to squeeze a few cycles out of those functions and
it would be a pity to pointlessly waste cycles for the !namespace case.
I can see the urge for this, but please let us think it through properly
before rushing anything in which we are going to regret once we want to do
more sophisticated time domain management, e.g. support for isolated clock
real time. I'm worried, that without a clear plan about the overall
picture, we end up with duct tape which is hard to distangle after the
fact.
There have been a few other things brought up versus time management in
general, like the TSN folks utilizing grand clock masters which expose
random time instead of proper TAI. Plus some requirements for exposing some
sort of 'monotonic' clocks which are derived from external synchronization
mechanisms, but should not affect the regular time keeping clocks.
While different issues, these all fall into the category of separate time
domains, so taking a step back to the drawing board is probably the best
thing what we can do now.
There are certainly a few things which can be looked at independently,
e.g. the VDSO mechanics or general mechanisms to avoid plastering the whole
kernel with these name space functions applying offsets left and right. I
rather have dedicated core functionality which replaces/amends existing
timer functions to become time namespace aware.
I'll try to find some time in the next weeks to look deeper into that, but
I can't promise anything before returning from LPC. Btw, LPC would be a
great opportunity to discuss that. Are you and the other name space wizards
there by any chance?
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH v5 01/15] sched/core: uclamp: extend sched_setattr to support utilization clamping
From: Randy Dunlap @ 2018-10-29 19:33 UTC (permalink / raw)
To: Patrick Bellasi, linux-kernel, linux-pm
Cc: Ingo Molnar, Peter Zijlstra, Tejun Heo, Rafael J . Wysocki,
Vincent Guittot, Viresh Kumar, Paul Turner, Quentin Perret,
Dietmar Eggemann, Morten Rasmussen, Juri Lelli, Todd Kjos,
Joel Fernandes, Steve Muckle, Suren Baghdasaryan, linux-api
In-Reply-To: <20181029183311.29175-2-patrick.bellasi@arm.com>
On 10/29/18 11:32 AM, Patrick Bellasi wrote:
> diff --git a/include/uapi/linux/sched/types.h b/include/uapi/linux/sched/types.h
> index 10fbb8031930..fde7301ed28c 100644
> --- a/include/uapi/linux/sched/types.h
> +++ b/include/uapi/linux/sched/types.h
> @@ -53,6 +73,30 @@ struct sched_param {
> * As of now, the SCHED_DEADLINE policy (sched_dl scheduling class) is the
> * only user of this new interface. More information about the algorithm
> * available in the scheduling class file or in Documentation/.
> + *
> + * Task Utilization Attributes
> + * ===========================
> + *
> + * A subset of sched_attr attributes allows to specify the utilization which
> + * should be expected by a task. These attributes allow to inform the
> + * scheduler about the utilization boundaries within which it is expected to
> + * schedule the task. These boundaries are valuable hints to support scheduler
> + * decisions on both task placement and frequencies selection.
> + *
> + * @sched_util_min represents the minimum utilization
> + * @sched_util_max represents the maximum utilization
> + *
> + * Utilization is a value in the range [0..SCHED_CAPACITY_SCALE] which
> + * represents the percentage of CPU time used by a task when running at the
> + * maximum frequency on the highest capacity CPU of the system. Thus, for
> + * example, a 20% utilization task is a task running for 2ms every 10ms.
> + *
> + * A task with a min utilization value bigger then 0 is more likely to be
than
> + * scheduled on a CPU which has a capacity big enough to fit the specified
> + * minimum utilization value.
> + * A task with a max utilization value smaller then 1024 is more likely to be
> + * scheduled on a CPU which do not necessarily have more capacity then the
does than
> + * specified max utilization value.
> */
> struct sched_attr {
> __u32 size;
cheers.
--
~Randy
^ permalink raw reply
* [PATCH v5 02/15] sched/core: make sched_setattr able to tune the current policy
From: Patrick Bellasi @ 2018-10-29 18:32 UTC (permalink / raw)
To: linux-kernel, linux-pm
Cc: Ingo Molnar, Peter Zijlstra, Tejun Heo, Rafael J . Wysocki,
Vincent Guittot, Viresh Kumar, Paul Turner, Quentin Perret,
Dietmar Eggemann, Morten Rasmussen, Juri Lelli, Todd Kjos,
Joel Fernandes, Steve Muckle, Suren Baghdasaryan, linux-api
In-Reply-To: <20181029183311.29175-1-patrick.bellasi@arm.com>
Currently, sched_setattr mandates that a policy is always specified.
Since utilization clamp attributes could apply across different
scheduling policies (i.e. all but SCHED_DEADLINE), this requires also
to always know which policy a task has before changing its clamp values.
This is not just cumbersome but it's also racy, indeed we cannot be
sure that a task policy has been changed in between its policy read and
the actual clamp value change. Sometimes however, this could be the
actual use-case, we wanna change the clamps without affecting the
policy.
Let's fix this adding an additional attribute flag
(SCHED_FLAG_TUNE_POLICY) which, when specified, will ensure to always
force the usage of the current policy. This is done by re-using the
SETPARAM_POLICY thing we already have for the sched_setparam syscall,
thus extending its usage to the non-POSIX sched_setattr while not
exposing that internal concept to user-space.
Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Todd Kjos <tkjos@google.com>
Cc: Joel Fernandes <joelaf@google.com>
Cc: Steve Muckle <smuckle@google.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-api@vger.kernel.org
---
Changes in v5:
Message-ID: <20180905110108.GC20267@localhost.localdomain>
- allow to change clamp values without affecting current policy
---
include/uapi/linux/sched.h | 6 +++++-
kernel/sched/core.c | 11 ++++++++++-
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h
index c27d6e81517b..62498d749bec 100644
--- a/include/uapi/linux/sched.h
+++ b/include/uapi/linux/sched.h
@@ -40,6 +40,8 @@
/* SCHED_ISO: reserved but not implemented yet */
#define SCHED_IDLE 5
#define SCHED_DEADLINE 6
+/* Must be the last entry: used check attr.policy values */
+#define SCHED_POLICY_MAX 7
/* Can be ORed in to make sure the process is reverted back to SCHED_NORMAL on fork */
#define SCHED_RESET_ON_FORK 0x40000000
@@ -50,11 +52,13 @@
#define SCHED_FLAG_RESET_ON_FORK 0x01
#define SCHED_FLAG_RECLAIM 0x02
#define SCHED_FLAG_DL_OVERRUN 0x04
-#define SCHED_FLAG_UTIL_CLAMP 0x08
+#define SCHED_FLAG_TUNE_POLICY 0x08
+#define SCHED_FLAG_UTIL_CLAMP 0x10
#define SCHED_FLAG_ALL (SCHED_FLAG_RESET_ON_FORK | \
SCHED_FLAG_RECLAIM | \
SCHED_FLAG_DL_OVERRUN | \
+ SCHED_FLAG_TUNE_POLICY | \
SCHED_FLAG_UTIL_CLAMP)
#endif /* _UAPI_LINUX_SCHED_H */
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3701bb1e6698..9a2e12eaa377 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4595,8 +4595,17 @@ SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr,
if (retval)
return retval;
- if ((int)attr.sched_policy < 0)
+ /*
+ * A valid policy has always to be required from userspace. Unless
+ * the SCHED_FLAG_TUNE_POLICY is set, in which case, the current
+ * policy will be enforced for this call.
+ */
+ if (attr.sched_policy >= SCHED_POLICY_MAX &&
+ !(attr.sched_flags & SCHED_FLAG_TUNE_POLICY)) {
return -EINVAL;
+ }
+ if (attr.sched_flags & SCHED_FLAG_TUNE_POLICY)
+ attr.sched_policy = SETPARAM_POLICY;
rcu_read_lock();
retval = -ESRCH;
--
2.18.0
^ permalink raw reply related
* [PATCH v5 01/15] sched/core: uclamp: extend sched_setattr to support utilization clamping
From: Patrick Bellasi @ 2018-10-29 18:32 UTC (permalink / raw)
To: linux-kernel, linux-pm
Cc: Ingo Molnar, Peter Zijlstra, Tejun Heo, Rafael J . Wysocki,
Vincent Guittot, Viresh Kumar, Paul Turner, Quentin Perret,
Dietmar Eggemann, Morten Rasmussen, Juri Lelli, Todd Kjos,
Joel Fernandes, Steve Muckle, Suren Baghdasaryan, Randy Dunlap,
linux-api
In-Reply-To: <20181029183311.29175-1-patrick.bellasi@arm.com>
The SCHED_DEADLINE scheduling class provides an advanced and formal
model to define tasks requirements which can be translated into proper
decisions for both task placements and frequencies selections.
Other classes have a more simplified model which is essentially based on
the relatively simple concept of POSIX priorities.
Such a simple priority based model however does not allow to exploit
some of the most advanced features of the Linux scheduler like, for
example, driving frequencies selection via the schedutil cpufreq
governor. However, also for non SCHED_DEADLINE tasks, it's still
interesting to define tasks properties which can be used to better
support certain scheduler decisions.
Utilization clamping aims at exposing to user-space a new set of
per-task attributes which can be used to provide the scheduler with some
hints about the expected/required utilization for a task.
This will allow to implement a more advanced per-task frequency control
mechanism which is not based just on a "passive" measured task
utilization but on a more "proactive" approach. For example, it could be
possible to boost interactive tasks, thus getting better performance, or
cap background tasks, thus being more energy/thermal efficient.
Ultimately, such a mechanism can be considered similar to the cpufreq
powersave, performance and userspace governors but with a much fine
grained and per-task control.
Let's introduce a new API to set utilization clamping values for a
specified task by extending sched_setattr, a syscall which already
allows to define task specific properties for different scheduling
classes. Specifically, a new pair of attributes allows to specify a
minimum and maximum utilization which the scheduler should consider for
a task.
Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Paul Turner <pjt@google.com>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Todd Kjos <tkjos@google.com>
Cc: Joel Fernandes <joelaf@google.com>
Cc: Steve Muckle <smuckle@google.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Quentin Perret <quentin.perret@arm.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Morten Rasmussen <morten.rasmussen@arm.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-api@vger.kernel.org
---
Changes in v5:
Others:
- rebased on v4.19
Changes in v4:
Message-ID: <87897157-0b49-a0be-f66c-81cc2942b4dd@infradead.org>
- remove not required default setting
- fixed some tabs/spaces
Message-ID: <20180807095905.GB2288@localhost.localdomain>
- replace/rephrase "bandwidth" references to use "capacity"
- better stress that this do not enforce any bandwidth requirement
but "just" give hints to the scheduler
- fixed some typos
Others:
- add support for SCHED_FLAG_RESET_ON_FORK
default clamps are now set for init_task and inherited/reset at
fork time (when then flag is set for the parent)
- rebased on v4.19-rc1
Changes in v3:
Message-ID: <CAJuCfpF6=L=0LrmNnJrTNPazT4dWKqNv+thhN0dwpKCgUzs9sg@mail.gmail.com>
- removed UCLAMP_NONE not used by this patch
Others:
- rebased on tip/sched/core
Changes in v2:
- rebased on v4.18-rc4
- move at the head of the series
As discussed at OSPM, using a [0..SCHED_CAPACITY_SCALE] range seems to
be acceptable. However, an additional patch has been added at the end of
the series which introduces a simple abstraction to use a more
generic [0..100] range.
At OSPM we also discarded the idea to "recycle" the usage of
sched_runtime and sched_period which would have made the API too
much complex for limited benefits.
---
include/linux/sched.h | 13 +++++++
include/uapi/linux/sched.h | 4 +-
include/uapi/linux/sched/types.h | 67 +++++++++++++++++++++++++++-----
init/Kconfig | 21 ++++++++++
init/init_task.c | 5 +++
kernel/sched/core.c | 39 +++++++++++++++++++
6 files changed, 139 insertions(+), 10 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 977cb57d7bc9..880a0c5c1f87 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -279,6 +279,14 @@ struct vtime {
u64 gtime;
};
+enum uclamp_id {
+ UCLAMP_MIN = 0, /* Minimum utilization */
+ UCLAMP_MAX, /* Maximum utilization */
+
+ /* Utilization clamping constraints count */
+ UCLAMP_CNT
+};
+
struct sched_info {
#ifdef CONFIG_SCHED_INFO
/* Cumulative counters: */
@@ -649,6 +657,11 @@ struct task_struct {
#endif
struct sched_dl_entity dl;
+#ifdef CONFIG_UCLAMP_TASK
+ /* Utlization clamp values for this task */
+ int uclamp[UCLAMP_CNT];
+#endif
+
#ifdef CONFIG_PREEMPT_NOTIFIERS
/* List of struct preempt_notifier: */
struct hlist_head preempt_notifiers;
diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h
index 22627f80063e..c27d6e81517b 100644
--- a/include/uapi/linux/sched.h
+++ b/include/uapi/linux/sched.h
@@ -50,9 +50,11 @@
#define SCHED_FLAG_RESET_ON_FORK 0x01
#define SCHED_FLAG_RECLAIM 0x02
#define SCHED_FLAG_DL_OVERRUN 0x04
+#define SCHED_FLAG_UTIL_CLAMP 0x08
#define SCHED_FLAG_ALL (SCHED_FLAG_RESET_ON_FORK | \
SCHED_FLAG_RECLAIM | \
- SCHED_FLAG_DL_OVERRUN)
+ SCHED_FLAG_DL_OVERRUN | \
+ SCHED_FLAG_UTIL_CLAMP)
#endif /* _UAPI_LINUX_SCHED_H */
diff --git a/include/uapi/linux/sched/types.h b/include/uapi/linux/sched/types.h
index 10fbb8031930..fde7301ed28c 100644
--- a/include/uapi/linux/sched/types.h
+++ b/include/uapi/linux/sched/types.h
@@ -9,6 +9,7 @@ struct sched_param {
};
#define SCHED_ATTR_SIZE_VER0 48 /* sizeof first published struct */
+#define SCHED_ATTR_SIZE_VER1 56 /* add: util_{min,max} */
/*
* Extended scheduling parameters data structure.
@@ -21,8 +22,33 @@ struct sched_param {
* the tasks may be useful for a wide variety of application fields, e.g.,
* multimedia, streaming, automation and control, and many others.
*
- * This variant (sched_attr) is meant at describing a so-called
- * sporadic time-constrained task. In such model a task is specified by:
+ * This variant (sched_attr) allows to define additional attributes to
+ * improve the scheduler knowledge about task requirements.
+ *
+ * Scheduling Class Attributes
+ * ===========================
+ *
+ * A subset of sched_attr attributes specifies the
+ * scheduling policy and relative POSIX attributes:
+ *
+ * @size size of the structure, for fwd/bwd compat.
+ *
+ * @sched_policy task's scheduling policy
+ * @sched_nice task's nice value (SCHED_NORMAL/BATCH)
+ * @sched_priority task's static priority (SCHED_FIFO/RR)
+ *
+ * Certain more advanced scheduling features can be controlled by a
+ * predefined set of flags via the attribute:
+ *
+ * @sched_flags for customizing the scheduler behaviour
+ *
+ * Sporadic Time-Constrained Tasks Attributes
+ * ==========================================
+ *
+ * A subset of sched_attr attributes allows to describe a so-called
+ * sporadic time-constrained task.
+ *
+ * In such model a task is specified by:
* - the activation period or minimum instance inter-arrival time;
* - the maximum (or average, depending on the actual scheduling
* discipline) computation time of all instances, a.k.a. runtime;
@@ -34,14 +60,8 @@ struct sched_param {
* than the runtime and must be completed by time instant t equal to
* the instance activation time + the deadline.
*
- * This is reflected by the actual fields of the sched_attr structure:
+ * This is reflected by the following fields of the sched_attr structure:
*
- * @size size of the structure, for fwd/bwd compat.
- *
- * @sched_policy task's scheduling policy
- * @sched_flags for customizing the scheduler behaviour
- * @sched_nice task's nice value (SCHED_NORMAL/BATCH)
- * @sched_priority task's static priority (SCHED_FIFO/RR)
* @sched_deadline representative of the task's deadline
* @sched_runtime representative of the task's runtime
* @sched_period representative of the task's period
@@ -53,6 +73,30 @@ struct sched_param {
* As of now, the SCHED_DEADLINE policy (sched_dl scheduling class) is the
* only user of this new interface. More information about the algorithm
* available in the scheduling class file or in Documentation/.
+ *
+ * Task Utilization Attributes
+ * ===========================
+ *
+ * A subset of sched_attr attributes allows to specify the utilization which
+ * should be expected by a task. These attributes allow to inform the
+ * scheduler about the utilization boundaries within which it is expected to
+ * schedule the task. These boundaries are valuable hints to support scheduler
+ * decisions on both task placement and frequencies selection.
+ *
+ * @sched_util_min represents the minimum utilization
+ * @sched_util_max represents the maximum utilization
+ *
+ * Utilization is a value in the range [0..SCHED_CAPACITY_SCALE] which
+ * represents the percentage of CPU time used by a task when running at the
+ * maximum frequency on the highest capacity CPU of the system. Thus, for
+ * example, a 20% utilization task is a task running for 2ms every 10ms.
+ *
+ * A task with a min utilization value bigger then 0 is more likely to be
+ * scheduled on a CPU which has a capacity big enough to fit the specified
+ * minimum utilization value.
+ * A task with a max utilization value smaller then 1024 is more likely to be
+ * scheduled on a CPU which do not necessarily have more capacity then the
+ * specified max utilization value.
*/
struct sched_attr {
__u32 size;
@@ -70,6 +114,11 @@ struct sched_attr {
__u64 sched_runtime;
__u64 sched_deadline;
__u64 sched_period;
+
+ /* Utilization hints */
+ __u32 sched_util_min;
+ __u32 sched_util_max;
+
};
#endif /* _UAPI_LINUX_SCHED_TYPES_H */
diff --git a/init/Kconfig b/init/Kconfig
index 1e234e2f1cba..738974c4f628 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -613,6 +613,27 @@ config HAVE_UNSTABLE_SCHED_CLOCK
config GENERIC_SCHED_CLOCK
bool
+menu "Scheduler features"
+
+config UCLAMP_TASK
+ bool "Enable utilization clamping for RT/FAIR tasks"
+ depends on CPU_FREQ_GOV_SCHEDUTIL
+ help
+ This feature enables the scheduler to track the clamped utilization
+ of each CPU based on RUNNABLE tasks currently scheduled on that CPU.
+
+ When this option is enabled, the user can specify a min and max CPU
+ utilization which is allowed for RUNNABLE tasks.
+ The max utilization allows to request a maximum frequency a task should
+ use, while the min utilization allows to request a minimum frequency a
+ task should use.
+ Both min and max utilization clamp values are hints to the scheduler,
+ aiming at improving its frequency selection policy, but they do not
+ enforce or grant any specific bandwidth for tasks.
+
+ If in doubt, say N.
+
+endmenu
#
# For architectures that want to enable the support for NUMA-affine scheduler
# balancing logic:
diff --git a/init/init_task.c b/init/init_task.c
index 5aebe3be4d7c..5bfdcc3fb839 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -6,6 +6,7 @@
#include <linux/sched/sysctl.h>
#include <linux/sched/rt.h>
#include <linux/sched/task.h>
+#include <linux/sched/topology.h>
#include <linux/init.h>
#include <linux/fs.h>
#include <linux/mm.h>
@@ -91,6 +92,10 @@ struct task_struct init_task
#endif
#ifdef CONFIG_CGROUP_SCHED
.sched_task_group = &root_task_group,
+#endif
+#ifdef CONFIG_UCLAMP_TASK
+ .uclamp[UCLAMP_MIN] = 0,
+ .uclamp[UCLAMP_MAX] = SCHED_CAPACITY_SCALE,
#endif
.ptraced = LIST_HEAD_INIT(init_task.ptraced),
.ptrace_entry = LIST_HEAD_INIT(init_task.ptrace_entry),
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ad97f3ba5ec5..3701bb1e6698 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -716,6 +716,28 @@ static void set_load_weight(struct task_struct *p, bool update_load)
}
}
+#ifdef CONFIG_UCLAMP_TASK
+static inline int __setscheduler_uclamp(struct task_struct *p,
+ const struct sched_attr *attr)
+{
+ if (attr->sched_util_min > attr->sched_util_max)
+ return -EINVAL;
+ if (attr->sched_util_max > SCHED_CAPACITY_SCALE)
+ return -EINVAL;
+
+ p->uclamp[UCLAMP_MIN] = attr->sched_util_min;
+ p->uclamp[UCLAMP_MAX] = attr->sched_util_max;
+
+ return 0;
+}
+#else /* CONFIG_UCLAMP_TASK */
+static inline int __setscheduler_uclamp(struct task_struct *p,
+ const struct sched_attr *attr)
+{
+ return -EINVAL;
+}
+#endif /* CONFIG_UCLAMP_TASK */
+
static inline void enqueue_task(struct rq *rq, struct task_struct *p, int flags)
{
if (!(flags & ENQUEUE_NOCLOCK))
@@ -2320,6 +2342,11 @@ int sched_fork(unsigned long clone_flags, struct task_struct *p)
p->prio = p->normal_prio = __normal_prio(p);
set_load_weight(p, false);
+#ifdef CONFIG_UCLAMP_TASK
+ p->uclamp[UCLAMP_MIN] = 0;
+ p->uclamp[UCLAMP_MAX] = SCHED_CAPACITY_SCALE;
+#endif
+
/*
* We don't need the reset flag anymore after the fork. It has
* fulfilled its duty:
@@ -4215,6 +4242,13 @@ static int __sched_setscheduler(struct task_struct *p,
return retval;
}
+ /* Configure utilization clamps for the task */
+ if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP) {
+ retval = __setscheduler_uclamp(p, attr);
+ if (retval)
+ return retval;
+ }
+
/*
* Make sure no PI-waiters arrive (or leave) while we are
* changing the priority of the task:
@@ -4721,6 +4755,11 @@ SYSCALL_DEFINE4(sched_getattr, pid_t, pid, struct sched_attr __user *, uattr,
else
attr.sched_nice = task_nice(p);
+#ifdef CONFIG_UCLAMP_TASK
+ attr.sched_util_min = p->uclamp[UCLAMP_MIN];
+ attr.sched_util_max = p->uclamp[UCLAMP_MAX];
+#endif
+
rcu_read_unlock();
retval = sched_read_attr(uattr, &attr, size);
--
2.18.0
^ permalink raw reply related
* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Dave Hansen @ 2018-10-29 17:29 UTC (permalink / raw)
To: Jann Horn
Cc: wad, Kees Cook, Linux API, Dave Hansen, linuxram, Andy Lutomirski,
msammler, linuxppc-dev
In-Reply-To: <CAG48ez1mY7Osdsc4iJ1JKq=C60wb9bQF=aHbi1fb=pb_V2XMJA@mail.gmail.com>
On 10/29/18 9:48 AM, Jann Horn wrote:
> On Mon, Oct 29, 2018 at 5:37 PM Dave Hansen <dave.hansen@intel.com> wrote:
>> I'm not sure this is a great use for PKRU. I *think* the basic problem
>> is that you want to communicate some rights information down into a
>> filter, and you want to communicate it with PKRU. While it's handy to
>> have an extra register that nobody (generally) mucks with, I'm not quite
>> convinced that we want to repurpose it this way.
>
> That's not how I understand it; I believe that the context is probably
> https://arxiv.org/pdf/1801.06822.pdf ?
> My understanding is that PKRU is used for lightweight in-process
> sandboxing, and to extend this sandbox protection to the syscall
> interface, it is necessary to expose PKRU state to seccomp filters.
> In other words, this isn't using PKRU exclusively for passing rights
> into a filter, but it has to use PKRU anyway.
PKRU gives information about rights to various bits of application data.
From that, a seccomp filter can infer the context, and thus the ability
for the code to call a given syscall at a certain point in time.
This makes PKRU an opt-in part of the syscall ABI, which is pretty
interesting. We _could_ do the same kind of thing with any callee-saved
general purpose register, but PKRU is particularly attractive because
there is only one instruction that writes to it (well, outside of
XSAVE*), and random library code is very unlikely at this point to be
using it.
PKRU getting reset on signals, and the requirement now that it *can't*
be changed if you make syscalls probably needs to get thought about very
carefully before we do this, though.
^ permalink raw reply
* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Dave Hansen @ 2018-10-29 17:07 UTC (permalink / raw)
To: Michael Sammler, Jann Horn
Cc: wad, Kees Cook, Rik van Riel, Linux API, Dave Hansen, linuxram,
Andy Lutomirski, linuxppc-dev
In-Reply-To: <24afa443-92e8-8202-845a-c1e1df1a3226@mpi-sws.org>
On 10/29/18 10:02 AM, Michael Sammler wrote:
>>> Also, I'm not sure the kernel provides the PKRU guarantees you want at
>>> the moment. Our implementation *probably* works, but it's mostly by
>>> accident.
> I don't know, which guarantees about the PKRU are provided at the
> moment, but the only guarantee needed for this patch is, that the kernel
> does not change the bits of the PKRU register, which belong to pkeys
> allocated by the user program, between the syscall entry and the call to
> secure_computing(). Is there are use case where the kernel would like to
> modify these bits of the PKRU?
We've been talking about doing more lax save/restore of the XSAVE
content (PKRU is part of this content). We would, for instance, only
restore it when returning to userspace, but PKRU might not be up-to-date
with the value in current->fpu.
It's not a deal-breaker with your approach, it's just something to be
careful of and make sure PKRU is up-to-date before you go use it.
^ permalink raw reply
* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Michael Sammler @ 2018-10-29 17:05 UTC (permalink / raw)
To: Ram Pai, Kees Cook
Cc: PowerPC, Linux API, Will Drewry, Dave Hansen, Andy Lutomirski
In-Reply-To: <20181029164806.GA5494@ram.oc3035372033.ibm.com>
On 10/29/2018 05:48 PM, Ram Pai wrote:
> On Mon, Oct 29, 2018 at 09:25:15AM -0700, Kees Cook wrote:
>> On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
>>> Add the current value of an architecture specific protection keys
>>> register (currently PKRU on x86) to data available for seccomp-bpf
>>> programs to work on. This allows filters based on the currently
>>> enabled protection keys.
>>>
>>> Support for protection keys on the POWER architecture is not part of
>>> this patch since I do not have access to a PowerPC, but adding support
>>> for it can be achieved by setting sd->pkeys to the AMR register in
>>> populate_seccomp_data.
> Maybe you can use a generic read_pkey() function, and each arch can
> provide their own implementation. In the case of powerpc it is
> mfspr(SPRN_AMR);
>
> something like the code below might help?
>
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 92a9962..d79efe3 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -89,6 +89,11 @@ static inline u16 pte_to_pkey_bits(u64 pteflags)
> #define __mm_pkey_is_reserved(pkey) (reserved_allocation_mask & \
> pkey_alloc_mask(pkey))
>
> +static inline u64 read_pkey()
> +{
> + return mfspr(SPRN_AMR);
> +}
> +
> static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
> {
> if (pkey < 0 || pkey >= arch_max_pkey())
> diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
> index 19b137f..feea114 100644
> --- a/arch/x86/include/asm/pkeys.h
> +++ b/arch/x86/include/asm/pkeys.h
> @@ -9,6 +9,11 @@
> extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> unsigned long init_val);
>
> +static inline u64 read_pkey()
> +{
> + return read_pkru();
> +}
> +
> static inline bool arch_pkeys_enabled(void)
> {
> return boot_cpu_has(X86_FEATURE_OSPKE);
> diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
> index 2955ba97..f1538c7 100644
> --- a/include/linux/pkeys.h
> +++ b/include/linux/pkeys.h
> @@ -13,6 +13,11 @@
> #define PKEY_DEDICATED_EXECUTE_ONLY 0
> #define ARCH_VM_PKEY_FLAGS 0
>
> +static inline u64 read_pkey()
> +{
> + return 0;
> +}
> +
> static inline int vma_pkey(struct vm_area_struct *vma)
> {
> return 0;
>
> RP
Thank you very much, this helps indeed. I will add this to the next
version of this patch.
>>> One use case for this patch is disabling unnecessary system calls for a
>>> library (e.g. network i/o for a crypto library) while the library runs
>>> without disabling the system calls for the whole program (by changing
>>> the protection keys before and after the library executes). Using this
>>> one could ensure that the library behaves a expected (e.g. the crypto
>>> library not sending private keys to a malicious server).
>>>
>>> This patch also enables lightweight sandboxing of untrusted code using
>>> memory protection keys: Protection keys provide memory isolation but
>>> for a sandbox system call isolation is needed as well. This patch
>>> allows writing a seccomp filter to prevent system calls by the
>>> untrusted code while still allowing system calls for the trusted code.
>> Isn't PKU instruction based? Couldn't a malicious library just change
>> the state of the MPK registers? This seems like an easy way to bypass
>> any filters that used PKU. I'm not convinced this is a meaningful
>> barrier that should be enforced by seccomp.
>>
>> This can also be done with a signal handler with SECCOMP_RET_TRAP and
>> check instruction pointer vs PKU state.
>>
>> -Kees
>>
>>> An alternative design would be to extend (c)BPF with a new instruction
>>> to read the state of a protection key. This alternate design would
>>> provide a simpler interface to the user space since the BPF program
>>> would not need to deal with the architecture specific pkeys field in
>>> seccomp_data, but the question is, how much of an advantage this would
>>> be as the nr field in seccomp_data is already architecture specific.
>>> Adding a new instruction for BPF programs is more complicated than
>>> this patch and might be a breaking change.
>>>
>>> Results of selftests/seccomp_benchmark.c on a x86 machine with pkeys
>>> support:
>>>
>>> With patch:
>>> Benchmarking 33554432 samples...
>>> 28.019505558 - 18.676858522 = 9342647036
>>> getpid native: 278 ns
>>> 42.279109885 - 28.019657031 = 14259452854
>>> getpid RET_ALLOW: 424 ns
>>> Estimated seccomp overhead per syscall: 146 ns
>>>
>>> Without patch:
>>> Benchmarking 33554432 samples...
>>> 28.059619466 - 18.706769155 = 9352850311
>>> getpid native: 278 ns
>>> 42.299228279 - 28.059761804 = 14239466475
>>> getpid RET_ALLOW: 424 ns
>>> Estimated seccomp overhead per syscall: 146 ns
>>>
>>> Cc: Kees Cook <keescook@chromium.org>
>>> Cc: Andy Lutomirski <luto@amacapital.net>
>>> Cc: Will Drewry <wad@chromium.org>
>>> Cc: Ram Pai <linuxram@us.ibm.com>
>>> Cc: linuxppc-dev@lists.ozlabs.org
>>> Cc: linux-api@vger.kernel.org
>>> Signed-off-by: Michael Sammler <msammler@mpi-sws.org>
>>> ---
>>> Changes to the previous version:
>>> - added motivation, notes about POWER, alternative design and benchmark results to the commit log
>>> - renamed pkru field in seccomp_data to pkeys
>>> - changed size of pkru field to __u64 and removed reserved field
>>> - added test for x86
>>>
>>> arch/mips/kernel/ptrace.c | 1 +
>>> arch/x86/entry/common.c | 1 +
>>> include/uapi/linux/seccomp.h | 3 +
>>> kernel/seccomp.c | 1 +
>>> tools/testing/selftests/seccomp/seccomp_bpf.c | 107 +++++++++++++++++++++++++-
>>> 5 files changed, 112 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
>>> index e5ba56c0..a58dd04d 100644
>>> --- a/arch/mips/kernel/ptrace.c
>>> +++ b/arch/mips/kernel/ptrace.c
>>> @@ -1277,6 +1277,7 @@ asmlinkage long syscall_trace_enter(struct pt_regs *regs, long syscall)
>>> for (i = 0; i < 6; i++)
>>> sd.args[i] = args[i];
>>> sd.instruction_pointer = KSTK_EIP(current);
>>> + sd.pkeys = 0;
>>>
>>> ret = __secure_computing(&sd);
>>> if (ret == -1)
>>> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
>>> index 3b2490b8..20c51bf2 100644
>>> --- a/arch/x86/entry/common.c
>>> +++ b/arch/x86/entry/common.c
>>> @@ -98,6 +98,7 @@ static long syscall_trace_enter(struct pt_regs *regs)
>>> sd.arch = arch;
>>> sd.nr = regs->orig_ax;
>>> sd.instruction_pointer = regs->ip;
>>> + sd.pkeys = read_pkru();
>>> #ifdef CONFIG_X86_64
>>> if (arch == AUDIT_ARCH_X86_64) {
>>> sd.args[0] = regs->di;
>>> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
>>> index 9efc0e73..3aa2d934 100644
>>> --- a/include/uapi/linux/seccomp.h
>>> +++ b/include/uapi/linux/seccomp.h
>>> @@ -52,12 +52,15 @@
>>> * @instruction_pointer: at the time of the system call.
>>> * @args: up to 6 system call arguments always stored as 64-bit values
>>> * regardless of the architecture.
>>> + * @pkeys: value of an architecture specific protection keys register
>>> + * (currently PKRU on x86)
>>> */
>>> struct seccomp_data {
>>> int nr;
>>> __u32 arch;
>>> __u64 instruction_pointer;
>>> __u64 args[6];
>>> + __u64 pkeys;
>>> };
>>>
>>> #endif /* _UAPI_LINUX_SECCOMP_H */
>>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>>> index fd023ac2..dfb8b0d6 100644
>>> --- a/kernel/seccomp.c
>>> +++ b/kernel/seccomp.c
>>> @@ -91,6 +91,7 @@ static void populate_seccomp_data(struct seccomp_data *sd)
>>> sd->args[4] = args[4];
>>> sd->args[5] = args[5];
>>> sd->instruction_pointer = KSTK_EIP(task);
>>> + sd->pkeys = 0;
>>> }
>>>
>>> /**
>>> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
>>> index e1473234..f7f8fa6f 100644
>>> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
>>> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
>>> @@ -82,6 +82,7 @@ struct seccomp_data {
>>> __u32 arch;
>>> __u64 instruction_pointer;
>>> __u64 args[6];
>>> + __u64 pkeys;
>>> };
>>> #endif
>>>
>>> @@ -732,7 +733,9 @@ TEST(KILL_process)
>>> TEST(arg_out_of_range)
>>> {
>>> struct sock_filter filter[] = {
>>> - BPF_STMT(BPF_LD|BPF_W|BPF_ABS, syscall_arg(6)),
>>> + BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
>>> + offsetof(struct seccomp_data, pkeys)
>>> + + sizeof(__u64)),
>>> BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
>>> };
>>> struct sock_fprog prog = {
>>> @@ -2933,6 +2936,108 @@ skip:
>>> ASSERT_EQ(0, kill(pid, SIGKILL));
>>> }
>>>
>>> +#if defined(__i386__) || defined(__x86_64__)
>>> +static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
>>> + unsigned int *ecx, unsigned int *edx)
>>> +{
>>> + /* ecx is often an input as well as an output. */
>>> + asm volatile(
>>> + "cpuid;"
>>> + : "=a" (*eax),
>>> + "=b" (*ebx),
>>> + "=c" (*ecx),
>>> + "=d" (*edx)
>>> + : "0" (*eax), "2" (*ecx));
>>> +}
>>> +
>>> +/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx) */
>>> +#define X86_FEATURE_PKU (1<<3) /* Protection Keys for Userspace */
>>> +#define X86_FEATURE_OSPKE (1<<4) /* OS Protection Keys Enable */
>>> +
>>> +static inline int cpu_has_pku(void)
>>> +{
>>> + unsigned int eax;
>>> + unsigned int ebx;
>>> + unsigned int ecx;
>>> + unsigned int edx;
>>> +
>>> + eax = 0x7;
>>> + ecx = 0x0;
>>> + __cpuid(&eax, &ebx, &ecx, &edx);
>>> +
>>> + if (!(ecx & X86_FEATURE_PKU))
>>> + return 0;
>>> + if (!(ecx & X86_FEATURE_OSPKE))
>>> + return 0;
>>> + return 1;
>>> +}
>>> +
>>> +static inline __u32 read_pkru(void)
>>> +{
>>> + if (!cpu_has_pku())
>>> + return 0;
>>> +
>>> + __u32 ecx = 0;
>>> + __u32 edx, pkru;
>>> +
>>> + /*
>>> + * "rdpkru" instruction. Places PKRU contents in to EAX,
>>> + * clears EDX and requires that ecx=0.
>>> + */
>>> + asm volatile(".byte 0x0f,0x01,0xee\n\t"
>>> + : "=a" (pkru), "=d" (edx)
>>> + : "c" (ecx));
>>> + return pkru;
>>> +}
>>> +
>>> +static inline void write_pkru(__u32 pkru)
>>> +{
>>> + if (!cpu_has_pku())
>>> + return;
>>> +
>>> + __u32 ecx = 0, edx = 0;
>>> +
>>> + /*
>>> + * "wrpkru" instruction. Loads contents in EAX to PKRU,
>>> + * requires that ecx = edx = 0.
>>> + */
>>> + asm volatile(".byte 0x0f,0x01,0xef\n\t"
>>> + : : "a" (pkru), "c"(ecx), "d"(edx));
>>> +}
>>> +
>>> +#define TEST_PKRU 0x55555550
>>> +
>>> +TEST_SIGNAL(pkeys_set, SIGSYS)
>>> +{
>>> + write_pkru(TEST_PKRU);
>>> + /* read back the written value because pkru might not be supported */
>>> + __u32 pkru = read_pkru();
>>> +
>>> + struct sock_filter filter[] = {
>>> + BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
>>> + offsetof(struct seccomp_data, pkeys)),
>>> + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, pkru, 1, 0),
>>> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
>>> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
>>> + };
>>> + struct sock_fprog prog = {
>>> + .len = (unsigned short)ARRAY_SIZE(filter),
>>> + .filter = filter,
>>> + };
>>> + long ret;
>>> +
>>> + ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
>>> + ASSERT_EQ(0, ret);
>>> +
>>> + ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog);
>>> + ASSERT_EQ(0, ret);
>>> +
>>> + /* should never return. */
>>> + EXPECT_EQ(0, syscall(__NR_getpid));
>>> +}
>>> +#endif
>>> +
>>> +
>>> /*
>>> * TODO:
>>> * - add microbenchmarks
>>> --
>>> 2.11.0
>>
>>
>> --
>> Kees Cook
^ permalink raw reply
* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Michael Sammler @ 2018-10-29 17:02 UTC (permalink / raw)
To: Jann Horn, Dave Hansen
Cc: wad, Kees Cook, Linux API, Dave Hansen, linuxram, Andy Lutomirski,
linuxppc-dev
In-Reply-To: <CAG48ez1mY7Osdsc4iJ1JKq=C60wb9bQF=aHbi1fb=pb_V2XMJA@mail.gmail.com>
On 10/29/2018 05:48 PM, Jann Horn wrote:
> On Mon, Oct 29, 2018 at 5:37 PM Dave Hansen <dave.hansen@intel.com> wrote:
>> On 10/29/18 9:25 AM, Kees Cook wrote:
>>> On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
>>>> Add the current value of an architecture specific protection keys
>>>> register (currently PKRU on x86) to data available for seccomp-bpf
>>>> programs to work on. This allows filters based on the currently
>>>> enabled protection keys.
>> How does the current "assignment" of protection keys to the various uses
>> get communicated to the filter?
> I assume that you first allocate your protection keys, then install the filter?
>
Yes, but I agree that it should probably be documented, that the filter
should only look at the parts of the PKRU, which belong to pkeys the
user space program allocated (if the kernel wants to use some parts of
the PKRU for its own purposes).
>> I'm not sure this is a great use for PKRU. I *think* the basic problem
>> is that you want to communicate some rights information down into a
>> filter, and you want to communicate it with PKRU. While it's handy to
>> have an extra register that nobody (generally) mucks with, I'm not quite
>> convinced that we want to repurpose it this way.
> That's not how I understand it; I believe that the context is probably
> https://arxiv.org/pdf/1801.06822.pdf ?
> My understanding is that PKRU is used for lightweight in-process
> sandboxing, and to extend this sandbox protection to the syscall
> interface, it is necessary to expose PKRU state to seccomp filters.
> In other words, this isn't using PKRU exclusively for passing rights
> into a filter, but it has to use PKRU anyway.
Yes, https://arxiv.org/pdf/1801.06822.pdf is indeed the context and what
you say is correct.
>> Also, I'm not sure the kernel provides the PKRU guarantees you want at
>> the moment. Our implementation *probably* works, but it's mostly by
>> accident.
I don't know, which guarantees about the PKRU are provided at the
moment, but the only guarantee needed for this patch is, that the kernel
does not change the bits of the PKRU register, which belong to pkeys
allocated by the user program, between the syscall entry and the call to
secure_computing(). Is there are use case where the kernel would like to
modify these bits of the PKRU?
-- MIchael
^ permalink raw reply
* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Jann Horn @ 2018-10-29 16:48 UTC (permalink / raw)
To: Dave Hansen
Cc: wad, Kees Cook, Linux API, Dave Hansen, linuxram, Andy Lutomirski,
msammler, linuxppc-dev
In-Reply-To: <7d93080b-68bd-7563-bd3b-e7ee1545e367@intel.com>
On Mon, Oct 29, 2018 at 5:37 PM Dave Hansen <dave.hansen@intel.com> wrote:
> On 10/29/18 9:25 AM, Kees Cook wrote:
> > On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
> >> Add the current value of an architecture specific protection keys
> >> register (currently PKRU on x86) to data available for seccomp-bpf
> >> programs to work on. This allows filters based on the currently
> >> enabled protection keys.
>
> How does the current "assignment" of protection keys to the various uses
> get communicated to the filter?
I assume that you first allocate your protection keys, then install the filter?
> I'm not sure this is a great use for PKRU. I *think* the basic problem
> is that you want to communicate some rights information down into a
> filter, and you want to communicate it with PKRU. While it's handy to
> have an extra register that nobody (generally) mucks with, I'm not quite
> convinced that we want to repurpose it this way.
That's not how I understand it; I believe that the context is probably
https://arxiv.org/pdf/1801.06822.pdf ?
My understanding is that PKRU is used for lightweight in-process
sandboxing, and to extend this sandbox protection to the syscall
interface, it is necessary to expose PKRU state to seccomp filters.
In other words, this isn't using PKRU exclusively for passing rights
into a filter, but it has to use PKRU anyway.
> Also, I'm not sure the kernel provides the PKRU guarantees you want at
> the moment. Our implementation *probably* works, but it's mostly by
> accident.
^ permalink raw reply
* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Ram Pai @ 2018-10-29 16:48 UTC (permalink / raw)
To: Kees Cook
Cc: Will Drewry, Linux API, Dave Hansen, Andy Lutomirski,
Michael Sammler, PowerPC
In-Reply-To: <CAGXu5jJ2fFmEXfUzga3XBXcsHPuPWWf5zLeyn8+Z1oZVa8oTuQ@mail.gmail.com>
On Mon, Oct 29, 2018 at 09:25:15AM -0700, Kees Cook wrote:
> On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
> > Add the current value of an architecture specific protection keys
> > register (currently PKRU on x86) to data available for seccomp-bpf
> > programs to work on. This allows filters based on the currently
> > enabled protection keys.
> >
> > Support for protection keys on the POWER architecture is not part of
> > this patch since I do not have access to a PowerPC, but adding support
> > for it can be achieved by setting sd->pkeys to the AMR register in
> > populate_seccomp_data.
Maybe you can use a generic read_pkey() function, and each arch can
provide their own implementation. In the case of powerpc it is
mfspr(SPRN_AMR);
something like the code below might help?
diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 92a9962..d79efe3 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -89,6 +89,11 @@ static inline u16 pte_to_pkey_bits(u64 pteflags)
#define __mm_pkey_is_reserved(pkey) (reserved_allocation_mask & \
pkey_alloc_mask(pkey))
+static inline u64 read_pkey()
+{
+ return mfspr(SPRN_AMR);
+}
+
static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)
{
if (pkey < 0 || pkey >= arch_max_pkey())
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index 19b137f..feea114 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -9,6 +9,11 @@
extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
unsigned long init_val);
+static inline u64 read_pkey()
+{
+ return read_pkru();
+}
+
static inline bool arch_pkeys_enabled(void)
{
return boot_cpu_has(X86_FEATURE_OSPKE);
diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
index 2955ba97..f1538c7 100644
--- a/include/linux/pkeys.h
+++ b/include/linux/pkeys.h
@@ -13,6 +13,11 @@
#define PKEY_DEDICATED_EXECUTE_ONLY 0
#define ARCH_VM_PKEY_FLAGS 0
+static inline u64 read_pkey()
+{
+ return 0;
+}
+
static inline int vma_pkey(struct vm_area_struct *vma)
{
return 0;
RP
> >
> > One use case for this patch is disabling unnecessary system calls for a
> > library (e.g. network i/o for a crypto library) while the library runs
> > without disabling the system calls for the whole program (by changing
> > the protection keys before and after the library executes). Using this
> > one could ensure that the library behaves a expected (e.g. the crypto
> > library not sending private keys to a malicious server).
> >
> > This patch also enables lightweight sandboxing of untrusted code using
> > memory protection keys: Protection keys provide memory isolation but
> > for a sandbox system call isolation is needed as well. This patch
> > allows writing a seccomp filter to prevent system calls by the
> > untrusted code while still allowing system calls for the trusted code.
>
> Isn't PKU instruction based? Couldn't a malicious library just change
> the state of the MPK registers? This seems like an easy way to bypass
> any filters that used PKU. I'm not convinced this is a meaningful
> barrier that should be enforced by seccomp.
>
> This can also be done with a signal handler with SECCOMP_RET_TRAP and
> check instruction pointer vs PKU state.
>
> -Kees
>
> >
> > An alternative design would be to extend (c)BPF with a new instruction
> > to read the state of a protection key. This alternate design would
> > provide a simpler interface to the user space since the BPF program
> > would not need to deal with the architecture specific pkeys field in
> > seccomp_data, but the question is, how much of an advantage this would
> > be as the nr field in seccomp_data is already architecture specific.
> > Adding a new instruction for BPF programs is more complicated than
> > this patch and might be a breaking change.
> >
> > Results of selftests/seccomp_benchmark.c on a x86 machine with pkeys
> > support:
> >
> > With patch:
> > Benchmarking 33554432 samples...
> > 28.019505558 - 18.676858522 = 9342647036
> > getpid native: 278 ns
> > 42.279109885 - 28.019657031 = 14259452854
> > getpid RET_ALLOW: 424 ns
> > Estimated seccomp overhead per syscall: 146 ns
> >
> > Without patch:
> > Benchmarking 33554432 samples...
> > 28.059619466 - 18.706769155 = 9352850311
> > getpid native: 278 ns
> > 42.299228279 - 28.059761804 = 14239466475
> > getpid RET_ALLOW: 424 ns
> > Estimated seccomp overhead per syscall: 146 ns
> >
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Will Drewry <wad@chromium.org>
> > Cc: Ram Pai <linuxram@us.ibm.com>
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: linux-api@vger.kernel.org
> > Signed-off-by: Michael Sammler <msammler@mpi-sws.org>
> > ---
> > Changes to the previous version:
> > - added motivation, notes about POWER, alternative design and benchmark results to the commit log
> > - renamed pkru field in seccomp_data to pkeys
> > - changed size of pkru field to __u64 and removed reserved field
> > - added test for x86
> >
> > arch/mips/kernel/ptrace.c | 1 +
> > arch/x86/entry/common.c | 1 +
> > include/uapi/linux/seccomp.h | 3 +
> > kernel/seccomp.c | 1 +
> > tools/testing/selftests/seccomp/seccomp_bpf.c | 107 +++++++++++++++++++++++++-
> > 5 files changed, 112 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
> > index e5ba56c0..a58dd04d 100644
> > --- a/arch/mips/kernel/ptrace.c
> > +++ b/arch/mips/kernel/ptrace.c
> > @@ -1277,6 +1277,7 @@ asmlinkage long syscall_trace_enter(struct pt_regs *regs, long syscall)
> > for (i = 0; i < 6; i++)
> > sd.args[i] = args[i];
> > sd.instruction_pointer = KSTK_EIP(current);
> > + sd.pkeys = 0;
> >
> > ret = __secure_computing(&sd);
> > if (ret == -1)
> > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> > index 3b2490b8..20c51bf2 100644
> > --- a/arch/x86/entry/common.c
> > +++ b/arch/x86/entry/common.c
> > @@ -98,6 +98,7 @@ static long syscall_trace_enter(struct pt_regs *regs)
> > sd.arch = arch;
> > sd.nr = regs->orig_ax;
> > sd.instruction_pointer = regs->ip;
> > + sd.pkeys = read_pkru();
> > #ifdef CONFIG_X86_64
> > if (arch == AUDIT_ARCH_X86_64) {
> > sd.args[0] = regs->di;
> > diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> > index 9efc0e73..3aa2d934 100644
> > --- a/include/uapi/linux/seccomp.h
> > +++ b/include/uapi/linux/seccomp.h
> > @@ -52,12 +52,15 @@
> > * @instruction_pointer: at the time of the system call.
> > * @args: up to 6 system call arguments always stored as 64-bit values
> > * regardless of the architecture.
> > + * @pkeys: value of an architecture specific protection keys register
> > + * (currently PKRU on x86)
> > */
> > struct seccomp_data {
> > int nr;
> > __u32 arch;
> > __u64 instruction_pointer;
> > __u64 args[6];
> > + __u64 pkeys;
> > };
> >
> > #endif /* _UAPI_LINUX_SECCOMP_H */
> > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > index fd023ac2..dfb8b0d6 100644
> > --- a/kernel/seccomp.c
> > +++ b/kernel/seccomp.c
> > @@ -91,6 +91,7 @@ static void populate_seccomp_data(struct seccomp_data *sd)
> > sd->args[4] = args[4];
> > sd->args[5] = args[5];
> > sd->instruction_pointer = KSTK_EIP(task);
> > + sd->pkeys = 0;
> > }
> >
> > /**
> > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> > index e1473234..f7f8fa6f 100644
> > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> > @@ -82,6 +82,7 @@ struct seccomp_data {
> > __u32 arch;
> > __u64 instruction_pointer;
> > __u64 args[6];
> > + __u64 pkeys;
> > };
> > #endif
> >
> > @@ -732,7 +733,9 @@ TEST(KILL_process)
> > TEST(arg_out_of_range)
> > {
> > struct sock_filter filter[] = {
> > - BPF_STMT(BPF_LD|BPF_W|BPF_ABS, syscall_arg(6)),
> > + BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> > + offsetof(struct seccomp_data, pkeys)
> > + + sizeof(__u64)),
> > BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> > };
> > struct sock_fprog prog = {
> > @@ -2933,6 +2936,108 @@ skip:
> > ASSERT_EQ(0, kill(pid, SIGKILL));
> > }
> >
> > +#if defined(__i386__) || defined(__x86_64__)
> > +static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
> > + unsigned int *ecx, unsigned int *edx)
> > +{
> > + /* ecx is often an input as well as an output. */
> > + asm volatile(
> > + "cpuid;"
> > + : "=a" (*eax),
> > + "=b" (*ebx),
> > + "=c" (*ecx),
> > + "=d" (*edx)
> > + : "0" (*eax), "2" (*ecx));
> > +}
> > +
> > +/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx) */
> > +#define X86_FEATURE_PKU (1<<3) /* Protection Keys for Userspace */
> > +#define X86_FEATURE_OSPKE (1<<4) /* OS Protection Keys Enable */
> > +
> > +static inline int cpu_has_pku(void)
> > +{
> > + unsigned int eax;
> > + unsigned int ebx;
> > + unsigned int ecx;
> > + unsigned int edx;
> > +
> > + eax = 0x7;
> > + ecx = 0x0;
> > + __cpuid(&eax, &ebx, &ecx, &edx);
> > +
> > + if (!(ecx & X86_FEATURE_PKU))
> > + return 0;
> > + if (!(ecx & X86_FEATURE_OSPKE))
> > + return 0;
> > + return 1;
> > +}
> > +
> > +static inline __u32 read_pkru(void)
> > +{
> > + if (!cpu_has_pku())
> > + return 0;
> > +
> > + __u32 ecx = 0;
> > + __u32 edx, pkru;
> > +
> > + /*
> > + * "rdpkru" instruction. Places PKRU contents in to EAX,
> > + * clears EDX and requires that ecx=0.
> > + */
> > + asm volatile(".byte 0x0f,0x01,0xee\n\t"
> > + : "=a" (pkru), "=d" (edx)
> > + : "c" (ecx));
> > + return pkru;
> > +}
> > +
> > +static inline void write_pkru(__u32 pkru)
> > +{
> > + if (!cpu_has_pku())
> > + return;
> > +
> > + __u32 ecx = 0, edx = 0;
> > +
> > + /*
> > + * "wrpkru" instruction. Loads contents in EAX to PKRU,
> > + * requires that ecx = edx = 0.
> > + */
> > + asm volatile(".byte 0x0f,0x01,0xef\n\t"
> > + : : "a" (pkru), "c"(ecx), "d"(edx));
> > +}
> > +
> > +#define TEST_PKRU 0x55555550
> > +
> > +TEST_SIGNAL(pkeys_set, SIGSYS)
> > +{
> > + write_pkru(TEST_PKRU);
> > + /* read back the written value because pkru might not be supported */
> > + __u32 pkru = read_pkru();
> > +
> > + struct sock_filter filter[] = {
> > + BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> > + offsetof(struct seccomp_data, pkeys)),
> > + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, pkru, 1, 0),
> > + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> > + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
> > + };
> > + struct sock_fprog prog = {
> > + .len = (unsigned short)ARRAY_SIZE(filter),
> > + .filter = filter,
> > + };
> > + long ret;
> > +
> > + ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
> > + ASSERT_EQ(0, ret);
> > +
> > + ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog);
> > + ASSERT_EQ(0, ret);
> > +
> > + /* should never return. */
> > + EXPECT_EQ(0, syscall(__NR_getpid));
> > +}
> > +#endif
> > +
> > +
> > /*
> > * TODO:
> > * - add microbenchmarks
> > --
> > 2.11.0
>
>
>
> --
> Kees Cook
--
Ram Pai
^ permalink raw reply related
* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Jann Horn @ 2018-10-29 16:42 UTC (permalink / raw)
To: Kees Cook
Cc: wad, Linux API, Dave Hansen, linuxram, Andy Lutomirski, msammler,
linuxppc-dev
In-Reply-To: <CAGXu5jJ2fFmEXfUzga3XBXcsHPuPWWf5zLeyn8+Z1oZVa8oTuQ@mail.gmail.com>
On Mon, Oct 29, 2018 at 5:25 PM Kees Cook <keescook@chromium.org> wrote:
> On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
> > Add the current value of an architecture specific protection keys
> > register (currently PKRU on x86) to data available for seccomp-bpf
> > programs to work on. This allows filters based on the currently
> > enabled protection keys.
> >
> > Support for protection keys on the POWER architecture is not part of
> > this patch since I do not have access to a PowerPC, but adding support
> > for it can be achieved by setting sd->pkeys to the AMR register in
> > populate_seccomp_data.
> >
> > One use case for this patch is disabling unnecessary system calls for a
> > library (e.g. network i/o for a crypto library) while the library runs
> > without disabling the system calls for the whole program (by changing
> > the protection keys before and after the library executes). Using this
> > one could ensure that the library behaves a expected (e.g. the crypto
> > library not sending private keys to a malicious server).
> >
> > This patch also enables lightweight sandboxing of untrusted code using
> > memory protection keys: Protection keys provide memory isolation but
> > for a sandbox system call isolation is needed as well. This patch
> > allows writing a seccomp filter to prevent system calls by the
> > untrusted code while still allowing system calls for the trusted code.
>
> Isn't PKU instruction based? Couldn't a malicious library just change
> the state of the MPK registers? This seems like an easy way to bypass
> any filters that used PKU. I'm not convinced this is a meaningful
> barrier that should be enforced by seccomp.
The idea is that if you know that there are no unwanted WRPKRU
instructions in your code, and all WRPKRU instructions in your code
are directly followed by code that verifies correctness of the
protection key, then you can implement lightweight sandboxes inside a
single process. See https://arxiv.org/pdf/1801.06822.pdf for one
description of this approach.
> This can also be done with a signal handler with SECCOMP_RET_TRAP and
> check instruction pointer vs PKU state.
But then you can't prevent an attacker from just jumping directly to
your syscall instruction. And it's slow.
^ permalink raw reply
* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Dave Hansen @ 2018-10-29 16:37 UTC (permalink / raw)
To: Kees Cook, Michael Sammler
Cc: Will Drewry, Linux API, Dave Hansen, Ram Pai, Andy Lutomirski,
PowerPC
In-Reply-To: <CAGXu5jJ2fFmEXfUzga3XBXcsHPuPWWf5zLeyn8+Z1oZVa8oTuQ@mail.gmail.com>
On 10/29/18 9:25 AM, Kees Cook wrote:
> On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
>> Add the current value of an architecture specific protection keys
>> register (currently PKRU on x86) to data available for seccomp-bpf
>> programs to work on. This allows filters based on the currently
>> enabled protection keys.
How does the current "assignment" of protection keys to the various uses
get communicated to the filter?
I'm not sure this is a great use for PKRU. I *think* the basic problem
is that you want to communicate some rights information down into a
filter, and you want to communicate it with PKRU. While it's handy to
have an extra register that nobody (generally) mucks with, I'm not quite
convinced that we want to repurpose it this way.
Also, I'm not sure the kernel provides the PKRU guarantees you want at
the moment. Our implementation *probably* works, but it's mostly by
accident.
^ permalink raw reply
* Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Kees Cook @ 2018-10-29 16:25 UTC (permalink / raw)
To: Michael Sammler
Cc: Will Drewry, Linux API, Dave Hansen, Ram Pai, Andy Lutomirski,
PowerPC
In-Reply-To: <20181029112343.27454-1-msammler@mpi-sws.org>
On Mon, Oct 29, 2018 at 4:23 AM, Michael Sammler <msammler@mpi-sws.org> wrote:
> Add the current value of an architecture specific protection keys
> register (currently PKRU on x86) to data available for seccomp-bpf
> programs to work on. This allows filters based on the currently
> enabled protection keys.
>
> Support for protection keys on the POWER architecture is not part of
> this patch since I do not have access to a PowerPC, but adding support
> for it can be achieved by setting sd->pkeys to the AMR register in
> populate_seccomp_data.
>
> One use case for this patch is disabling unnecessary system calls for a
> library (e.g. network i/o for a crypto library) while the library runs
> without disabling the system calls for the whole program (by changing
> the protection keys before and after the library executes). Using this
> one could ensure that the library behaves a expected (e.g. the crypto
> library not sending private keys to a malicious server).
>
> This patch also enables lightweight sandboxing of untrusted code using
> memory protection keys: Protection keys provide memory isolation but
> for a sandbox system call isolation is needed as well. This patch
> allows writing a seccomp filter to prevent system calls by the
> untrusted code while still allowing system calls for the trusted code.
Isn't PKU instruction based? Couldn't a malicious library just change
the state of the MPK registers? This seems like an easy way to bypass
any filters that used PKU. I'm not convinced this is a meaningful
barrier that should be enforced by seccomp.
This can also be done with a signal handler with SECCOMP_RET_TRAP and
check instruction pointer vs PKU state.
-Kees
>
> An alternative design would be to extend (c)BPF with a new instruction
> to read the state of a protection key. This alternate design would
> provide a simpler interface to the user space since the BPF program
> would not need to deal with the architecture specific pkeys field in
> seccomp_data, but the question is, how much of an advantage this would
> be as the nr field in seccomp_data is already architecture specific.
> Adding a new instruction for BPF programs is more complicated than
> this patch and might be a breaking change.
>
> Results of selftests/seccomp_benchmark.c on a x86 machine with pkeys
> support:
>
> With patch:
> Benchmarking 33554432 samples...
> 28.019505558 - 18.676858522 = 9342647036
> getpid native: 278 ns
> 42.279109885 - 28.019657031 = 14259452854
> getpid RET_ALLOW: 424 ns
> Estimated seccomp overhead per syscall: 146 ns
>
> Without patch:
> Benchmarking 33554432 samples...
> 28.059619466 - 18.706769155 = 9352850311
> getpid native: 278 ns
> 42.299228279 - 28.059761804 = 14239466475
> getpid RET_ALLOW: 424 ns
> Estimated seccomp overhead per syscall: 146 ns
>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Will Drewry <wad@chromium.org>
> Cc: Ram Pai <linuxram@us.ibm.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-api@vger.kernel.org
> Signed-off-by: Michael Sammler <msammler@mpi-sws.org>
> ---
> Changes to the previous version:
> - added motivation, notes about POWER, alternative design and benchmark results to the commit log
> - renamed pkru field in seccomp_data to pkeys
> - changed size of pkru field to __u64 and removed reserved field
> - added test for x86
>
> arch/mips/kernel/ptrace.c | 1 +
> arch/x86/entry/common.c | 1 +
> include/uapi/linux/seccomp.h | 3 +
> kernel/seccomp.c | 1 +
> tools/testing/selftests/seccomp/seccomp_bpf.c | 107 +++++++++++++++++++++++++-
> 5 files changed, 112 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
> index e5ba56c0..a58dd04d 100644
> --- a/arch/mips/kernel/ptrace.c
> +++ b/arch/mips/kernel/ptrace.c
> @@ -1277,6 +1277,7 @@ asmlinkage long syscall_trace_enter(struct pt_regs *regs, long syscall)
> for (i = 0; i < 6; i++)
> sd.args[i] = args[i];
> sd.instruction_pointer = KSTK_EIP(current);
> + sd.pkeys = 0;
>
> ret = __secure_computing(&sd);
> if (ret == -1)
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index 3b2490b8..20c51bf2 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -98,6 +98,7 @@ static long syscall_trace_enter(struct pt_regs *regs)
> sd.arch = arch;
> sd.nr = regs->orig_ax;
> sd.instruction_pointer = regs->ip;
> + sd.pkeys = read_pkru();
> #ifdef CONFIG_X86_64
> if (arch == AUDIT_ARCH_X86_64) {
> sd.args[0] = regs->di;
> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> index 9efc0e73..3aa2d934 100644
> --- a/include/uapi/linux/seccomp.h
> +++ b/include/uapi/linux/seccomp.h
> @@ -52,12 +52,15 @@
> * @instruction_pointer: at the time of the system call.
> * @args: up to 6 system call arguments always stored as 64-bit values
> * regardless of the architecture.
> + * @pkeys: value of an architecture specific protection keys register
> + * (currently PKRU on x86)
> */
> struct seccomp_data {
> int nr;
> __u32 arch;
> __u64 instruction_pointer;
> __u64 args[6];
> + __u64 pkeys;
> };
>
> #endif /* _UAPI_LINUX_SECCOMP_H */
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index fd023ac2..dfb8b0d6 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -91,6 +91,7 @@ static void populate_seccomp_data(struct seccomp_data *sd)
> sd->args[4] = args[4];
> sd->args[5] = args[5];
> sd->instruction_pointer = KSTK_EIP(task);
> + sd->pkeys = 0;
> }
>
> /**
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index e1473234..f7f8fa6f 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -82,6 +82,7 @@ struct seccomp_data {
> __u32 arch;
> __u64 instruction_pointer;
> __u64 args[6];
> + __u64 pkeys;
> };
> #endif
>
> @@ -732,7 +733,9 @@ TEST(KILL_process)
> TEST(arg_out_of_range)
> {
> struct sock_filter filter[] = {
> - BPF_STMT(BPF_LD|BPF_W|BPF_ABS, syscall_arg(6)),
> + BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> + offsetof(struct seccomp_data, pkeys)
> + + sizeof(__u64)),
> BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> };
> struct sock_fprog prog = {
> @@ -2933,6 +2936,108 @@ skip:
> ASSERT_EQ(0, kill(pid, SIGKILL));
> }
>
> +#if defined(__i386__) || defined(__x86_64__)
> +static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
> + unsigned int *ecx, unsigned int *edx)
> +{
> + /* ecx is often an input as well as an output. */
> + asm volatile(
> + "cpuid;"
> + : "=a" (*eax),
> + "=b" (*ebx),
> + "=c" (*ecx),
> + "=d" (*edx)
> + : "0" (*eax), "2" (*ecx));
> +}
> +
> +/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx) */
> +#define X86_FEATURE_PKU (1<<3) /* Protection Keys for Userspace */
> +#define X86_FEATURE_OSPKE (1<<4) /* OS Protection Keys Enable */
> +
> +static inline int cpu_has_pku(void)
> +{
> + unsigned int eax;
> + unsigned int ebx;
> + unsigned int ecx;
> + unsigned int edx;
> +
> + eax = 0x7;
> + ecx = 0x0;
> + __cpuid(&eax, &ebx, &ecx, &edx);
> +
> + if (!(ecx & X86_FEATURE_PKU))
> + return 0;
> + if (!(ecx & X86_FEATURE_OSPKE))
> + return 0;
> + return 1;
> +}
> +
> +static inline __u32 read_pkru(void)
> +{
> + if (!cpu_has_pku())
> + return 0;
> +
> + __u32 ecx = 0;
> + __u32 edx, pkru;
> +
> + /*
> + * "rdpkru" instruction. Places PKRU contents in to EAX,
> + * clears EDX and requires that ecx=0.
> + */
> + asm volatile(".byte 0x0f,0x01,0xee\n\t"
> + : "=a" (pkru), "=d" (edx)
> + : "c" (ecx));
> + return pkru;
> +}
> +
> +static inline void write_pkru(__u32 pkru)
> +{
> + if (!cpu_has_pku())
> + return;
> +
> + __u32 ecx = 0, edx = 0;
> +
> + /*
> + * "wrpkru" instruction. Loads contents in EAX to PKRU,
> + * requires that ecx = edx = 0.
> + */
> + asm volatile(".byte 0x0f,0x01,0xef\n\t"
> + : : "a" (pkru), "c"(ecx), "d"(edx));
> +}
> +
> +#define TEST_PKRU 0x55555550
> +
> +TEST_SIGNAL(pkeys_set, SIGSYS)
> +{
> + write_pkru(TEST_PKRU);
> + /* read back the written value because pkru might not be supported */
> + __u32 pkru = read_pkru();
> +
> + struct sock_filter filter[] = {
> + BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> + offsetof(struct seccomp_data, pkeys)),
> + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, pkru, 1, 0),
> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
> + };
> + struct sock_fprog prog = {
> + .len = (unsigned short)ARRAY_SIZE(filter),
> + .filter = filter,
> + };
> + long ret;
> +
> + ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
> + ASSERT_EQ(0, ret);
> +
> + ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog);
> + ASSERT_EQ(0, ret);
> +
> + /* should never return. */
> + EXPECT_EQ(0, syscall(__NR_getpid));
> +}
> +#endif
> +
> +
> /*
> * TODO:
> * - add microbenchmarks
> --
> 2.11.0
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v3 0/2] sysctl: handle overflow for file-max
From: Christian Brauner @ 2018-10-29 14:58 UTC (permalink / raw)
To: keescook, linux-kernel
Cc: ebiederm, mcgrof, akpm, joe.lawrence, longman, linux, viro,
adobriyan, linux-api
In-Reply-To: <20181016223322.16844-1-christian@brauner.io>
On Wed, Oct 17, 2018 at 12:33:20AM +0200, Christian Brauner wrote:
> Hey,
>
> Here is v3 of this patchset. Changelogs are in the individual commits.
>
> Currently, when writing
>
> echo 18446744073709551616 > /proc/sys/fs/file-max
>
> /proc/sys/fs/file-max will overflow and be set to 0. That quickly
> crashes the system.
>
> The first version of this patch intended to detect the overflow and cap
> at ULONG_MAX. However, we should not do this and rather return EINVAL on
> overflow. The reasons are:
> - this aligns with other sysctl handlers that simply reject overflows
> (cf. [1], [2], and a bunch of others)
> - we already do a partial fail on overflow right now
> Namely, when the TMPBUFLEN is exceeded. So we already reject values
> such as 184467440737095516160 (21 chars) but accept values such as
> 18446744073709551616 (20 chars) but both are overflows. So we should
> just always reject 64bit overflows and not special-case this based on
> the number of chars.
>
> (This patchset is in reference to https://lkml.org/lkml/2018/10/11/585.)
Just so that we don't forget, can we make sure that this gets picked
into linux-next? :)
Christian
>
> Thanks!
> Christian
>
> [1]: fb910c42cceb ("sysctl: check for UINT_MAX before unsigned int min/max")
> [2]: 196851bed522 ("s390/topology: correct topology mode proc handler")
>
> Christian Brauner (2):
> sysctl: handle overflow in proc_get_long
> sysctl: handle overflow for file-max
>
> kernel/sysctl.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 46 insertions(+), 1 deletion(-)
>
> --
> 2.17.1
>
^ permalink raw reply
* [RFC PATCH] seccomp: Add protection keys into seccomp_data
From: Michael Sammler @ 2018-10-29 11:23 UTC (permalink / raw)
Cc: Will Drewry, Kees Cook, linux-api, Ram Pai, Andy Lutomirski,
Michael Sammler, linuxppc-dev
Add the current value of an architecture specific protection keys
register (currently PKRU on x86) to data available for seccomp-bpf
programs to work on. This allows filters based on the currently
enabled protection keys.
Support for protection keys on the POWER architecture is not part of
this patch since I do not have access to a PowerPC, but adding support
for it can be achieved by setting sd->pkeys to the AMR register in
populate_seccomp_data.
One use case for this patch is disabling unnecessary system calls for a
library (e.g. network i/o for a crypto library) while the library runs
without disabling the system calls for the whole program (by changing
the protection keys before and after the library executes). Using this
one could ensure that the library behaves a expected (e.g. the crypto
library not sending private keys to a malicious server).
This patch also enables lightweight sandboxing of untrusted code using
memory protection keys: Protection keys provide memory isolation but
for a sandbox system call isolation is needed as well. This patch
allows writing a seccomp filter to prevent system calls by the
untrusted code while still allowing system calls for the trusted code.
An alternative design would be to extend (c)BPF with a new instruction
to read the state of a protection key. This alternate design would
provide a simpler interface to the user space since the BPF program
would not need to deal with the architecture specific pkeys field in
seccomp_data, but the question is, how much of an advantage this would
be as the nr field in seccomp_data is already architecture specific.
Adding a new instruction for BPF programs is more complicated than
this patch and might be a breaking change.
Results of selftests/seccomp_benchmark.c on a x86 machine with pkeys
support:
With patch:
Benchmarking 33554432 samples...
28.019505558 - 18.676858522 = 9342647036
getpid native: 278 ns
42.279109885 - 28.019657031 = 14259452854
getpid RET_ALLOW: 424 ns
Estimated seccomp overhead per syscall: 146 ns
Without patch:
Benchmarking 33554432 samples...
28.059619466 - 18.706769155 = 9352850311
getpid native: 278 ns
42.299228279 - 28.059761804 = 14239466475
getpid RET_ALLOW: 424 ns
Estimated seccomp overhead per syscall: 146 ns
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Will Drewry <wad@chromium.org>
Cc: Ram Pai <linuxram@us.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-api@vger.kernel.org
Signed-off-by: Michael Sammler <msammler@mpi-sws.org>
---
Changes to the previous version:
- added motivation, notes about POWER, alternative design and benchmark results to the commit log
- renamed pkru field in seccomp_data to pkeys
- changed size of pkru field to __u64 and removed reserved field
- added test for x86
arch/mips/kernel/ptrace.c | 1 +
arch/x86/entry/common.c | 1 +
include/uapi/linux/seccomp.h | 3 +
kernel/seccomp.c | 1 +
tools/testing/selftests/seccomp/seccomp_bpf.c | 107 +++++++++++++++++++++++++-
5 files changed, 112 insertions(+), 1 deletion(-)
diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
index e5ba56c0..a58dd04d 100644
--- a/arch/mips/kernel/ptrace.c
+++ b/arch/mips/kernel/ptrace.c
@@ -1277,6 +1277,7 @@ asmlinkage long syscall_trace_enter(struct pt_regs *regs, long syscall)
for (i = 0; i < 6; i++)
sd.args[i] = args[i];
sd.instruction_pointer = KSTK_EIP(current);
+ sd.pkeys = 0;
ret = __secure_computing(&sd);
if (ret == -1)
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 3b2490b8..20c51bf2 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -98,6 +98,7 @@ static long syscall_trace_enter(struct pt_regs *regs)
sd.arch = arch;
sd.nr = regs->orig_ax;
sd.instruction_pointer = regs->ip;
+ sd.pkeys = read_pkru();
#ifdef CONFIG_X86_64
if (arch == AUDIT_ARCH_X86_64) {
sd.args[0] = regs->di;
diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index 9efc0e73..3aa2d934 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -52,12 +52,15 @@
* @instruction_pointer: at the time of the system call.
* @args: up to 6 system call arguments always stored as 64-bit values
* regardless of the architecture.
+ * @pkeys: value of an architecture specific protection keys register
+ * (currently PKRU on x86)
*/
struct seccomp_data {
int nr;
__u32 arch;
__u64 instruction_pointer;
__u64 args[6];
+ __u64 pkeys;
};
#endif /* _UAPI_LINUX_SECCOMP_H */
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index fd023ac2..dfb8b0d6 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -91,6 +91,7 @@ static void populate_seccomp_data(struct seccomp_data *sd)
sd->args[4] = args[4];
sd->args[5] = args[5];
sd->instruction_pointer = KSTK_EIP(task);
+ sd->pkeys = 0;
}
/**
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index e1473234..f7f8fa6f 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -82,6 +82,7 @@ struct seccomp_data {
__u32 arch;
__u64 instruction_pointer;
__u64 args[6];
+ __u64 pkeys;
};
#endif
@@ -732,7 +733,9 @@ TEST(KILL_process)
TEST(arg_out_of_range)
{
struct sock_filter filter[] = {
- BPF_STMT(BPF_LD|BPF_W|BPF_ABS, syscall_arg(6)),
+ BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
+ offsetof(struct seccomp_data, pkeys)
+ + sizeof(__u64)),
BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
};
struct sock_fprog prog = {
@@ -2933,6 +2936,108 @@ skip:
ASSERT_EQ(0, kill(pid, SIGKILL));
}
+#if defined(__i386__) || defined(__x86_64__)
+static inline void __cpuid(unsigned int *eax, unsigned int *ebx,
+ unsigned int *ecx, unsigned int *edx)
+{
+ /* ecx is often an input as well as an output. */
+ asm volatile(
+ "cpuid;"
+ : "=a" (*eax),
+ "=b" (*ebx),
+ "=c" (*ecx),
+ "=d" (*edx)
+ : "0" (*eax), "2" (*ecx));
+}
+
+/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx) */
+#define X86_FEATURE_PKU (1<<3) /* Protection Keys for Userspace */
+#define X86_FEATURE_OSPKE (1<<4) /* OS Protection Keys Enable */
+
+static inline int cpu_has_pku(void)
+{
+ unsigned int eax;
+ unsigned int ebx;
+ unsigned int ecx;
+ unsigned int edx;
+
+ eax = 0x7;
+ ecx = 0x0;
+ __cpuid(&eax, &ebx, &ecx, &edx);
+
+ if (!(ecx & X86_FEATURE_PKU))
+ return 0;
+ if (!(ecx & X86_FEATURE_OSPKE))
+ return 0;
+ return 1;
+}
+
+static inline __u32 read_pkru(void)
+{
+ if (!cpu_has_pku())
+ return 0;
+
+ __u32 ecx = 0;
+ __u32 edx, pkru;
+
+ /*
+ * "rdpkru" instruction. Places PKRU contents in to EAX,
+ * clears EDX and requires that ecx=0.
+ */
+ asm volatile(".byte 0x0f,0x01,0xee\n\t"
+ : "=a" (pkru), "=d" (edx)
+ : "c" (ecx));
+ return pkru;
+}
+
+static inline void write_pkru(__u32 pkru)
+{
+ if (!cpu_has_pku())
+ return;
+
+ __u32 ecx = 0, edx = 0;
+
+ /*
+ * "wrpkru" instruction. Loads contents in EAX to PKRU,
+ * requires that ecx = edx = 0.
+ */
+ asm volatile(".byte 0x0f,0x01,0xef\n\t"
+ : : "a" (pkru), "c"(ecx), "d"(edx));
+}
+
+#define TEST_PKRU 0x55555550
+
+TEST_SIGNAL(pkeys_set, SIGSYS)
+{
+ write_pkru(TEST_PKRU);
+ /* read back the written value because pkru might not be supported */
+ __u32 pkru = read_pkru();
+
+ struct sock_filter filter[] = {
+ BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
+ offsetof(struct seccomp_data, pkeys)),
+ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, pkru, 1, 0),
+ BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+ BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
+ };
+ struct sock_fprog prog = {
+ .len = (unsigned short)ARRAY_SIZE(filter),
+ .filter = filter,
+ };
+ long ret;
+
+ ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
+ ASSERT_EQ(0, ret);
+
+ ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog);
+ ASSERT_EQ(0, ret);
+
+ /* should never return. */
+ EXPECT_EQ(0, syscall(__NR_getpid));
+}
+#endif
+
+
/*
* TODO:
* - add microbenchmarks
--
2.11.0
^ permalink raw reply related
* Re: [PATCH ghak90 (was ghak32) V4 03/10] audit: log container info of syscalls
From: Paul Moore @ 2018-10-28 7:53 UTC (permalink / raw)
To: casey
Cc: sgrubb, luto, rgb, linux-api, containers, linux-kernel, viro,
dhowells, carlos, linux-audit, netfilter-devel, ebiederm, simo,
netdev, linux-fsdevel, Eric Paris, Serge Hallyn
In-Reply-To: <e1d8a8cf-5013-ffbe-923b-604851de836d@schaufler-ca.com>
On Fri, Oct 26, 2018 at 4:13 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 10/25/2018 2:55 PM, Steve Grubb wrote:
> > ...
> > And historically speaking setting audit loginuid produces a LOGIN
> > event, so it only makes sense to consider binding container ID to
> > container as a CONTAINER event. For other supplemental records, we name
> > things what they are: PATH, CWD, SOCKADDR, etc. So, CONTAINER_ID makes
> > sense. CONTAINER_OP sounds like its for operations on a container. Do
> > we have any operations on a container?
>
> The answer has to be "no", because containers are, by emphatic assertion,
> not kernel constructs. Any CONTAINER_OP event has to come from user space.
> I think.
It is very important that we do not confuse operations on the audit
container id with operations on the containers themselves. Of course
at a higher level, e.g. audit log analysis, we want to equate the two,
and if the container runtime which manages the audit container id is
sane that should be a reasonable assumption, but in this particular
patchset AUDIT_CONTAINER_OP is referring to operations involving just
the audit container id.
If there is a need for additional container operation auditing (note
well that I did not say audit container id here) then those audit
records can, and should, be generated by the container runtime itself,
similar to what we do with libvirt for virtualization.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH ghak90 (was ghak32) V4 03/10] audit: log container info of syscalls
From: Casey Schaufler @ 2018-10-26 8:09 UTC (permalink / raw)
To: Steve Grubb, Paul Moore
Cc: luto, rgb, linux-api, containers, linux-kernel, viro, dhowells,
carlos, linux-audit, netfilter-devel, ebiederm, simo, netdev,
linux-fsdevel, Eric Paris, Serge Hallyn
In-Reply-To: <20181025235527.15a39d75@ivy-bridge>
On 10/25/2018 2:55 PM, Steve Grubb wrote:
> ...
> And historically speaking setting audit loginuid produces a LOGIN
> event, so it only makes sense to consider binding container ID to
> container as a CONTAINER event. For other supplemental records, we name
> things what they are: PATH, CWD, SOCKADDR, etc. So, CONTAINER_ID makes
> sense. CONTAINER_OP sounds like its for operations on a container. Do
> we have any operations on a container?
The answer has to be "no", because containers are, by emphatic assertion,
not kernel constructs. Any CONTAINER_OP event has to come from user space.
I think.
^ permalink raw reply
* Re: [PATCH] seccomp: Add pkru into seccomp_data
From: Ram Pai @ 2018-10-26 5:52 UTC (permalink / raw)
To: Florian Weimer
Cc: Will Drewry, Kees Cook, linux-api, Andy Lutomirski,
Michael Sammler, linuxppc-dev
In-Reply-To: <875zxqo0ee.fsf@oldenburg.str.redhat.com>
On Thu, Oct 25, 2018 at 11:12:25AM +0200, Florian Weimer wrote:
> * Michael Sammler:
>
> > Thank you for the pointer about the POWER implementation. I am not
> > familiar with POWER in general and its protection key feature at
> > all. Would the AMR register be the correct register to expose here?
>
> Yes, according to my notes, the register is called AMR (special purpose
> register 13).
Yes. it is AMR register.
RP
^ permalink raw reply
* Re: [PATCH] seccomp: Add pkru into seccomp_data
From: Andy Lutomirski @ 2018-10-26 0:49 UTC (permalink / raw)
To: Kees Cook
Cc: Florian Weimer, Will Drewry, Linux API, linuxram, Michael Sammler,
linuxppc-dev
In-Reply-To: <CAGXu5jLE2VBqo_rkBJ+BJkp1==WLLg=N=-3gjTCCjcN1=bodSg@mail.gmail.com>
> On Oct 25, 2018, at 5:35 PM, Kees Cook <keescook@chromium.org> wrote:
>
>> On Fri, Oct 26, 2018 at 12:00 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> You could bite the bullet and add seccomp eBPF support :)
>
> I'm not convinced this is a good enough reason for gaining the eBPF
> attack surface yet.
>
>
Is it an interesting attack surface? It’s certainly scarier if you’re worried about attacks from the sandbox creator, but the security inside the sandbox should be more or less equivalent, no?
^ permalink raw reply
* Re: [PATCH] seccomp: Add pkru into seccomp_data
From: Kees Cook @ 2018-10-26 0:35 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Florian Weimer, Will Drewry, Linux API, linuxram, Michael Sammler,
linuxppc-dev
In-Reply-To: <CALCETrX_E5_p1-MjCcdtRQbziCCzXcOCfxRBckZx_zbO1QX=Dw@mail.gmail.com>
On Fri, Oct 26, 2018 at 12:00 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> You could bite the bullet and add seccomp eBPF support :)
I'm not convinced this is a good enough reason for gaining the eBPF
attack surface yet.
-Kees
--
Kees Cook
^ permalink raw reply
* Re: [PATCH] seccomp: Add pkru into seccomp_data
From: Andy Lutomirski @ 2018-10-25 23:00 UTC (permalink / raw)
To: Michael Sammler
Cc: Florian Weimer, Will Drewry, Kees Cook, Linux API, linuxram,
linuxppc-dev
In-Reply-To: <c4d3f5c1-a2dc-5b44-4d12-1fae829a372c@mpi-sws.org>
On Thu, Oct 25, 2018 at 9:42 AM Michael Sammler <msammler@mpi-sws.org> wrote:
>
> On 10/25/2018 11:12 AM, Florian Weimer wrote:
> >> I understand your concern about exposing the number of protection keys
> >> in the ABI. One idea would be to state, that the pkru field (which
> >> should probably be renamed) contains an architecture specific value,
> >> which could then be the PKRU on x86 and AMR (or another register) on
> >> POWER. This new field should probably be extended to __u64 and the
> >> reserved field removed.
> > POWER also has proper read/write bit separation, not PKEY_DISABLE_ACCESS
> > (disable read and write) and PKEY_DISABLE_WRITE like Intel. It's
> > currently translated by the kernel, but I really need a
> > PKEY_DISABLE_READ bit in glibc to implement pkey_get in case the memory
> > is write-only.
> The idea here would be to simply provide the raw value of the register
> (PKRU on x86, AMR on POWER) to the BPF program and let the BPF program
> (or maybe a higher level library like libseccomp) deal with the
> complications of interpreting this architecture specific value (similar
> how the BPF program currently already has to deal with architecture
> specific system call numbers). If an architecture were to support more
> protection keys than fit into the field, the architecture specific value
> stored in the field might simply be the first protection keys. If there
> was interest, it would be possible to add more architecture specific
> fields to seccomp_data.
> >> Another idea would be to not add a field in the seccomp_data
> >> structure, but instead provide a new BPF instruction, which reads the
> >> value of a specified protection key.
> > I would prefer that if it's possible. We should make sure that the bits
> > are the same as those returned from pkey_get. I have an implementation
> > on POWER, but have yet to figure out the implications for 32-bit because
> > I do not know the AMR register size there.
> >
> > Thanks,
> > Florian
> I have had a look at how BPF is implemented and it does not seem to be
> easy to just add an BPF instruction for seccomp since (as far as I
> understand) the code of the classical BPF (as used by seccomp) is shared
> with the code of eBPF, which is used in many parts of the kernel and
> there is at least one interpreter and one JIT compiler for BPF. But
> maybe someone with more experience than me can comment on how hard it
> would be to add an instruction to BPF.
>
You could bite the bullet and add seccomp eBPF support :)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox