* Re: [PATCH v4] seccomp: add ptrace options for suspend/resume
From: Andy Lutomirski @ 2015-06-12 23:27 UTC (permalink / raw)
To: Kees Cook
Cc: Oleg Nesterov, Tycho Andersen,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
Will Drewry, Roland McGrath, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <CAGXu5jJovcC1S4OANpqbVe6e86xr4W9Y7897MsSDOpvEqezpnA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Jun 10, 2015 at 1:18 PM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> On Wed, Jun 10, 2015 at 10:20 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> On Wed, Jun 10, 2015 at 9:31 AM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>> On 06/09, Andy Lutomirski wrote:
>>>>
>>>> On Tue, Jun 9, 2015 at 5:49 PM, Tycho Andersen
>>>> >
>>>> > @@ -556,6 +556,15 @@ static int ptrace_setoptions(struct task_struct *child, unsigned long data)
>>>> > if (data & ~(unsigned long)PTRACE_O_MASK)
>>>> > return -EINVAL;
>>>> >
>>>> > + if (unlikely(data & PTRACE_O_SUSPEND_SECCOMP)) {
>>>
>>> Well, we should do this if
>>>
>>> (data & O_SUSPEND) && !(flags & O_SUSPEND)
>>>
>>> or at least if
>>>
>>> (data ^ flags) & O_SUSPEND
>>>
>>>
>>>> > + if (!config_enabled(CONFIG_CHECKPOINT_RESTORE) ||
>>>> > + !config_enabled(CONFIG_SECCOMP))
>>>> > + return -EINVAL;
>>>> > +
>>>> > + if (!capable(CAP_SYS_ADMIN))
>>>> > + return -EPERM;
>>>>
>>>> I tend to think that we should also require that current not be using
>>>> seccomp. Otherwise, in principle, there's a seccomp bypass for
>>>> privileged-but-seccomped programs.
>>>
>>> Andy, I simply can't understand why do we need any security check at all.
>>>
>>> OK, yes, in theory we can have a seccomped CAP_SYS_ADMIN process, seccomp
>>> doesn't filter ptrace, you hack that process and force it to attach to
>>> another CAP_SYS_ADMIN/seccomped process, etc, etc... Looks too paranoid
>>> to me.
>>
>> I've sometimes considered having privileged processes I write fork and
>> seccomp their child. Of course, if you're allowing ptrace through
>> your seccomp filter, you open a giant can of worms, but I think we
>> should take the more paranoid approach to start and relax it later as
>> needed. After all, for the intended use of this patch, stuff will
>> break regardless of what we do if the ptracer is itself seccomped.
>>
>> I could be convinced that if the ptracer is outside seccomp then we
>> shouldn't need the CAP_SYS_ADMIN check. That would at least make this
>> work in a user namespace.
>
> But not if that namespace is running under a manager that has added a
> seccomp filter to do things like drop finit_module, as lxc does.
In that case, criu isn't going to handle seccomp right regardless of
what our security check is, so I think we can safely deal with the
security aspects of that case once we figure out the functionality
part.
IOW, I think I still like the direct "you must not be seccomped in
order to suspend seccomp" rule.
--Andy
^ permalink raw reply
* Re: [PATCH v4] seccomp: add ptrace options for suspend/resume
From: Kees Cook @ 2015-06-12 23:29 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Oleg Nesterov, Tycho Andersen,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
Will Drewry, Roland McGrath, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <CALCETrUSjKVWoMM8aVeevAZRHqtiE7zOSUvjdW_yCBTnR07Ahg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jun 12, 2015 at 4:27 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Wed, Jun 10, 2015 at 1:18 PM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>> On Wed, Jun 10, 2015 at 10:20 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>> On Wed, Jun 10, 2015 at 9:31 AM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>>> On 06/09, Andy Lutomirski wrote:
>>>>>
>>>>> On Tue, Jun 9, 2015 at 5:49 PM, Tycho Andersen
>>>>> >
>>>>> > @@ -556,6 +556,15 @@ static int ptrace_setoptions(struct task_struct *child, unsigned long data)
>>>>> > if (data & ~(unsigned long)PTRACE_O_MASK)
>>>>> > return -EINVAL;
>>>>> >
>>>>> > + if (unlikely(data & PTRACE_O_SUSPEND_SECCOMP)) {
>>>>
>>>> Well, we should do this if
>>>>
>>>> (data & O_SUSPEND) && !(flags & O_SUSPEND)
>>>>
>>>> or at least if
>>>>
>>>> (data ^ flags) & O_SUSPEND
>>>>
>>>>
>>>>> > + if (!config_enabled(CONFIG_CHECKPOINT_RESTORE) ||
>>>>> > + !config_enabled(CONFIG_SECCOMP))
>>>>> > + return -EINVAL;
>>>>> > +
>>>>> > + if (!capable(CAP_SYS_ADMIN))
>>>>> > + return -EPERM;
>>>>>
>>>>> I tend to think that we should also require that current not be using
>>>>> seccomp. Otherwise, in principle, there's a seccomp bypass for
>>>>> privileged-but-seccomped programs.
>>>>
>>>> Andy, I simply can't understand why do we need any security check at all.
>>>>
>>>> OK, yes, in theory we can have a seccomped CAP_SYS_ADMIN process, seccomp
>>>> doesn't filter ptrace, you hack that process and force it to attach to
>>>> another CAP_SYS_ADMIN/seccomped process, etc, etc... Looks too paranoid
>>>> to me.
>>>
>>> I've sometimes considered having privileged processes I write fork and
>>> seccomp their child. Of course, if you're allowing ptrace through
>>> your seccomp filter, you open a giant can of worms, but I think we
>>> should take the more paranoid approach to start and relax it later as
>>> needed. After all, for the intended use of this patch, stuff will
>>> break regardless of what we do if the ptracer is itself seccomped.
>>>
>>> I could be convinced that if the ptracer is outside seccomp then we
>>> shouldn't need the CAP_SYS_ADMIN check. That would at least make this
>>> work in a user namespace.
>>
>> But not if that namespace is running under a manager that has added a
>> seccomp filter to do things like drop finit_module, as lxc does.
>
> In that case, criu isn't going to handle seccomp right regardless of
> what our security check is, so I think we can safely deal with the
> security aspects of that case once we figure out the functionality
> part.
>
> IOW, I think I still like the direct "you must not be seccomped in
> order to suspend seccomp" rule.
Adding that restriction would be fine by me.
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Alexei Starovoitov @ 2015-06-12 23:38 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David S. Miller, Ingo Molnar, Steven Rostedt, Wang Nan, Li Zefan,
Daniel Wagner, Daniel Borkmann, Linux API, Network Development,
linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrVnhCoD9=nzDpB6UvmXyGz4Th_m1fstJDyRUKzfsSaKHw@mail.gmail.com>
On 6/12/15 4:25 PM, Andy Lutomirski wrote:
> It's a dangerous tool. Also, shouldn't the returned uid match the
> namespace of the task that installed the probe, not the task that's
> being probed?
so leaking info to unprivileged apps is the concern?
The whole thing is for root only as you know.
The non-root is still far away. Today root needs to see the whole
kernel. That was the goal from the beginning.
^ permalink raw reply
* Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Andy Lutomirski @ 2015-06-12 23:47 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Steven Rostedt, Wang Nan, Li Zefan,
Daniel Wagner, Daniel Borkmann, Linux API, Network Development,
linux-kernel@vger.kernel.org
In-Reply-To: <557B6D74.2070305@plumgrid.com>
On Fri, Jun 12, 2015 at 4:38 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On 6/12/15 4:25 PM, Andy Lutomirski wrote:
>>
>> It's a dangerous tool. Also, shouldn't the returned uid match the
>> namespace of the task that installed the probe, not the task that's
>> being probed?
>
>
> so leaking info to unprivileged apps is the concern?
> The whole thing is for root only as you know.
> The non-root is still far away. Today root needs to see the whole
> kernel. That was the goal from the beginning.
>
This is more of a correctness issue than a security issue. ISTM using
current_user_ns() in a kprobe is asking for trouble. It certainly
allows any unprivilege user to show any uid it wants to the probe,
which is probably not what the installer of the probe expects.
--Andy
^ permalink raw reply
* Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Alexei Starovoitov @ 2015-06-12 23:55 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David S. Miller, Ingo Molnar, Steven Rostedt, Wang Nan, Li Zefan,
Daniel Wagner, Daniel Borkmann, Linux API, Network Development,
linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrU0OKoWKgxzUyi9A88Pm1VDfiCmgwbNycZoGbiHa6PzWA@mail.gmail.com>
On 6/12/15 4:47 PM, Andy Lutomirski wrote:
> On Fri, Jun 12, 2015 at 4:38 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>> On 6/12/15 4:25 PM, Andy Lutomirski wrote:
>>>
>>> It's a dangerous tool. Also, shouldn't the returned uid match the
>>> namespace of the task that installed the probe, not the task that's
>>> being probed?
>>
>>
>> so leaking info to unprivileged apps is the concern?
>> The whole thing is for root only as you know.
>> The non-root is still far away. Today root needs to see the whole
>> kernel. That was the goal from the beginning.
>>
>
> This is more of a correctness issue than a security issue. ISTM using
> current_user_ns() in a kprobe is asking for trouble. It certainly
> allows any unprivilege user to show any uid it wants to the probe,
> which is probably not what the installer of the probe expects.
probe doesn't expect anything. it doesn't make any decisions.
bpf is read only. it's _visibility_ into the kernel.
It's not used for security.
When we start connecting eBPF to seccomp I would agree that uid
handling needs to be done carefully, but we're not there yet.
I don't want to kill _visibility_ because in some distant future
bpf becomes a decision making tool in security area and
get_current_uid() will return numbers that shouldn't be blindly
used to reject/accept a user requesting something. That's far away.
^ permalink raw reply
* Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Andy Lutomirski @ 2015-06-13 0:03 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Steven Rostedt, Wang Nan, Li Zefan,
Daniel Wagner, Daniel Borkmann, Linux API, Network Development,
linux-kernel@vger.kernel.org
In-Reply-To: <557B718B.80604@plumgrid.com>
On Fri, Jun 12, 2015 at 4:55 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On 6/12/15 4:47 PM, Andy Lutomirski wrote:
>>
>> On Fri, Jun 12, 2015 at 4:38 PM, Alexei Starovoitov <ast@plumgrid.com>
>> wrote:
>>>
>>> On 6/12/15 4:25 PM, Andy Lutomirski wrote:
>>>>
>>>>
>>>> It's a dangerous tool. Also, shouldn't the returned uid match the
>>>> namespace of the task that installed the probe, not the task that's
>>>> being probed?
>>>
>>>
>>>
>>> so leaking info to unprivileged apps is the concern?
>>> The whole thing is for root only as you know.
>>> The non-root is still far away. Today root needs to see the whole
>>> kernel. That was the goal from the beginning.
>>>
>>
>> This is more of a correctness issue than a security issue. ISTM using
>> current_user_ns() in a kprobe is asking for trouble. It certainly
>> allows any unprivilege user to show any uid it wants to the probe,
>> which is probably not what the installer of the probe expects.
>
>
> probe doesn't expect anything. it doesn't make any decisions.
> bpf is read only. it's _visibility_ into the kernel.
> It's not used for security.
> When we start connecting eBPF to seccomp I would agree that uid
> handling needs to be done carefully, but we're not there yet.
> I don't want to kill _visibility_ because in some distant future
> bpf becomes a decision making tool in security area and
> get_current_uid() will return numbers that shouldn't be blindly
> used to reject/accept a user requesting something. That's far away.
>
All that is true, but the code that *installed* the bpf probe might
get might confused when it logs that uid 0 did such-and-such when
really some unprivileged userns root did it.
Also, as you start calling more and more non-trivial functions from
bpf, you might need to start preventing bpf probe installations in
those functions.
--Andy
^ permalink raw reply
* Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Alexei Starovoitov @ 2015-06-13 0:15 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David S. Miller, Ingo Molnar, Steven Rostedt, Wang Nan, Li Zefan,
Daniel Wagner, Daniel Borkmann, Linux API, Network Development,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CALCETrWYHkBtVr5D0zdrXswoH2gSXsxhtD23w6J7EP=HEwPt8A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 6/12/15 5:03 PM, Andy Lutomirski wrote:
> On Fri, Jun 12, 2015 at 4:55 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>> On 6/12/15 4:47 PM, Andy Lutomirski wrote:
>>>
>>> On Fri, Jun 12, 2015 at 4:38 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
>>> wrote:
>>>>
>>>> On 6/12/15 4:25 PM, Andy Lutomirski wrote:
>>>>>
>>>>>
>>>>> It's a dangerous tool. Also, shouldn't the returned uid match the
>>>>> namespace of the task that installed the probe, not the task that's
>>>>> being probed?
>>>>
>>>>
>>>>
>>>> so leaking info to unprivileged apps is the concern?
>>>> The whole thing is for root only as you know.
>>>> The non-root is still far away. Today root needs to see the whole
>>>> kernel. That was the goal from the beginning.
>>>>
>>>
>>> This is more of a correctness issue than a security issue. ISTM using
>>> current_user_ns() in a kprobe is asking for trouble. It certainly
>>> allows any unprivilege user to show any uid it wants to the probe,
>>> which is probably not what the installer of the probe expects.
>>
>>
>> probe doesn't expect anything. it doesn't make any decisions.
>> bpf is read only. it's _visibility_ into the kernel.
>> It's not used for security.
>> When we start connecting eBPF to seccomp I would agree that uid
>> handling needs to be done carefully, but we're not there yet.
>> I don't want to kill _visibility_ because in some distant future
>> bpf becomes a decision making tool in security area and
>> get_current_uid() will return numbers that shouldn't be blindly
>> used to reject/accept a user requesting something. That's far away.
>>
>
> All that is true, but the code that *installed* the bpf probe might
> get might confused when it logs that uid 0 did such-and-such when
> really some unprivileged userns root did it.
so what specifically you proposing?
Use from_kuid(&init_user_ns,...) instead?
> Also, as you start calling more and more non-trivial functions from
> bpf, you might need to start preventing bpf probe installations in
> those functions.
yes. may be. I don't want to blacklist stuff yet, unless it
causes crashes. Recursive check is already there. Probably
something else will be needed.
^ permalink raw reply
* Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Andy Lutomirski @ 2015-06-13 0:24 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Steven Rostedt, Wang Nan, Li Zefan,
Daniel Wagner, Daniel Borkmann, Linux API, Network Development,
linux-kernel@vger.kernel.org
In-Reply-To: <557B763F.7000003@plumgrid.com>
On Fri, Jun 12, 2015 at 5:15 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On 6/12/15 5:03 PM, Andy Lutomirski wrote:
>>
>> On Fri, Jun 12, 2015 at 4:55 PM, Alexei Starovoitov <ast@plumgrid.com>
>> wrote:
>>>
>>> On 6/12/15 4:47 PM, Andy Lutomirski wrote:
>>>>
>>>>
>>>> On Fri, Jun 12, 2015 at 4:38 PM, Alexei Starovoitov <ast@plumgrid.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 6/12/15 4:25 PM, Andy Lutomirski wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> It's a dangerous tool. Also, shouldn't the returned uid match the
>>>>>> namespace of the task that installed the probe, not the task that's
>>>>>> being probed?
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> so leaking info to unprivileged apps is the concern?
>>>>> The whole thing is for root only as you know.
>>>>> The non-root is still far away. Today root needs to see the whole
>>>>> kernel. That was the goal from the beginning.
>>>>>
>>>>
>>>> This is more of a correctness issue than a security issue. ISTM using
>>>> current_user_ns() in a kprobe is asking for trouble. It certainly
>>>> allows any unprivilege user to show any uid it wants to the probe,
>>>> which is probably not what the installer of the probe expects.
>>>
>>>
>>>
>>> probe doesn't expect anything. it doesn't make any decisions.
>>> bpf is read only. it's _visibility_ into the kernel.
>>> It's not used for security.
>>> When we start connecting eBPF to seccomp I would agree that uid
>>> handling needs to be done carefully, but we're not there yet.
>>> I don't want to kill _visibility_ because in some distant future
>>> bpf becomes a decision making tool in security area and
>>> get_current_uid() will return numbers that shouldn't be blindly
>>> used to reject/accept a user requesting something. That's far away.
>>>
>>
>> All that is true, but the code that *installed* the bpf probe might
>> get might confused when it logs that uid 0 did such-and-such when
>> really some unprivileged userns root did it.
>
>
> so what specifically you proposing?
> Use from_kuid(&init_user_ns,...) instead?
That seems reasonable to me. After all, you can't install one of
these probes from a non-init userns.
--Andy
^ permalink raw reply
* Re: [PATCH net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Alexei Starovoitov @ 2015-06-13 0:26 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David S. Miller, Ingo Molnar, Steven Rostedt, Wang Nan, Li Zefan,
Daniel Wagner, Daniel Borkmann, Linux API, Network Development,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CALCETrWUuoZz2XOox5FJJPAfFdBakLnY1d7LLP5Uxq9tqgkpzg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 6/12/15 5:24 PM, Andy Lutomirski wrote:
>> >so what specifically you proposing?
>> >Use from_kuid(&init_user_ns,...) instead?
> That seems reasonable to me. After all, you can't install one of
> these probes from a non-init userns.
ok. will respin with that change.
^ permalink raw reply
* [PATCH v2 net-next 0/3] bpf: share helpers between tracing and networking
From: Alexei Starovoitov @ 2015-06-13 2:39 UTC (permalink / raw)
To: David S. Miller
Cc: Andy Lutomirski, Ingo Molnar, Steven Rostedt, Wang Nan,
lizefan-hv44wF8Li93QT0dZR+AlfA, Daniel Wagner, Daniel Borkmann,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
v1->v2: switched to init_user_ns from current_user_ns as suggested by Andy
Introduce new helpers to access 'struct task_struct'->pid, tgid, uid, gid, comm
fields in tracing and networking.
Share bpf_trace_printk() and bpf_get_smp_processor_id() helpers between
tracing and networking.
Alexei Starovoitov (3):
bpf: introduce current->pid, tgid, uid, gid, comm accessors
bpf: allow networking programs to use bpf_trace_printk() for
debugging
bpf: let kprobe programs use bpf_get_smp_processor_id() helper
include/linux/bpf.h | 4 +++
include/uapi/linux/bpf.h | 19 +++++++++++++
kernel/bpf/core.c | 7 +++++
kernel/bpf/helpers.c | 58 ++++++++++++++++++++++++++++++++++++++
kernel/trace/bpf_trace.c | 28 ++++++++++++------
net/core/filter.c | 8 ++++++
samples/bpf/bpf_helpers.h | 6 ++++
samples/bpf/tracex2_kern.c | 24 ++++++++++++----
samples/bpf/tracex2_user.c | 67 ++++++++++++++++++++++++++++++++++++++------
9 files changed, 199 insertions(+), 22 deletions(-)
--
1.7.9.5
^ permalink raw reply
* [PATCH v2 net-next 1/3] bpf: introduce current->pid, tgid, uid, gid, comm accessors
From: Alexei Starovoitov @ 2015-06-13 2:39 UTC (permalink / raw)
To: David S. Miller
Cc: Andy Lutomirski, Ingo Molnar, Steven Rostedt, Wang Nan,
lizefan-hv44wF8Li93QT0dZR+AlfA, Daniel Wagner, Daniel Borkmann,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1434163154-5218-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
eBPF programs attached to kprobes need to filter based on
current->pid, uid and other fields, so introduce helper functions:
u64 bpf_get_current_pid_tgid(void)
Return: current->tgid << 32 | current->pid
u64 bpf_get_current_uid_gid(void)
Return: current_gid << 32 | current_uid
bpf_get_current_comm(char *buf, int size_of_buf)
stores current->comm into buf
They can be used from the programs attached to TC as well to classify packets
based on current task fields.
Update tracex2 example to print histogram of write syscalls for each process
instead of aggregated for all.
Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
---
v1->v2: switched to init_user_ns from current_user_ns as suggested by Andy
These helpers will be mainly used by bpf+tracing, but the patch is targeting
net-next tree to minimize merge conflicts and they're useful in TC too.
The feature was requested by Wang Nan <wangnan0-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> and
Brendan Gregg <brendan.d.gregg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
We've considered several alternatives:
1: 5 different helpers
Cons: every call adds performance overhead
2a: single helper that populates 'struct bpf_task_info'
and uses 'flags' with bit per field.
struct bpf_task_info {
__u32 pid;
__u32 tgid;
__u32 uid;
__u32 gid;
char comm[16];
};
bpf_get_current_task_info(task_info, size, flags)
bit 0 - fill in pid
bit 1 - fill in tgid
Pros: single helper.
Cons: not easy to use and a lot of compares in the helper itself
(two compares for each field).
2b. single helper that populates 'struct bpf_task_info'
and uses 'size' to tell how many fields to fill in.
bpf_get_current_task_info(task_info, size);
if (size >= offsetof(struct bpf_task_info, pid) + sizeof(info->pid))
info->pid = task->pid;
if (size >= offsetof(struct bpf_task_info, tgid) + sizeof(info->tgid))
info->tgid = task->tgid;
Pros: single call (with single compare per field).
Cons: still hard to use when only some middle field (like uid) is needed.
These three helpers looks as the best balance between performance and usability.
include/linux/bpf.h | 3 ++
include/uapi/linux/bpf.h | 19 +++++++++++++
kernel/bpf/core.c | 3 ++
kernel/bpf/helpers.c | 58 ++++++++++++++++++++++++++++++++++++++
kernel/trace/bpf_trace.c | 6 ++++
net/core/filter.c | 6 ++++
samples/bpf/bpf_helpers.h | 6 ++++
samples/bpf/tracex2_kern.c | 24 ++++++++++++----
samples/bpf/tracex2_user.c | 67 ++++++++++++++++++++++++++++++++++++++------
9 files changed, 178 insertions(+), 14 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 2235aee8096a..1b9a3f5b27f6 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -188,5 +188,8 @@ extern const struct bpf_func_proto bpf_get_prandom_u32_proto;
extern const struct bpf_func_proto bpf_get_smp_processor_id_proto;
extern const struct bpf_func_proto bpf_tail_call_proto;
extern const struct bpf_func_proto bpf_ktime_get_ns_proto;
+extern const struct bpf_func_proto bpf_get_current_pid_tgid_proto;
+extern const struct bpf_func_proto bpf_get_current_uid_gid_proto;
+extern const struct bpf_func_proto bpf_get_current_comm_proto;
#endif /* _LINUX_BPF_H */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 602f05b7a275..29ef6f99e43d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -230,6 +230,25 @@ enum bpf_func_id {
* Return: 0 on success
*/
BPF_FUNC_clone_redirect,
+
+ /**
+ * u64 bpf_get_current_pid_tgid(void)
+ * Return: current->tgid << 32 | current->pid
+ */
+ BPF_FUNC_get_current_pid_tgid,
+
+ /**
+ * u64 bpf_get_current_uid_gid(void)
+ * Return: current_gid << 32 | current_uid
+ */
+ BPF_FUNC_get_current_uid_gid,
+
+ /**
+ * bpf_get_current_comm(char *buf, int size_of_buf)
+ * stores current->comm into buf
+ * Return: 0 on success
+ */
+ BPF_FUNC_get_current_comm,
__BPF_FUNC_MAX_ID,
};
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 1e00aa3316dc..1fc45cc83076 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -730,6 +730,9 @@ const struct bpf_func_proto bpf_map_delete_elem_proto __weak;
const struct bpf_func_proto bpf_get_prandom_u32_proto __weak;
const struct bpf_func_proto bpf_get_smp_processor_id_proto __weak;
const struct bpf_func_proto bpf_ktime_get_ns_proto __weak;
+const struct bpf_func_proto bpf_get_current_pid_tgid_proto __weak;
+const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
+const struct bpf_func_proto bpf_get_current_comm_proto __weak;
/* Always built-in helper functions. */
const struct bpf_func_proto bpf_tail_call_proto = {
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 7ad5d8842d5b..1447ec09421e 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -14,6 +14,8 @@
#include <linux/random.h>
#include <linux/smp.h>
#include <linux/ktime.h>
+#include <linux/sched.h>
+#include <linux/uidgid.h>
/* If kernel subsystem is allowing eBPF programs to call this function,
* inside its own verifier_ops->get_func_proto() callback it should return
@@ -124,3 +126,59 @@ const struct bpf_func_proto bpf_ktime_get_ns_proto = {
.gpl_only = true,
.ret_type = RET_INTEGER,
};
+
+static u64 bpf_get_current_pid_tgid(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
+{
+ struct task_struct *task = current;
+
+ if (!task)
+ return -EINVAL;
+
+ return (u64) task->tgid << 32 | task->pid;
+}
+
+const struct bpf_func_proto bpf_get_current_pid_tgid_proto = {
+ .func = bpf_get_current_pid_tgid,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+};
+
+static u64 bpf_get_current_uid_gid(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
+{
+ struct task_struct *task = current;
+ kuid_t uid;
+ kgid_t gid;
+
+ if (!task)
+ return -EINVAL;
+
+ current_uid_gid(&uid, &gid);
+ return (u64) from_kgid(&init_user_ns, gid) << 32 |
+ from_kuid(&init_user_ns, uid);
+}
+
+const struct bpf_func_proto bpf_get_current_uid_gid_proto = {
+ .func = bpf_get_current_uid_gid,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+};
+
+static u64 bpf_get_current_comm(u64 r1, u64 size, u64 r3, u64 r4, u64 r5)
+{
+ struct task_struct *task = current;
+ char *buf = (char *) (long) r1;
+
+ if (!task)
+ return -EINVAL;
+
+ memcpy(buf, task->comm, min_t(size_t, size, sizeof(task->comm)));
+ return 0;
+}
+
+const struct bpf_func_proto bpf_get_current_comm_proto = {
+ .func = bpf_get_current_comm,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_STACK,
+ .arg2_type = ARG_CONST_STACK_SIZE,
+};
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 50c4015a8ad3..3a17638cdf46 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -162,6 +162,12 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
return &bpf_ktime_get_ns_proto;
case BPF_FUNC_tail_call:
return &bpf_tail_call_proto;
+ case BPF_FUNC_get_current_pid_tgid:
+ return &bpf_get_current_pid_tgid_proto;
+ case BPF_FUNC_get_current_uid_gid:
+ return &bpf_get_current_uid_gid_proto;
+ case BPF_FUNC_get_current_comm:
+ return &bpf_get_current_comm_proto;
case BPF_FUNC_trace_printk:
/*
diff --git a/net/core/filter.c b/net/core/filter.c
index d271c06bf01f..20aa51ccbf9d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1459,6 +1459,12 @@ tc_cls_act_func_proto(enum bpf_func_id func_id)
return &bpf_l4_csum_replace_proto;
case BPF_FUNC_clone_redirect:
return &bpf_clone_redirect_proto;
+ case BPF_FUNC_get_current_pid_tgid:
+ return &bpf_get_current_pid_tgid_proto;
+ case BPF_FUNC_get_current_uid_gid:
+ return &bpf_get_current_uid_gid_proto;
+ case BPF_FUNC_get_current_comm:
+ return &bpf_get_current_comm_proto;
default:
return sk_filter_func_proto(func_id);
}
diff --git a/samples/bpf/bpf_helpers.h b/samples/bpf/bpf_helpers.h
index f531a0b3282d..bdf1c1607b80 100644
--- a/samples/bpf/bpf_helpers.h
+++ b/samples/bpf/bpf_helpers.h
@@ -25,6 +25,12 @@ static void (*bpf_tail_call)(void *ctx, void *map, int index) =
(void *) BPF_FUNC_tail_call;
static unsigned long long (*bpf_get_smp_processor_id)(void) =
(void *) BPF_FUNC_get_smp_processor_id;
+static unsigned long long (*bpf_get_current_pid_tgid)(void) =
+ (void *) BPF_FUNC_get_current_pid_tgid;
+static unsigned long long (*bpf_get_current_uid_gid)(void) =
+ (void *) BPF_FUNC_get_current_uid_gid;
+static int (*bpf_get_current_comm)(void *buf, int buf_size) =
+ (void *) BPF_FUNC_get_current_comm;
/* llvm builtin functions that eBPF C program may use to
* emit BPF_LD_ABS and BPF_LD_IND instructions
diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c
index 19ec1cfc45db..dc50f4f2943f 100644
--- a/samples/bpf/tracex2_kern.c
+++ b/samples/bpf/tracex2_kern.c
@@ -62,11 +62,18 @@ static unsigned int log2l(unsigned long v)
return log2(v);
}
+struct hist_key {
+ char comm[16];
+ u64 pid_tgid;
+ u64 uid_gid;
+ u32 index;
+};
+
struct bpf_map_def SEC("maps") my_hist_map = {
- .type = BPF_MAP_TYPE_ARRAY,
- .key_size = sizeof(u32),
+ .type = BPF_MAP_TYPE_HASH,
+ .key_size = sizeof(struct hist_key),
.value_size = sizeof(long),
- .max_entries = 64,
+ .max_entries = 1024,
};
SEC("kprobe/sys_write")
@@ -75,11 +82,18 @@ int bpf_prog3(struct pt_regs *ctx)
long write_size = ctx->dx; /* arg3 */
long init_val = 1;
long *value;
- u32 index = log2l(write_size);
+ struct hist_key key = {};
+
+ key.index = log2l(write_size);
+ key.pid_tgid = bpf_get_current_pid_tgid();
+ key.uid_gid = bpf_get_current_uid_gid();
+ bpf_get_current_comm(&key.comm, sizeof(key.comm));
- value = bpf_map_lookup_elem(&my_hist_map, &index);
+ value = bpf_map_lookup_elem(&my_hist_map, &key);
if (value)
__sync_fetch_and_add(value, 1);
+ else
+ bpf_map_update_elem(&my_hist_map, &key, &init_val, BPF_ANY);
return 0;
}
char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/tracex2_user.c b/samples/bpf/tracex2_user.c
index 91b8d0896fbb..cd0241c1447a 100644
--- a/samples/bpf/tracex2_user.c
+++ b/samples/bpf/tracex2_user.c
@@ -3,6 +3,7 @@
#include <stdlib.h>
#include <signal.h>
#include <linux/bpf.h>
+#include <string.h>
#include "libbpf.h"
#include "bpf_load.h"
@@ -20,23 +21,42 @@ static void stars(char *str, long val, long max, int width)
str[i] = '\0';
}
-static void print_hist(int fd)
+struct task {
+ char comm[16];
+ __u64 pid_tgid;
+ __u64 uid_gid;
+};
+
+struct hist_key {
+ struct task t;
+ __u32 index;
+};
+
+#define SIZE sizeof(struct task)
+
+static void print_hist_for_pid(int fd, void *task)
{
- int key;
+ struct hist_key key = {}, next_key;
+ char starstr[MAX_STARS];
long value;
long data[MAX_INDEX] = {};
- char starstr[MAX_STARS];
- int i;
int max_ind = -1;
long max_value = 0;
+ int i, ind;
- for (key = 0; key < MAX_INDEX; key++) {
- bpf_lookup_elem(fd, &key, &value);
- data[key] = value;
- if (value && key > max_ind)
- max_ind = key;
+ while (bpf_get_next_key(fd, &key, &next_key) == 0) {
+ if (memcmp(&next_key, task, SIZE)) {
+ key = next_key;
+ continue;
+ }
+ bpf_lookup_elem(fd, &next_key, &value);
+ ind = next_key.index;
+ data[ind] = value;
+ if (value && ind > max_ind)
+ max_ind = ind;
if (value > max_value)
max_value = value;
+ key = next_key;
}
printf(" syscall write() stats\n");
@@ -48,6 +68,35 @@ static void print_hist(int fd)
MAX_STARS, starstr);
}
}
+
+static void print_hist(int fd)
+{
+ struct hist_key key = {}, next_key;
+ static struct task tasks[1024];
+ int task_cnt = 0;
+ int i;
+
+ while (bpf_get_next_key(fd, &key, &next_key) == 0) {
+ int found = 0;
+
+ for (i = 0; i < task_cnt; i++)
+ if (memcmp(&tasks[i], &next_key, SIZE) == 0)
+ found = 1;
+ if (!found)
+ memcpy(&tasks[task_cnt++], &next_key, SIZE);
+ key = next_key;
+ }
+
+ for (i = 0; i < task_cnt; i++) {
+ printf("\npid %d cmd %s uid %d\n",
+ (__u32) tasks[i].pid_tgid,
+ tasks[i].comm,
+ (__u32) tasks[i].uid_gid);
+ print_hist_for_pid(fd, &tasks[i]);
+ }
+
+}
+
static void int_exit(int sig)
{
print_hist(map_fd[1]);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 net-next 2/3] bpf: allow networking programs to use bpf_trace_printk() for debugging
From: Alexei Starovoitov @ 2015-06-13 2:39 UTC (permalink / raw)
To: David S. Miller
Cc: Andy Lutomirski, Ingo Molnar, Steven Rostedt, Wang Nan, lizefan,
Daniel Wagner, Daniel Borkmann, linux-api, netdev, linux-kernel
In-Reply-To: <1434163154-5218-1-git-send-email-ast@plumgrid.com>
bpf_trace_printk() is a helper function used to debug eBPF programs.
Let socket and TC programs use it as well.
Note, it's DEBUG ONLY helper. If it's used in the program,
the kernel will print warning banner to make sure users don't use
it in production.
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
v1->v2: no changes
include/linux/bpf.h | 1 +
kernel/bpf/core.c | 4 ++++
kernel/trace/bpf_trace.c | 20 ++++++++++++--------
net/core/filter.c | 2 ++
4 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 1b9a3f5b27f6..4383476a0d48 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -150,6 +150,7 @@ struct bpf_array {
u64 bpf_tail_call(u64 ctx, u64 r2, u64 index, u64 r4, u64 r5);
void bpf_prog_array_map_clear(struct bpf_map *map);
bool bpf_prog_array_compatible(struct bpf_array *array, const struct bpf_prog *fp);
+const struct bpf_func_proto *bpf_get_trace_printk_proto(void);
#ifdef CONFIG_BPF_SYSCALL
void bpf_register_prog_type(struct bpf_prog_type_list *tl);
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 1fc45cc83076..c5bedc82bc1c 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -733,6 +733,10 @@ const struct bpf_func_proto bpf_ktime_get_ns_proto __weak;
const struct bpf_func_proto bpf_get_current_pid_tgid_proto __weak;
const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
const struct bpf_func_proto bpf_get_current_comm_proto __weak;
+const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
+{
+ return NULL;
+}
/* Always built-in helper functions. */
const struct bpf_func_proto bpf_tail_call_proto = {
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 3a17638cdf46..4f9b5d41869b 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -147,6 +147,17 @@ static const struct bpf_func_proto bpf_trace_printk_proto = {
.arg2_type = ARG_CONST_STACK_SIZE,
};
+const struct bpf_func_proto *bpf_get_trace_printk_proto(void)
+{
+ /*
+ * this program might be calling bpf_trace_printk,
+ * so allocate per-cpu printk buffers
+ */
+ trace_printk_init_buffers();
+
+ return &bpf_trace_printk_proto;
+}
+
static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func_id)
{
switch (func_id) {
@@ -168,15 +179,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
return &bpf_get_current_uid_gid_proto;
case BPF_FUNC_get_current_comm:
return &bpf_get_current_comm_proto;
-
case BPF_FUNC_trace_printk:
- /*
- * this program might be calling bpf_trace_printk,
- * so allocate per-cpu printk buffers
- */
- trace_printk_init_buffers();
-
- return &bpf_trace_printk_proto;
+ return bpf_get_trace_printk_proto();
default:
return NULL;
}
diff --git a/net/core/filter.c b/net/core/filter.c
index 20aa51ccbf9d..65ff107d3d29 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1442,6 +1442,8 @@ sk_filter_func_proto(enum bpf_func_id func_id)
return &bpf_tail_call_proto;
case BPF_FUNC_ktime_get_ns:
return &bpf_ktime_get_ns_proto;
+ case BPF_FUNC_trace_printk:
+ return bpf_get_trace_printk_proto();
default:
return NULL;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 net-next 3/3] bpf: let kprobe programs use bpf_get_smp_processor_id() helper
From: Alexei Starovoitov @ 2015-06-13 2:39 UTC (permalink / raw)
To: David S. Miller
Cc: Andy Lutomirski, Ingo Molnar, Steven Rostedt, Wang Nan, lizefan,
Daniel Wagner, Daniel Borkmann, linux-api, netdev, linux-kernel
In-Reply-To: <1434163154-5218-1-git-send-email-ast@plumgrid.com>
It's useful to do per-cpu histograms.
Suggested-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
v1->v2: no changes
kernel/trace/bpf_trace.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 4f9b5d41869b..88a041adee90 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -181,6 +181,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
return &bpf_get_current_comm_proto;
case BPF_FUNC_trace_printk:
return bpf_get_trace_printk_proto();
+ case BPF_FUNC_get_smp_processor_id:
+ return &bpf_get_smp_processor_id_proto;
default:
return NULL;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2 net-next 3/3] bpf: let kprobe programs use bpf_get_smp_processor_id() helper
From: Daniel Borkmann @ 2015-06-13 8:23 UTC (permalink / raw)
To: Alexei Starovoitov, David S. Miller
Cc: Andy Lutomirski, Ingo Molnar, Steven Rostedt, Wang Nan,
lizefan-hv44wF8Li93QT0dZR+AlfA, Daniel Wagner,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1434163154-5218-4-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
On 06/13/2015 04:39 AM, Alexei Starovoitov wrote:
> It's useful to do per-cpu histograms.
>
> Suggested-by: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
> Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Acked-by: Daniel Borkmann <daniel-FeC+5ew28dpmcu3hnIyYJQ@public.gmane.org>
^ permalink raw reply
* [PATCH v5] seccomp: add ptrace options for suspend/resume
From: Tycho Andersen @ 2015-06-13 15:02 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
Cc: Tycho Andersen, Kees Cook, Andy Lutomirski, Will Drewry,
Roland McGrath, Oleg Nesterov, Pavel Emelyanov, Serge E. Hallyn
This patch is the first step in enabling checkpoint/restore of processes
with seccomp enabled.
One of the things CRIU does while dumping tasks is inject code into them
via ptrace to collect information that is only available to the process
itself. However, if we are in a seccomp mode where these processes are
prohibited from making these syscalls, then what CRIU does kills the task.
This patch adds a new ptrace option, PTRACE_O_SUSPEND_SECCOMP, that enables
a task from the init user namespace which has CAP_SYS_ADMIN and no seccomp
filters to disable (and re-enable) seccomp filters for another task so that
they can be successfully dumped (and restored). We restrict the set of
processes that can disable seccomp through ptrace because although today
ptrace can be used to bypass seccomp, there is some discussion of closing
this loophole in the future and we would like this patch to not depend on
that behavior and be future proofed for when it is removed.
Note that seccomp can be suspended before any filters are actually
installed; this behavior is useful on criu restore, so that we can suspend
seccomp, restore the filters, unmap our restore code from the restored
process' address space, and then resume the task by detaching and have the
filters resumed as well.
v2 changes:
* require that the tracer have no seccomp filters installed
* drop TIF_NOTSC manipulation from the patch
* change from ptrace command to a ptrace option and use this ptrace option
as the flag to check. This means that as soon as the tracer
detaches/dies, seccomp is re-enabled and as a corrollary that one can not
disable seccomp across PTRACE_ATTACHs.
v3 changes:
* get rid of various #ifdefs everywhere
* report more sensible errors when PTRACE_O_SUSPEND_SECCOMP is incorrectly
used
v4 changes:
* get rid of may_suspend_seccomp() in favor of a capable() check in ptrace
directly
v5 changes:
* check that seccomp is not enabled (or suspended) on the tracer
Signed-off-by: Tycho Andersen <tycho.andersen-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
CC: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
CC: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
CC: Will Drewry <wad-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
CC: Roland McGrath <roland-/Z5OmTQCD9xF6kxbq+BtvQ@public.gmane.org>
CC: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
CC: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
CC: Serge E. Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
---
include/linux/ptrace.h | 1 +
include/uapi/linux/ptrace.h | 6 ++++--
kernel/ptrace.c | 13 +++++++++++++
kernel/seccomp.c | 8 ++++++++
4 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 987a73a..061265f 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -34,6 +34,7 @@
#define PT_TRACE_SECCOMP PT_EVENT_FLAG(PTRACE_EVENT_SECCOMP)
#define PT_EXITKILL (PTRACE_O_EXITKILL << PT_OPT_FLAG_SHIFT)
+#define PT_SUSPEND_SECCOMP (PTRACE_O_SUSPEND_SECCOMP << PT_OPT_FLAG_SHIFT)
/* single stepping state bits (used on ARM and PA-RISC) */
#define PT_SINGLESTEP_BIT 31
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index cf1019e..a7a6979 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -89,9 +89,11 @@ struct ptrace_peeksiginfo_args {
#define PTRACE_O_TRACESECCOMP (1 << PTRACE_EVENT_SECCOMP)
/* eventless options */
-#define PTRACE_O_EXITKILL (1 << 20)
+#define PTRACE_O_EXITKILL (1 << 20)
+#define PTRACE_O_SUSPEND_SECCOMP (1 << 21)
-#define PTRACE_O_MASK (0x000000ff | PTRACE_O_EXITKILL)
+#define PTRACE_O_MASK (\
+ 0x000000ff | PTRACE_O_EXITKILL | PTRACE_O_SUSPEND_SECCOMP)
#include <asm/ptrace.h>
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index c8e0e05..496028b 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -556,6 +556,19 @@ static int ptrace_setoptions(struct task_struct *child, unsigned long data)
if (data & ~(unsigned long)PTRACE_O_MASK)
return -EINVAL;
+ if (unlikely(data & PTRACE_O_SUSPEND_SECCOMP)) {
+ if (!config_enabled(CONFIG_CHECKPOINT_RESTORE) ||
+ !config_enabled(CONFIG_SECCOMP))
+ return -EINVAL;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ if (current->seccomp.mode != SECCOMP_MODE_DISABLED ||
+ current->ptrace & PT_SUSPEND_SECCOMP)
+ return -EPERM;
+ }
+
/* Avoid intermediate state when all opts are cleared */
flags = child->ptrace;
flags &= ~(PTRACE_O_MASK << PT_OPT_FLAG_SHIFT);
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 980fd26..645e42d 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -590,6 +590,10 @@ void secure_computing_strict(int this_syscall)
{
int mode = current->seccomp.mode;
+ if (config_enabled(CONFIG_CHECKPOINT_RESTORE) &&
+ unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
+ return;
+
if (mode == 0)
return;
else if (mode == SECCOMP_MODE_STRICT)
@@ -691,6 +695,10 @@ u32 seccomp_phase1(struct seccomp_data *sd)
int this_syscall = sd ? sd->nr :
syscall_get_nr(current, task_pt_regs(current));
+ if (config_enabled(CONFIG_CHECKPOINT_RESTORE) &&
+ unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
+ return SECCOMP_PHASE1_OK;
+
switch (mode) {
case SECCOMP_MODE_STRICT:
__secure_computing_strict(this_syscall); /* may call do_exit */
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v4] seccomp: add ptrace options for suspend/resume
From: Tycho Andersen @ 2015-06-13 15:06 UTC (permalink / raw)
To: Kees Cook
Cc: Andy Lutomirski, Oleg Nesterov,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
Will Drewry, Roland McGrath, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <CAGXu5jKD8nj-E87em6BqS_TAeca4-kqL2u7jqYFompzqPrvqfA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jun 12, 2015 at 04:29:00PM -0700, Kees Cook wrote:
> On Fri, Jun 12, 2015 at 4:27 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> > On Wed, Jun 10, 2015 at 1:18 PM, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> >> On Wed, Jun 10, 2015 at 10:20 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> >>> On Wed, Jun 10, 2015 at 9:31 AM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >>>> On 06/09, Andy Lutomirski wrote:
> >>>>>
> >>>>> On Tue, Jun 9, 2015 at 5:49 PM, Tycho Andersen
> >>>>> >
> >>>>> > @@ -556,6 +556,15 @@ static int ptrace_setoptions(struct task_struct *child, unsigned long data)
> >>>>> > if (data & ~(unsigned long)PTRACE_O_MASK)
> >>>>> > return -EINVAL;
> >>>>> >
> >>>>> > + if (unlikely(data & PTRACE_O_SUSPEND_SECCOMP)) {
> >>>>
> >>>> Well, we should do this if
> >>>>
> >>>> (data & O_SUSPEND) && !(flags & O_SUSPEND)
> >>>>
> >>>> or at least if
> >>>>
> >>>> (data ^ flags) & O_SUSPEND
> >>>>
> >>>>
> >>>>> > + if (!config_enabled(CONFIG_CHECKPOINT_RESTORE) ||
> >>>>> > + !config_enabled(CONFIG_SECCOMP))
> >>>>> > + return -EINVAL;
> >>>>> > +
> >>>>> > + if (!capable(CAP_SYS_ADMIN))
> >>>>> > + return -EPERM;
> >>>>>
> >>>>> I tend to think that we should also require that current not be using
> >>>>> seccomp. Otherwise, in principle, there's a seccomp bypass for
> >>>>> privileged-but-seccomped programs.
> >>>>
> >>>> Andy, I simply can't understand why do we need any security check at all.
> >>>>
> >>>> OK, yes, in theory we can have a seccomped CAP_SYS_ADMIN process, seccomp
> >>>> doesn't filter ptrace, you hack that process and force it to attach to
> >>>> another CAP_SYS_ADMIN/seccomped process, etc, etc... Looks too paranoid
> >>>> to me.
> >>>
> >>> I've sometimes considered having privileged processes I write fork and
> >>> seccomp their child. Of course, if you're allowing ptrace through
> >>> your seccomp filter, you open a giant can of worms, but I think we
> >>> should take the more paranoid approach to start and relax it later as
> >>> needed. After all, for the intended use of this patch, stuff will
> >>> break regardless of what we do if the ptracer is itself seccomped.
> >>>
> >>> I could be convinced that if the ptracer is outside seccomp then we
> >>> shouldn't need the CAP_SYS_ADMIN check. That would at least make this
> >>> work in a user namespace.
> >>
> >> But not if that namespace is running under a manager that has added a
> >> seccomp filter to do things like drop finit_module, as lxc does.
> >
> > In that case, criu isn't going to handle seccomp right regardless of
> > what our security check is, so I think we can safely deal with the
> > security aspects of that case once we figure out the functionality
> > part.
> >
> > IOW, I think I still like the direct "you must not be seccomped in
> > order to suspend seccomp" rule.
>
> Adding that restriction would be fine by me.
Ok, I just sent v5 with this change. I didn't carry your ack in the
hopes that I could get you to take this patch in the seccomp tree. Let
me know if that's not the right thing to do.
Thanks,
Tycho
^ permalink raw reply
* Re: [RFC PATCH 07/18] kthread: Make iterant kthreads freezable by default
From: Tejun Heo @ 2015-06-13 23:22 UTC (permalink / raw)
To: Petr Mladek
Cc: Andrew Morton, Oleg Nesterov, Ingo Molnar, Peter Zijlstra,
Richard Weinberger, Steven Rostedt, David Woodhouse,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Trond Myklebust,
Anna Schumaker, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Chris Mason,
Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
Borislav Petkov, Michal Hocko,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150612132440.GH9409-KsEp0d+Q8qECVLCxKZUutA@public.gmane.org>
Hey, Petr.
On Fri, Jun 12, 2015 at 03:24:40PM +0200, Petr Mladek wrote:
> > * While hibernating, freezing writeback workers and whoever else which
> > might issue IOs. This is because we have to thaw devices to issue
> > IOs to write out the prepared hibernation image. If new IOs are
> > issued from page cache or whereever when the devices are thawed for
> > writing out the hibernation image, the hibernation image and the
> > data on the disk deviate.
>
> Just an idea. I do not say that it is a good solution. If we already
> thaw devices needed to write the data. It should be possible to thaw
> also kthreads needed to write the data.
Sure, we'd end up having to mark the involved kthreads and whatever
they may depend on with "freeze for snapshotting but not for writing
out images".
> > Note that this only works because currently none of the block
> > drivers which may be used to write out hibernation images depend on
> > freezable kthreads and hopefully nobody issues IOs from unfreezable
> > kthreads or bh or softirq, which, I think, can happen with
> > preallocated requests or bio based devices.
>
> It means that some kthreads must be stopped, some must be available,
> and we do not mind about the rest. I wonder which group is bigger.
> It might help to decide about the more safe default. It is not easy
> for me to decide :-/
Please do not spread freezer more than necessary. It's not about
which part is larger but about not completely losing track of what's
going on. At least now we can say "this needs freezer because XYZ"
and that XYZ actually points to something which needs to be
synchronized for PM operations. If we flip the polarity, all we're
left with is "this can't be frozen because it deadlocks with XYZ"
which is a lot less information, both in terms of relevance and
amount, than the other way around.
> > This is a very brittle approach. Instead of controlling things
> > where it actually can be controlled - the IO ingress point - we're
> > trying to control all its users with a pretty blunt tool while at
> > the same time depending on the fact that some of the low level
> > subsystems don't happen to make use of the said blunt tool.
> >
> > I think what we should do is simply blocking all non-image IOs from
> > the block layer while writing out hibernation image. It'd be
> > simpler and a lot more reliable.
>
> This sounds like an interesting idea to me. Do you already have some
> more precise plan in mind?
Unfortunately, no. No concrete plans or patches yet but even then I'm
pretty sure we don't wanna head the other direction. It's just wrong.
> Unfortunately, for me it is not easy to judge it. I wonder how many
> interfaces would need to get hacked to make this working.
Not much really. We just need a way to flag IOs to write image - be
that a bio / request flag or task flag and then a way to tell the
block layer to block everything else.
> Also I wonder if they would be considered as a hack or a clean
> solution by the affected parties. By other words, I wonder if it is
> realistic.
I'd say it's pretty realistic. This is synchronizing where the
operations need to be synchronized. What part would be hackish?
> > * Device drivers which interact with their devices in a fully
> > synchronous manner so that blocking the kthread always reliably
> > quiesces the device. For this to be true, the device shouldn't
> > carry over any state across the freezing points. There are drivers
> > which are actually like this and it works for them.
>
> I am trying to sort it in my head. In each case, we need to stop these
> kthreads in some well defined state. Also the kthread (or device)
> must be able to block the freezing if they are not in the state yet.
Whether kthreads need to be stopped or not is periphery once you have
a proper blocking / draining mechanism. The thing is most of these
kthreads are blocked on requests coming down from upper layers anyway.
Once you plug queueing of new requests and drain whatever is in
flight, the kthread isn't gonna do anything anyway.
> The stop points are defined by try_to_freeze() now. And freezable
> kthreads block the freezer until they reach these points.
I'm repeating myself but that only works for fully synchronous devices
(ie. kthread processing one command at a time). You need to drain
whatever is in flight too. You can't do that with simple freezing
points. There's a reason why a lot of drivers can't rely on freezer
for PM operations. Drivers which can fully depend on freezer would be
minority.
> > However, my impression is that people don't really scrutinize why
> > freezer works for a specific case and tend to spray them all over
> > and develop a fuzzy sense that freezing will somehow magically make
> > the driver ready for PM operatoins. It doesn't help that freezer is
> > such a blunt tool and our historical usage has always been fuzzy -
> > in the earlier days, we depended on freezer even when we knew it
> > wasn't sufficient probably because updating all subsystems and
> > drivers were such a huge undertaking and freezer kinda sorta works
> > in many cases.
>
> I try to better understand why freezer is considered to be a blunt
> tool. Is it because it is a generic API, try_to_freeze() is put on
> "random" locations, so that it does not define the safe point
> precisely enough?
Not that. I don't know how to explain it better. Hmmm... okay, let's
say there's a shared queue Q and N users o fit. If you wanna make Q
empty and keep it that way for a while, the right thing to do is
blocking new queueing and then wait till Q drains - you choke the
entity that you wanna control.
Instead of that, freezer is trying to block the "N" users part. In
majority of cases, it blocks enough but it's pretty difficult to be
sure whether you actually got all N of them (as some of them may not
involve kthreads at all or unfreezable kthreads might end up doing
those operations too on corner cases) and it's also not that clear
whether blocking the N users actually make Q empty. Maybe there are
things which can be in flight asynchronously on Q even all its N users
are blocked. This is inherently finicky.
> > IMHO we'd be a lot better served by blocking the kthread from PM
> > callbacks explicitly for these drivers to unify control points for
> > PM operations and make what's going on a lot more explicit. This
> > will surely involve a bit more boilerplate code but with the right
> > helpers it should be mostly trivial and I believe that it's likely
> > to encourage higher quality PM operations why getting rid of this
> > fuzzy feeling around the freezer.
>
> I agree. There is needed some synchronization between the device
> drivers and kthread to make sure that they are on the same note.
No, not kthreads. They should be synchronizing on the actual entities
that they wanna keep empty. Where the kthreads are don't matter at
all. It's the wrong thing to control in most cases. In the minority
of cases where blocking kthread is enough, we can do that explicitly
from the usual PM callbacks.
> If I get this correctly. It works the following way now. PM notifies
> both kthreads (via freezer) and device drivers (via callbacks) about
> that the suspend is in progress. They are supposed to go into a sane
> state. But there is no explicit communication between the kthread
> and the driver.
>
> What do you exactly mean with the callbacks? Should they set some
> flags that would navigate the kthread into some state?
The driver PM callbacks.
> > I'm strongly against this. We really don't want to make it even
> > fuzzier. There are finite things which need to be controlled for PM
> > operations and I believe what we need to do is identifying them and
> > implementing explicit and clear control mechanisms for them, not
> > spreading this feel-good mechanism even more, further obscuring where
> > those points are.
>
> I see. This explains why you are so strongly against changing the
> default. I see the following in the kernel sources:
>
> 61 set_freezable()
> 97 kthread_create()
> 259 kthread_run()
>
> I means that only about 17% kthreads are freezable these days.
And some of them probably don't even need it.
> > This becomes the game of "is it frozen ENOUGH yet?" and that "enough"
> > is extremely difficult to determine as we're not looking at the choke
> > spots at all and freezable kthreads only cover part of kernel
> > activity. The fuzzy enoughness also actually inhibits development of
> > proper mechanisms - "I believe this is frozen ENOUGH at this point so
> > it is probably safe to assume that X, Y and Z aren't happening
> > anymore" and it usually is except when it's not.
>
> I am not sure what you mean by frozen enough? I think that a tasks
> is either frozen or not. Do I miss something, please?
Please refer back to the above Q and N users example. Sure, each
kthread is either frozen or thawed; however, whether you got all of N
or not is pretty difficult to tell reliably and I'd say a lot of the
currently existing freezer usage is pretty fuzzy on that respect.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v5] seccomp: add ptrace options for suspend/resume
From: Oleg Nesterov @ 2015-06-13 23:52 UTC (permalink / raw)
To: Tycho Andersen
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Kees Cook, Andy Lutomirski,
Will Drewry, Roland McGrath, Pavel Emelyanov, Serge E. Hallyn
In-Reply-To: <1434207768-16729-1-git-send-email-tycho.andersen-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
On 06/13, Tycho Andersen wrote:
>
> This patch is the first step in enabling checkpoint/restore of processes
> with seccomp enabled.
So just in case, I am fine with this version.
> One of the things CRIU does while dumping tasks is inject code into them
> via ptrace to collect information that is only available to the process
> itself. However, if we are in a seccomp mode where these processes are
> prohibited from making these syscalls, then what CRIU does kills the task.
>
> This patch adds a new ptrace option, PTRACE_O_SUSPEND_SECCOMP, that enables
> a task from the init user namespace which has CAP_SYS_ADMIN and no seccomp
> filters to disable (and re-enable) seccomp filters for another task so that
> they can be successfully dumped (and restored). We restrict the set of
> processes that can disable seccomp through ptrace because although today
> ptrace can be used to bypass seccomp, there is some discussion of closing
> this loophole in the future and we would like this patch to not depend on
> that behavior and be future proofed for when it is removed.
>
> Note that seccomp can be suspended before any filters are actually
> installed; this behavior is useful on criu restore, so that we can suspend
> seccomp, restore the filters, unmap our restore code from the restored
> process' address space, and then resume the task by detaching and have the
> filters resumed as well.
>
> v2 changes:
>
> * require that the tracer have no seccomp filters installed
> * drop TIF_NOTSC manipulation from the patch
> * change from ptrace command to a ptrace option and use this ptrace option
> as the flag to check. This means that as soon as the tracer
> detaches/dies, seccomp is re-enabled and as a corrollary that one can not
> disable seccomp across PTRACE_ATTACHs.
>
> v3 changes:
>
> * get rid of various #ifdefs everywhere
> * report more sensible errors when PTRACE_O_SUSPEND_SECCOMP is incorrectly
> used
>
> v4 changes:
>
> * get rid of may_suspend_seccomp() in favor of a capable() check in ptrace
> directly
>
> v5 changes:
>
> * check that seccomp is not enabled (or suspended) on the tracer
>
> Signed-off-by: Tycho Andersen <tycho.andersen-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> CC: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> CC: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
> CC: Will Drewry <wad-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> CC: Roland McGrath <roland-/Z5OmTQCD9xF6kxbq+BtvQ@public.gmane.org>
> CC: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> CC: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> CC: Serge E. Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
> ---
> include/linux/ptrace.h | 1 +
> include/uapi/linux/ptrace.h | 6 ++++--
> kernel/ptrace.c | 13 +++++++++++++
> kernel/seccomp.c | 8 ++++++++
> 4 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
> index 987a73a..061265f 100644
> --- a/include/linux/ptrace.h
> +++ b/include/linux/ptrace.h
> @@ -34,6 +34,7 @@
> #define PT_TRACE_SECCOMP PT_EVENT_FLAG(PTRACE_EVENT_SECCOMP)
>
> #define PT_EXITKILL (PTRACE_O_EXITKILL << PT_OPT_FLAG_SHIFT)
> +#define PT_SUSPEND_SECCOMP (PTRACE_O_SUSPEND_SECCOMP << PT_OPT_FLAG_SHIFT)
>
> /* single stepping state bits (used on ARM and PA-RISC) */
> #define PT_SINGLESTEP_BIT 31
> diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
> index cf1019e..a7a6979 100644
> --- a/include/uapi/linux/ptrace.h
> +++ b/include/uapi/linux/ptrace.h
> @@ -89,9 +89,11 @@ struct ptrace_peeksiginfo_args {
> #define PTRACE_O_TRACESECCOMP (1 << PTRACE_EVENT_SECCOMP)
>
> /* eventless options */
> -#define PTRACE_O_EXITKILL (1 << 20)
> +#define PTRACE_O_EXITKILL (1 << 20)
> +#define PTRACE_O_SUSPEND_SECCOMP (1 << 21)
>
> -#define PTRACE_O_MASK (0x000000ff | PTRACE_O_EXITKILL)
> +#define PTRACE_O_MASK (\
> + 0x000000ff | PTRACE_O_EXITKILL | PTRACE_O_SUSPEND_SECCOMP)
>
> #include <asm/ptrace.h>
>
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index c8e0e05..496028b 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -556,6 +556,19 @@ static int ptrace_setoptions(struct task_struct *child, unsigned long data)
> if (data & ~(unsigned long)PTRACE_O_MASK)
> return -EINVAL;
>
> + if (unlikely(data & PTRACE_O_SUSPEND_SECCOMP)) {
> + if (!config_enabled(CONFIG_CHECKPOINT_RESTORE) ||
> + !config_enabled(CONFIG_SECCOMP))
> + return -EINVAL;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + if (current->seccomp.mode != SECCOMP_MODE_DISABLED ||
> + current->ptrace & PT_SUSPEND_SECCOMP)
> + return -EPERM;
> + }
> +
> /* Avoid intermediate state when all opts are cleared */
> flags = child->ptrace;
> flags &= ~(PTRACE_O_MASK << PT_OPT_FLAG_SHIFT);
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 980fd26..645e42d 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -590,6 +590,10 @@ void secure_computing_strict(int this_syscall)
> {
> int mode = current->seccomp.mode;
>
> + if (config_enabled(CONFIG_CHECKPOINT_RESTORE) &&
> + unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
> + return;
> +
> if (mode == 0)
> return;
> else if (mode == SECCOMP_MODE_STRICT)
> @@ -691,6 +695,10 @@ u32 seccomp_phase1(struct seccomp_data *sd)
> int this_syscall = sd ? sd->nr :
> syscall_get_nr(current, task_pt_regs(current));
>
> + if (config_enabled(CONFIG_CHECKPOINT_RESTORE) &&
> + unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
> + return SECCOMP_PHASE1_OK;
> +
> switch (mode) {
> case SECCOMP_MODE_STRICT:
> __secure_computing_strict(this_syscall); /* may call do_exit */
> --
> 2.1.4
>
^ permalink raw reply
* [RFC v2 23/24] m68k/mac: Fix PRAM accessors
From: Finn Thain @ 2015-06-14 7:46 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-m68k-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Geert Uytterhoeven,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150614074607.242676098@telegraphics.com.au>
[-- Attachment #1: mac68k-fix-pram-accessors --]
[-- Type: text/plain, Size: 2924 bytes --]
Signed-off-by: Finn Thain <fthain-PnMVE5gNl/W9dxLkCovUHLpzq4S04n8Q@public.gmane.org>
---
Tested on a PowerBook 520 and Quadra 650.
---
arch/m68k/mac/misc.c | 35 +++++++++++++++++++++++++++++------
include/uapi/linux/pmu.h | 2 ++
2 files changed, 31 insertions(+), 6 deletions(-)
Index: linux/arch/m68k/mac/misc.c
===================================================================
--- linux.orig/arch/m68k/mac/misc.c 2015-06-14 17:46:02.000000000 +1000
+++ linux/arch/m68k/mac/misc.c 2015-06-14 17:46:03.000000000 +1000
@@ -119,19 +119,22 @@ static void pmu_write_time(long data)
static unsigned char pmu_pram_read_byte(int offset)
{
struct adb_request req;
- if (pmu_request(&req, NULL, 3, PMU_READ_NVRAM,
- (offset >> 8) & 0xFF, offset & 0xFF) < 0)
+
+ if (pmu_request(&req, NULL, 3, PMU_READ_XPRAM,
+ offset & 0xFF, 1) < 0)
return 0;
while (!req.complete)
pmu_poll();
- return req.reply[3];
+
+ return req.reply[1];
}
static void pmu_pram_write_byte(unsigned char data, int offset)
{
struct adb_request req;
- if (pmu_request(&req, NULL, 4, PMU_WRITE_NVRAM,
- (offset >> 8) & 0xFF, offset & 0xFF, data) < 0)
+
+ if (pmu_request(&req, NULL, 4, PMU_WRITE_XPRAM,
+ offset & 0xFF, 1, data) < 0)
return;
while (!req.complete)
pmu_poll();
@@ -284,11 +287,31 @@ static void via_pram_command(int command
static unsigned char via_pram_read_byte(int offset)
{
- return 0;
+ unsigned char temp;
+ int addr = ((offset & 0xE0) << 3) | ((offset & 0x1F) << 2);
+
+ /* Use RTC command 0x38 for XPRAM access, as per MESS source code */
+ via_pram_command(addr | 0x3800 | 0x8001, &temp);
+
+ return temp;
}
static void via_pram_write_byte(unsigned char data, int offset)
{
+ unsigned char temp;
+ int addr = ((offset & 0xE0) << 3) | ((offset & 0x1F) << 2);
+
+ /* Clear the write protect bit */
+ temp = 0x55;
+ via_pram_command(0x34 | 0x01, &temp);
+
+ /* Write the byte to XPRAM */
+ temp = data;
+ via_pram_command(0x3800 | 0x0001 | addr, &temp);
+
+ /* Set the write protect bit */
+ temp = 0xD5;
+ via_pram_command(0x34 | 0x01, &temp);
}
/*
Index: linux/include/uapi/linux/pmu.h
===================================================================
--- linux.orig/include/uapi/linux/pmu.h 2015-06-14 17:45:34.000000000 +1000
+++ linux/include/uapi/linux/pmu.h 2015-06-14 17:46:03.000000000 +1000
@@ -18,7 +18,9 @@
#define PMU_POWER_CTRL 0x11 /* control power of some devices */
#define PMU_ADB_CMD 0x20 /* send ADB packet */
#define PMU_ADB_POLL_OFF 0x21 /* disable ADB auto-poll */
+#define PMU_WRITE_XPRAM 0x32 /* write eXtended Parameter RAM */
#define PMU_WRITE_NVRAM 0x33 /* write non-volatile RAM */
+#define PMU_READ_XPRAM 0x3a /* read eXtended Parameter RAM */
#define PMU_READ_NVRAM 0x3b /* read non-volatile RAM */
#define PMU_SET_RTC 0x30 /* set real-time clock */
#define PMU_READ_RTC 0x38 /* read real-time clock */
^ permalink raw reply
* [PATCH] include/uapi/linux/swab.h: define a silent macro to avoid sparse error
From: Bilel DRIRA @ 2015-06-14 22:23 UTC (permalink / raw)
To: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Bilel DRIRA
define a silent macro when _CHECKER_ is defined.
This change fixes the following sparse errors:
include/uapi/linux/swab.h:60:16: error: undefined identifier '__builtin_bswap32'
include/uapi/linux/swab.h:60:33: error: not a function <noident>
include/uapi/linux/swab.h:71:16: error: undefined identifier '__builtin_bswap64'
include/uapi/linux/swab.h:71:33: error: not a function <noident>
include/uapi/linux/swab.h:60:33: error: not a function <noident>
Signed-off-by: Bilel DRIRA <bilel.dr-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
include/uapi/linux/swab.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h
index 0e011eb..c04de49 100644
--- a/include/uapi/linux/swab.h
+++ b/include/uapi/linux/swab.h
@@ -5,6 +5,18 @@
#include <linux/compiler.h>
#include <asm/swab.h>
+#ifdef __CHECKER__
+
+#ifdef __HAVE_BUILTIN_BSWAP64__
+#define __builtin_bswap64(val) (0)
+#endif
+
+#ifdef __HAVE_BUILTIN_BSWAP32__
+#define __builtin_bswap32(val) (0)
+#endif
+
+#endif /* __CHECKER__ */
+
/*
* casts are necessary for constants, because we never know how for sure
* how U/UL/ULL map to __u16, __u32, __u64. At least not in a portable way.
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4] pagemap: switch to the new format and do some cleanup
From: Konstantin Khlebnikov @ 2015-06-15 5:56 UTC (permalink / raw)
To: linux-mm, Andrew Morton, Mark Williamson, Naoya Horiguchi
Cc: linux-api, linux-kernel, Kirill A. Shutemov
In-Reply-To: <20150609200021.21971.13598.stgit@zurg>
From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
This patch removes page-shift bits (scheduled to remove since 3.11) and
completes migration to the new bit layout. Also it cleans messy macro.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
v4: fix misprint PM_PFEAME_BITS -> PM_PFRAME_BITS
---
fs/proc/task_mmu.c | 147 ++++++++++++++++---------------------------------
tools/vm/page-types.c | 29 +++-------
2 files changed, 58 insertions(+), 118 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index f1b9ae8..99fa2ae 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -710,23 +710,6 @@ const struct file_operations proc_tid_smaps_operations = {
.release = proc_map_release,
};
-/*
- * We do not want to have constant page-shift bits sitting in
- * pagemap entries and are about to reuse them some time soon.
- *
- * Here's the "migration strategy":
- * 1. when the system boots these bits remain what they are,
- * but a warning about future change is printed in log;
- * 2. once anyone clears soft-dirty bits via clear_refs file,
- * these flag is set to denote, that user is aware of the
- * new API and those page-shift bits change their meaning.
- * The respective warning is printed in dmesg;
- * 3. In a couple of releases we will remove all the mentions
- * of page-shift in pagemap entries.
- */
-
-static bool soft_dirty_cleared __read_mostly;
-
enum clear_refs_types {
CLEAR_REFS_ALL = 1,
CLEAR_REFS_ANON,
@@ -887,13 +870,6 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
if (type < CLEAR_REFS_ALL || type >= CLEAR_REFS_LAST)
return -EINVAL;
- if (type == CLEAR_REFS_SOFT_DIRTY) {
- soft_dirty_cleared = true;
- pr_warn_once("The pagemap bits 55-60 has changed their meaning!"
- " See the linux/Documentation/vm/pagemap.txt for "
- "details.\n");
- }
-
task = get_proc_task(file_inode(file));
if (!task)
return -ESRCH;
@@ -961,38 +937,26 @@ typedef struct {
struct pagemapread {
int pos, len; /* units: PM_ENTRY_BYTES, not bytes */
pagemap_entry_t *buffer;
- bool v2;
bool show_pfn;
};
#define PAGEMAP_WALK_SIZE (PMD_SIZE)
#define PAGEMAP_WALK_MASK (PMD_MASK)
-#define PM_ENTRY_BYTES sizeof(pagemap_entry_t)
-#define PM_STATUS_BITS 3
-#define PM_STATUS_OFFSET (64 - PM_STATUS_BITS)
-#define PM_STATUS_MASK (((1LL << PM_STATUS_BITS) - 1) << PM_STATUS_OFFSET)
-#define PM_STATUS(nr) (((nr) << PM_STATUS_OFFSET) & PM_STATUS_MASK)
-#define PM_PSHIFT_BITS 6
-#define PM_PSHIFT_OFFSET (PM_STATUS_OFFSET - PM_PSHIFT_BITS)
-#define PM_PSHIFT_MASK (((1LL << PM_PSHIFT_BITS) - 1) << PM_PSHIFT_OFFSET)
-#define __PM_PSHIFT(x) (((u64) (x) << PM_PSHIFT_OFFSET) & PM_PSHIFT_MASK)
-#define PM_PFRAME_MASK ((1LL << PM_PSHIFT_OFFSET) - 1)
-#define PM_PFRAME(x) ((x) & PM_PFRAME_MASK)
-/* in "new" pagemap pshift bits are occupied with more status bits */
-#define PM_STATUS2(v2, x) (__PM_PSHIFT(v2 ? x : PAGE_SHIFT))
-
-#define __PM_SOFT_DIRTY (1LL)
-#define __PM_MMAP_EXCLUSIVE (2LL)
-#define PM_PRESENT PM_STATUS(4LL)
-#define PM_SWAP PM_STATUS(2LL)
-#define PM_FILE PM_STATUS(1LL)
-#define PM_NOT_PRESENT(v2) PM_STATUS2(v2, 0)
+#define PM_ENTRY_BYTES sizeof(pagemap_entry_t)
+#define PM_PFRAME_BITS 54
+#define PM_PFRAME_MASK GENMASK_ULL(PM_PFRAME_BITS - 1, 0)
+#define PM_SOFT_DIRTY BIT_ULL(55)
+#define PM_MMAP_EXCLUSIVE BIT_ULL(56)
+#define PM_FILE BIT_ULL(61)
+#define PM_SWAP BIT_ULL(62)
+#define PM_PRESENT BIT_ULL(63)
+
#define PM_END_OF_BUFFER 1
-static inline pagemap_entry_t make_pme(u64 val)
+static inline pagemap_entry_t make_pme(u64 frame, u64 flags)
{
- return (pagemap_entry_t) { .pme = val };
+ return (pagemap_entry_t) { .pme = (frame & PM_PFRAME_MASK) | flags };
}
static int add_to_pagemap(unsigned long addr, pagemap_entry_t *pme,
@@ -1013,7 +977,7 @@ static int pagemap_pte_hole(unsigned long start, unsigned long end,
while (addr < end) {
struct vm_area_struct *vma = find_vma(walk->mm, addr);
- pagemap_entry_t pme = make_pme(PM_NOT_PRESENT(pm->v2));
+ pagemap_entry_t pme = make_pme(0, 0);
/* End of address space hole, which we mark as non-present. */
unsigned long hole_end;
@@ -1033,7 +997,7 @@ static int pagemap_pte_hole(unsigned long start, unsigned long end,
/* Addresses in the VMA. */
if (vma->vm_flags & VM_SOFTDIRTY)
- pme.pme |= PM_STATUS2(pm->v2, __PM_SOFT_DIRTY);
+ pme = make_pme(0, PM_SOFT_DIRTY);
for (; addr < min(end, vma->vm_end); addr += PAGE_SIZE) {
err = add_to_pagemap(addr, &pme, pm);
if (err)
@@ -1044,50 +1008,44 @@ out:
return err;
}
-static void pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
+static pagemap_entry_t pte_to_pagemap_entry(struct pagemapread *pm,
struct vm_area_struct *vma, unsigned long addr, pte_t pte)
{
- u64 frame = 0, flags;
+ u64 frame = 0, flags = 0;
struct page *page = NULL;
- int flags2 = 0;
if (pte_present(pte)) {
if (pm->show_pfn)
frame = pte_pfn(pte);
- flags = PM_PRESENT;
+ flags |= PM_PRESENT;
page = vm_normal_page(vma, addr, pte);
if (pte_soft_dirty(pte))
- flags2 |= __PM_SOFT_DIRTY;
+ flags |= PM_SOFT_DIRTY;
} else if (is_swap_pte(pte)) {
swp_entry_t entry;
if (pte_swp_soft_dirty(pte))
- flags2 |= __PM_SOFT_DIRTY;
+ flags |= PM_SOFT_DIRTY;
entry = pte_to_swp_entry(pte);
frame = swp_type(entry) |
(swp_offset(entry) << MAX_SWAPFILES_SHIFT);
- flags = PM_SWAP;
+ flags |= PM_SWAP;
if (is_migration_entry(entry))
page = migration_entry_to_page(entry);
- } else {
- if (vma->vm_flags & VM_SOFTDIRTY)
- flags2 |= __PM_SOFT_DIRTY;
- *pme = make_pme(PM_NOT_PRESENT(pm->v2) | PM_STATUS2(pm->v2, flags2));
- return;
}
if (page && !PageAnon(page))
flags |= PM_FILE;
if (page && page_mapcount(page) == 1)
- flags2 |= __PM_MMAP_EXCLUSIVE;
- if ((vma->vm_flags & VM_SOFTDIRTY))
- flags2 |= __PM_SOFT_DIRTY;
+ flags |= PM_MMAP_EXCLUSIVE;
+ if (vma->vm_flags & VM_SOFTDIRTY)
+ flags |= PM_SOFT_DIRTY;
- *pme = make_pme(PM_PFRAME(frame) | PM_STATUS2(pm->v2, flags2) | flags);
+ return make_pme(frame, flags);
}
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-static void thp_pmd_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
- pmd_t pmd, int offset, int pmd_flags2)
+static pagemap_entry_t thp_pmd_to_pagemap_entry(struct pagemapread *pm,
+ pmd_t pmd, int offset, u64 flags)
{
u64 frame = 0;
@@ -1099,15 +1057,16 @@ static void thp_pmd_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *p
if (pmd_present(pmd)) {
if (pm->show_pfn)
frame = pmd_pfn(pmd) + offset;
- *pme = make_pme(PM_PFRAME(frame) | PM_PRESENT |
- PM_STATUS2(pm->v2, pmd_flags2));
- } else
- *pme = make_pme(PM_NOT_PRESENT(pm->v2) | PM_STATUS2(pm->v2, pmd_flags2));
+ flags |= PM_PRESENT;
+ }
+
+ return make_pme(frame, flags);
}
#else
-static inline void thp_pmd_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
- pmd_t pmd, int offset, int pmd_flags2)
+static pagemap_entry_t thp_pmd_to_pagemap_entry(struct pagemapread *pm,
+ pmd_t pmd, int offset, u64 flags)
{
+ return make_pme(0, 0);
}
#endif
@@ -1121,18 +1080,16 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
int err = 0;
if (pmd_trans_huge_lock(pmd, vma, &ptl) == 1) {
- int pmd_flags2;
+ u64 flags = 0;
if ((vma->vm_flags & VM_SOFTDIRTY) || pmd_soft_dirty(*pmd))
- pmd_flags2 = __PM_SOFT_DIRTY;
- else
- pmd_flags2 = 0;
+ flags |= PM_SOFT_DIRTY;
if (pmd_present(*pmd)) {
struct page *page = pmd_page(*pmd);
if (page_mapcount(page) == 1)
- pmd_flags2 |= __PM_MMAP_EXCLUSIVE;
+ flags |= PM_MMAP_EXCLUSIVE;
}
for (; addr != end; addr += PAGE_SIZE) {
@@ -1141,7 +1098,7 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
offset = (addr & ~PAGEMAP_WALK_MASK) >>
PAGE_SHIFT;
- thp_pmd_to_pagemap_entry(&pme, pm, *pmd, offset, pmd_flags2);
+ pme = thp_pmd_to_pagemap_entry(pm, *pmd, offset, flags);
err = add_to_pagemap(addr, &pme, pm);
if (err)
break;
@@ -1161,7 +1118,7 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
for (; addr < end; pte++, addr += PAGE_SIZE) {
pagemap_entry_t pme;
- pte_to_pagemap_entry(&pme, pm, vma, addr, *pte);
+ pme = pte_to_pagemap_entry(pm, vma, addr, *pte);
err = add_to_pagemap(addr, &pme, pm);
if (err)
break;
@@ -1174,19 +1131,18 @@ static int pagemap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
}
#ifdef CONFIG_HUGETLB_PAGE
-static void huge_pte_to_pagemap_entry(pagemap_entry_t *pme, struct pagemapread *pm,
- pte_t pte, int offset, int flags2)
+static pagemap_entry_t huge_pte_to_pagemap_entry(struct pagemapread *pm,
+ pte_t pte, int offset, u64 flags)
{
u64 frame = 0;
if (pte_present(pte)) {
if (pm->show_pfn)
frame = pte_pfn(pte) + offset;
- *pme = make_pme(PM_PFRAME(frame) | PM_PRESENT |
- PM_STATUS2(pm->v2, flags2));
- } else
- *pme = make_pme(PM_NOT_PRESENT(pm->v2) |
- PM_STATUS2(pm->v2, flags2));
+ flags |= PM_PRESENT;
+ }
+
+ return make_pme(frame, flags);
}
/* This function walks within one hugetlb entry in the single call */
@@ -1197,17 +1153,15 @@ static int pagemap_hugetlb_range(pte_t *pte, unsigned long hmask,
struct pagemapread *pm = walk->private;
struct vm_area_struct *vma = walk->vma;
int err = 0;
- int flags2;
+ u64 flags = 0;
pagemap_entry_t pme;
if (vma->vm_flags & VM_SOFTDIRTY)
- flags2 = __PM_SOFT_DIRTY;
- else
- flags2 = 0;
+ flags |= PM_SOFT_DIRTY;
for (; addr != end; addr += PAGE_SIZE) {
int offset = (addr & ~hmask) >> PAGE_SHIFT;
- huge_pte_to_pagemap_entry(&pme, pm, *pte, offset, flags2);
+ pme = huge_pte_to_pagemap_entry(pm, *pte, offset, flags);
err = add_to_pagemap(addr, &pme, pm);
if (err)
return err;
@@ -1228,7 +1182,9 @@ static int pagemap_hugetlb_range(pte_t *pte, unsigned long hmask,
* Bits 0-54 page frame number (PFN) if present
* Bits 0-4 swap type if swapped
* Bits 5-54 swap offset if swapped
- * Bits 55-60 page shift (page size = 1<<page shift)
+ * Bit 55 pte is soft-dirty (see Documentation/vm/soft-dirty.txt)
+ * Bit 56 page exclusively mapped
+ * Bits 57-60 zero
* Bit 61 page is file-page or shared-anon
* Bit 62 page swapped
* Bit 63 page present
@@ -1269,7 +1225,6 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
/* do not disclose physical addresses: attack vector */
pm.show_pfn = file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN);
- pm.v2 = soft_dirty_cleared;
pm.len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);
pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_TEMPORARY);
ret = -ENOMEM;
@@ -1339,10 +1294,6 @@ static int pagemap_open(struct inode *inode, struct file *file)
{
struct mm_struct *mm;
- pr_warn_once("Bits 55-60 of /proc/PID/pagemap entries are about "
- "to stop being page-shift some time soon. See the "
- "linux/Documentation/vm/pagemap.txt for details.\n");
-
mm = proc_mem_open(inode, PTRACE_MODE_READ);
if (IS_ERR(mm))
return PTR_ERR(mm);
diff --git a/tools/vm/page-types.c b/tools/vm/page-types.c
index 3a9f193..e1d5ff8 100644
--- a/tools/vm/page-types.c
+++ b/tools/vm/page-types.c
@@ -57,26 +57,15 @@
* pagemap kernel ABI bits
*/
-#define PM_ENTRY_BYTES sizeof(uint64_t)
-#define PM_STATUS_BITS 3
-#define PM_STATUS_OFFSET (64 - PM_STATUS_BITS)
-#define PM_STATUS_MASK (((1LL << PM_STATUS_BITS) - 1) << PM_STATUS_OFFSET)
-#define PM_STATUS(nr) (((nr) << PM_STATUS_OFFSET) & PM_STATUS_MASK)
-#define PM_PSHIFT_BITS 6
-#define PM_PSHIFT_OFFSET (PM_STATUS_OFFSET - PM_PSHIFT_BITS)
-#define PM_PSHIFT_MASK (((1LL << PM_PSHIFT_BITS) - 1) << PM_PSHIFT_OFFSET)
-#define __PM_PSHIFT(x) (((uint64_t) (x) << PM_PSHIFT_OFFSET) & PM_PSHIFT_MASK)
-#define PM_PFRAME_MASK ((1LL << PM_PSHIFT_OFFSET) - 1)
-#define PM_PFRAME(x) ((x) & PM_PFRAME_MASK)
-
-#define __PM_SOFT_DIRTY (1LL)
-#define __PM_MMAP_EXCLUSIVE (2LL)
-#define PM_PRESENT PM_STATUS(4LL)
-#define PM_SWAP PM_STATUS(2LL)
-#define PM_FILE PM_STATUS(1LL)
-#define PM_SOFT_DIRTY __PM_PSHIFT(__PM_SOFT_DIRTY)
-#define PM_MMAP_EXCLUSIVE __PM_PSHIFT(__PM_MMAP_EXCLUSIVE)
-
+#define PM_ENTRY_BYTES 8
+#define PM_PFRAME_BITS 54
+#define PM_PFRAME_MASK ((1LL << PM_PFRAME_BITS) - 1)
+#define PM_PFRAME(x) ((x) & PM_PFRAME_MASK)
+#define PM_SOFT_DIRTY (1ULL << 55)
+#define PM_MMAP_EXCLUSIVE (1ULL << 56)
+#define PM_FILE (1ULL << 61)
+#define PM_SWAP (1ULL << 62)
+#define PM_PRESENT (1ULL << 63)
/*
* kernel page flags
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: [RFC v2 23/24] m68k/mac: Fix PRAM accessors
From: Geert Uytterhoeven @ 2015-06-15 8:23 UTC (permalink / raw)
To: Finn Thain
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux/m68k,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20150614074613.054681242-PnMVE5gNl/W9dxLkCovUHLpzq4S04n8Q@public.gmane.org>
Hi Finn,
On Sun, Jun 14, 2015 at 9:46 AM, Finn Thain <fthain-PnMVE5gNl/W9dxLkCovUHLpzq4S04n8Q@public.gmane.org> wrote:
> --- linux.orig/arch/m68k/mac/misc.c 2015-06-14 17:46:02.000000000 +1000
> +++ linux/arch/m68k/mac/misc.c 2015-06-14 17:46:03.000000000 +1000
> @@ -284,11 +287,31 @@ static void via_pram_command(int command
>
> static unsigned char via_pram_read_byte(int offset)
> {
> - return 0;
> + unsigned char temp;
> + int addr = ((offset & 0xE0) << 3) | ((offset & 0x1F) << 2);
Can you please add #defines for the magic values?
> +
> + /* Use RTC command 0x38 for XPRAM access, as per MESS source code */
> + via_pram_command(addr | 0x3800 | 0x8001, &temp);
It seems 0x38 is already documented in <linux/pmu.h> (see below), or not (it's
shifted left by 8 bits?)?
> +
> + return temp;
> }
>
> static void via_pram_write_byte(unsigned char data, int offset)
> {
> + unsigned char temp;
> + int addr = ((offset & 0xE0) << 3) | ((offset & 0x1F) << 2);
> +
> + /* Clear the write protect bit */
> + temp = 0x55;
> + via_pram_command(0x34 | 0x01, &temp);
> +
> + /* Write the byte to XPRAM */
> + temp = data;
> + via_pram_command(0x3800 | 0x0001 | addr, &temp);
> +
> + /* Set the write protect bit */
> + temp = 0xD5;
> + via_pram_command(0x34 | 0x01, &temp);
More magic values...
> }
>
> /*
> Index: linux/include/uapi/linux/pmu.h
> ===================================================================
> --- linux.orig/include/uapi/linux/pmu.h 2015-06-14 17:45:34.000000000 +1000
> +++ linux/include/uapi/linux/pmu.h 2015-06-14 17:46:03.000000000 +1000
> @@ -18,7 +18,9 @@
> #define PMU_POWER_CTRL 0x11 /* control power of some devices */
> #define PMU_ADB_CMD 0x20 /* send ADB packet */
> #define PMU_ADB_POLL_OFF 0x21 /* disable ADB auto-poll */
> +#define PMU_WRITE_XPRAM 0x32 /* write eXtended Parameter RAM */
> #define PMU_WRITE_NVRAM 0x33 /* write non-volatile RAM */
> +#define PMU_READ_XPRAM 0x3a /* read eXtended Parameter RAM */
> #define PMU_READ_NVRAM 0x3b /* read non-volatile RAM */
> #define PMU_SET_RTC 0x30 /* set real-time clock */
> #define PMU_READ_RTC 0x38 /* read real-time clock */
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [RFC PATCH 07/18] kthread: Make iterant kthreads freezable by default
From: Petr Mladek @ 2015-06-15 9:28 UTC (permalink / raw)
To: Tejun Heo
Cc: Andrew Morton, Oleg Nesterov, Ingo Molnar, Peter Zijlstra,
Richard Weinberger, Steven Rostedt, David Woodhouse,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Trond Myklebust,
Anna Schumaker, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Chris Mason,
Paul E. McKenney, Thomas Gleixner, Linus Torvalds, Jiri Kosina,
Borislav Petkov, Michal Hocko,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150613232222.GB346-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
On Sat 2015-06-13 18:22:22, Tejun Heo wrote:
> > I try to better understand why freezer is considered to be a blunt
> > tool. Is it because it is a generic API, try_to_freeze() is put on
> > "random" locations, so that it does not define the safe point
> > precisely enough?
>
> Not that. I don't know how to explain it better. Hmmm... okay, let's
> say there's a shared queue Q and N users o fit. If you wanna make Q
> empty and keep it that way for a while, the right thing to do is
> blocking new queueing and then wait till Q drains - you choke the
> entity that you wanna control.
>
> Instead of that, freezer is trying to block the "N" users part. In
> majority of cases, it blocks enough but it's pretty difficult to be
> sure whether you actually got all N of them (as some of them may not
> involve kthreads at all or unfreezable kthreads might end up doing
> those operations too on corner cases) and it's also not that clear
> whether blocking the N users actually make Q empty. Maybe there are
> things which can be in flight asynchronously on Q even all its N users
> are blocked. This is inherently finicky.
I feel convinced that it does not make sense to make kthreads
freezable by default and that we should not use it when not
necessary.
Thanks a lot for patience and so detailed explanation.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v5] seccomp: add ptrace options for suspend/resume
From: Pavel Emelyanov @ 2015-06-15 9:41 UTC (permalink / raw)
To: Tycho Andersen, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
Cc: Kees Cook, Andy Lutomirski, Will Drewry, Roland McGrath,
Oleg Nesterov, Serge E. Hallyn
In-Reply-To: <1434207768-16729-1-git-send-email-tycho.andersen-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
On 06/13/2015 06:02 PM, Tycho Andersen wrote:
> This patch is the first step in enabling checkpoint/restore of processes
> with seccomp enabled.
>
> One of the things CRIU does while dumping tasks is inject code into them
> via ptrace to collect information that is only available to the process
> itself. However, if we are in a seccomp mode where these processes are
> prohibited from making these syscalls, then what CRIU does kills the task.
>
> This patch adds a new ptrace option, PTRACE_O_SUSPEND_SECCOMP, that enables
> a task from the init user namespace which has CAP_SYS_ADMIN and no seccomp
> filters to disable (and re-enable) seccomp filters for another task so that
> they can be successfully dumped (and restored). We restrict the set of
> processes that can disable seccomp through ptrace because although today
> ptrace can be used to bypass seccomp, there is some discussion of closing
> this loophole in the future and we would like this patch to not depend on
> that behavior and be future proofed for when it is removed.
>
> Note that seccomp can be suspended before any filters are actually
> installed; this behavior is useful on criu restore, so that we can suspend
> seccomp, restore the filters, unmap our restore code from the restored
> process' address space, and then resume the task by detaching and have the
> filters resumed as well.
>
> v2 changes:
>
> * require that the tracer have no seccomp filters installed
> * drop TIF_NOTSC manipulation from the patch
> * change from ptrace command to a ptrace option and use this ptrace option
> as the flag to check. This means that as soon as the tracer
> detaches/dies, seccomp is re-enabled and as a corrollary that one can not
> disable seccomp across PTRACE_ATTACHs.
>
> v3 changes:
>
> * get rid of various #ifdefs everywhere
> * report more sensible errors when PTRACE_O_SUSPEND_SECCOMP is incorrectly
> used
>
> v4 changes:
>
> * get rid of may_suspend_seccomp() in favor of a capable() check in ptrace
> directly
>
> v5 changes:
>
> * check that seccomp is not enabled (or suspended) on the tracer
>
> Signed-off-by: Tycho Andersen <tycho.andersen-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> CC: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> CC: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
> CC: Will Drewry <wad-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> CC: Roland McGrath <roland-/Z5OmTQCD9xF6kxbq+BtvQ@public.gmane.org>
> CC: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> CC: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> CC: Serge E. Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
Acked-by: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
^ permalink raw reply
* Re: [PATCH v2 0/7] CLONE_FD: Task exit notification via file descriptor
From: Florian Weimer @ 2015-06-15 10:06 UTC (permalink / raw)
To: Thiago Macieira
Cc: Josh Triplett, Al Viro, Andrew Morton, Andy Lutomirski,
Ingo Molnar, Kees Cook, Oleg Nesterov, Paul E. McKenney,
H. Peter Anvin, Rik van Riel, Thomas Gleixner, Michael Kerrisk,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, x86-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <1775865.3maNSqUW4u@tjmaciei-mobl4>
On 05/29/2015 10:27 PM, Thiago Macieira wrote:
>> It has been suggested (e.g.,
>> <https://sourceware.org/bugzilla/show_bug.cgi?id=15661#c3>) that you can
>> use the existing clone(2) without specifying SIGCHLD to create a new
>> process. The resulting child process is not supposed to show up in
>> wait(2), only in a waitpid(2) (or similar) explicitly specifying the
>> PID. Is this not the case?
>
> Hi Florian
>
> That sounds orthogonal to what we're looking for. Our objective is to get
> notification of when the child exited without resorting to SIGCHLD. If we use
> the regular clone(2) without SIGCHLD and without CLONE_FD, we get no
> notification. The only way to know of the child's termination is by a blocking
> waitpid(2), like you indicated, which is counter productive to our needs.
>
> We need something we can select(2)/poll(2) on.
Thanks for the clarification. I agree that this is a separate and quite
sensible use case.
--
Florian Weimer / Red Hat Product Security
^ 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