Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1470046 > unrolled thread

[RFC v2 09/10] landlock: Handle cgroups

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC v2 09/10] landlock: Handle cgroups Mickaël Salaün <mic@digikod.net> - 2016-08-25 12:50 +0200
    Re: [RFC v2 09/10] landlock: Handle cgroups Andy Lutomirski <luto@amacapital.net> - 2016-08-25 13:20 +0200
      Re: [RFC v2 09/10] landlock: Handle cgroups Mickaël Salaün <mic@digikod.net> - 2016-08-25 16:50 +0200
        Re: [RFC v2 09/10] landlock: Handle cgroups Tejun Heo <tj@kernel.org> - 2016-08-26 15:00 +0200
        Re: [RFC v2 09/10] landlock: Handle cgroups Andy Lutomirski <luto@amacapital.net> - 2016-08-26 16:30 +0200
          Re: [RFC v2 09/10] landlock: Handle cgroups Tejun Heo <tj@kernel.org> - 2016-08-26 18:00 +0200
    Re: [RFC v2 09/10] landlock: Handle cgroups Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-26 04:30 +0200
      Re: [RFC v2 09/10] landlock: Handle cgroups Mickaël Salaün <mic@digikod.net> - 2016-08-26 17:20 +0200
        Re: [RFC v2 09/10] landlock: Handle cgroups Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 01:10 +0200
          Re: [RFC v2 09/10] landlock: Handle cgroups Andy Lutomirski <luto@amacapital.net> - 2016-08-27 09:40 +0200
            Re: [RFC v2 09/10] landlock: Handle cgroups Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 20:20 +0200
              Re: [RFC v2 09/10] landlock: Handle cgroups Andy Lutomirski <luto@amacapital.net> - 2016-08-28 10:20 +0200
          Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Mickaël Salaün <mic@digikod.net> - 2016-08-27 16:20 +0200
            Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 20:10 +0200
              Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Mickaël Salaün <mic@digikod.net> - 2016-08-27 21:40 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 22:50 +0200
                  Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Mickaël Salaün <mic@digikod.net> - 2016-08-27 23:20 +0200
                    Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Andy Lutomirski <luto@amacapital.net> - 2016-08-28 10:20 +0200
                      Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Mickaël Salaün <mic@digikod.net> - 2016-08-28 11:50 +0200
                        Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Andy Lutomirski <luto@amacapital.net> - 2016-08-30 21:00 +0200
                          Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Mickaël Salaün <mic@digikod.net> - 2016-08-30 22:30 +0200
                            Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Andy Lutomirski <luto@amacapital.net> - 2016-08-30 22:30 +0200
                              Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Mickaël Salaün <mic@digikod.net> - 2016-08-30 22:40 +0200
                                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-30 23:00 +0200
                                  Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Andy Lutomirski <luto@amacapital.net> - 2016-08-30 23:50 +0200
                                    Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-31 03:40 +0200
                                      Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Andy Lutomirski <luto@amacapital.net> - 2016-08-31 05:30 +0200
          Re: [RFC v2 09/10] landlock: Handle cgroups (netfilter match) Mickaël Salaün <mic@digikod.net> - 2016-08-27 16:30 +0200
            Re: [RFC v2 09/10] landlock: Handle cgroups (netfilter match) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 20:40 +0200
          Re: [RFC v2 09/10] landlock: Handle cgroups (program types) Mickaël Salaün <mic@digikod.net> - 2016-08-27 16:40 +0200
            Re: [RFC v2 09/10] landlock: Handle cgroups (program types) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 20:30 +0200
              Re: [RFC v2 09/10] landlock: Handle cgroups (program types) Mickaël Salaün <mic@digikod.net> - 2016-08-27 22:00 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (program types) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 23:00 +0200
                  Re: [RFC v2 09/10] landlock: Handle cgroups (program types) Mickaël Salaün <mic@digikod.net> - 2016-08-27 23:20 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1472845 — Re: [RFC v2 09/10] landlock: Handle cgroups (performance)

[Multipart message — attachments visible in raw view] — view raw

On 30/08/2016 20:55, Andy Lutomirski wrote:
> On Sun, Aug 28, 2016 at 2:42 AM, Mickaël Salaün <mic@digikod.net> wrote:
>>
>>
>> On 28/08/2016 10:13, Andy Lutomirski wrote:
>>> On Aug 27, 2016 11:14 PM, "Mickaël Salaün" <mic@digikod.net> wrote:
>>>>
>>>>
>>>> On 27/08/2016 22:43, Alexei Starovoitov wrote:
>>>>> On Sat, Aug 27, 2016 at 09:35:14PM +0200, Mickaël Salaün wrote:
>>>>>> On 27/08/2016 20:06, Alexei Starovoitov wrote:
>>>>>>> On Sat, Aug 27, 2016 at 04:06:38PM +0200, Mickaël Salaün wrote:
>>>>>>>> As said above, Landlock will not run an eBPF programs when not strictly
>>>>>>>> needed. Attaching to a cgroup will have the same performance impact as
>>>>>>>> attaching to a process hierarchy.
>>>>>>>
>>>>>>> Having a prog per cgroup per lsm_hook is the only scalable way I
>>>>>>> could come up with. If you see another way, please propose.
>>>>>>> current->seccomp.landlock_prog is not the answer.
>>>>>>
>>>>>> Hum, I don't see the difference from a performance point of view between
>>>>>> a cgroup-based or a process hierarchy-based system.
>>>>>>
>>>>>> Maybe a better option should be to use an array of pointers with N
>>>>>> entries, one for each supported hook, instead of a unique pointer list?
>>>>>
>>>>> yes, clearly array dereference is faster than link list walk.
>>>>> Now the question is where to keep this prog_array[num_lsm_hooks] ?
>>>>> Since we cannot keep it inside task_struct, we have to allocate it.
>>>>> Every time the task is creted then. What to do on the fork? That
>>>>> will require changes all over. Then the obvious optimization would be
>>>>> to share this allocated array of prog pointers across multiple tasks...
>>>>> and little by little this new facility will look like cgroup.
>>>>> Hence the suggestion to put this array into cgroup from the start.
>>>>
>>>> I see your point :)
>>>>
>>>>>
>>>>>> Anyway, being able to attach an LSM hook program to a cgroup thanks to
>>>>>> the new BPF_PROG_ATTACH seems a good idea (while keeping the possibility
>>>>>> to use a process hierarchy). The downside will be to handle an LSM hook
>>>>>> program which is not triggered by a seccomp-filter, but this should be
>>>>>> needed anyway to handle interruptions.
>>>>>
>>>>> what do you mean 'not triggered by seccomp' ?
>>>>> You're not suggesting that this lsm has to enable seccomp to be functional?
>>>>> imo that's non starter due to overhead.
>>>>
>>>> Yes, for now, it is triggered by a new seccomp filter return value
>>>> RET_LANDLOCK, which can take a 16-bit value called cookie. This must not
>>>> be needed but could be useful to bind a seccomp filter security policy
>>>> with a Landlock one. Waiting for Kees's point of view…
>>>>
>>>
>>> I'm not Kees, but I'd be okay with that.  I still think that doing
>>> this by process hierarchy a la seccomp will be easier to use and to
>>> understand (which is quite important for this kind of work) than doing
>>> it by cgroup.
>>>
>>> A feature I've wanted to add for a while is to have an fd that
>>> represents a seccomp layer, the idea being that you would set up your
>>> seccomp layer (with syscall filter, landlock hooks, etc) and then you
>>> would have a syscall to install that layer.  Then an unprivileged
>>> sandbox manager could set up its layer and still be able to inject new
>>> processes into it later on, no cgroups needed.
>>
>> A nice thing I didn't highlight about Landlock is that a process can
>> prepare a layer of rules (arraymap of handles + Landlock programs) and
>> pass the file descriptors of the Landlock programs to another process.
>> This process could then apply this programs to get sandboxed. However,
>> for now, because a Landlock program is only triggered by a seccomp
>> filter (which do not follow the Landlock programs as a FD), they will be
>> useless.
>>
>> The FD referring to an arraymap of handles can also be used to update a
>> map and change the behavior of a Landlock program. A master process can
>> then add or remove restrictions to another process hierarchy on the fly.
> 
> Maybe this could be extended a little bit.  The fd could hold the
> seccomp filter *and* the LSM hook filters.  FMODE_EXECUTE could give
> the ability to install it and FMODE_WRITE could give the ability to
> modify it.
> 

This is interesting! It should be possible to append the seccomp stack
of a source process to the seccomp stack of the target process when a
Landlock program is passed and then activated through seccomp(2).

For the FMODE_EXECUTE/FMODE_WRITE, are you suggesting to manage
permission of the eBPF program FD in a specific way?

[toc] | [prev] | [next] | [standalone]

#1472847 — Re: [RFC v2 09/10] landlock: Handle cgroups (performance)

On Tue, Aug 30, 2016 at 1:20 PM, Mickaël Salaün <mic@digikod.net> wrote:
>
> On 30/08/2016 20:55, Andy Lutomirski wrote:
>> On Sun, Aug 28, 2016 at 2:42 AM, Mickaël Salaün <mic@digikod.net> wrote:
>>>
>>>
>>> On 28/08/2016 10:13, Andy Lutomirski wrote:
>>>> On Aug 27, 2016 11:14 PM, "Mickaël Salaün" <mic@digikod.net> wrote:
>>>>>
>>>>>
>>>>> On 27/08/2016 22:43, Alexei Starovoitov wrote:
>>>>>> On Sat, Aug 27, 2016 at 09:35:14PM +0200, Mickaël Salaün wrote:
>>>>>>> On 27/08/2016 20:06, Alexei Starovoitov wrote:
>>>>>>>> On Sat, Aug 27, 2016 at 04:06:38PM +0200, Mickaël Salaün wrote:
>>>>>>>>> As said above, Landlock will not run an eBPF programs when not strictly
>>>>>>>>> needed. Attaching to a cgroup will have the same performance impact as
>>>>>>>>> attaching to a process hierarchy.
>>>>>>>>
>>>>>>>> Having a prog per cgroup per lsm_hook is the only scalable way I
>>>>>>>> could come up with. If you see another way, please propose.
>>>>>>>> current->seccomp.landlock_prog is not the answer.
>>>>>>>
>>>>>>> Hum, I don't see the difference from a performance point of view between
>>>>>>> a cgroup-based or a process hierarchy-based system.
>>>>>>>
>>>>>>> Maybe a better option should be to use an array of pointers with N
>>>>>>> entries, one for each supported hook, instead of a unique pointer list?
>>>>>>
>>>>>> yes, clearly array dereference is faster than link list walk.
>>>>>> Now the question is where to keep this prog_array[num_lsm_hooks] ?
>>>>>> Since we cannot keep it inside task_struct, we have to allocate it.
>>>>>> Every time the task is creted then. What to do on the fork? That
>>>>>> will require changes all over. Then the obvious optimization would be
>>>>>> to share this allocated array of prog pointers across multiple tasks...
>>>>>> and little by little this new facility will look like cgroup.
>>>>>> Hence the suggestion to put this array into cgroup from the start.
>>>>>
>>>>> I see your point :)
>>>>>
>>>>>>
>>>>>>> Anyway, being able to attach an LSM hook program to a cgroup thanks to
>>>>>>> the new BPF_PROG_ATTACH seems a good idea (while keeping the possibility
>>>>>>> to use a process hierarchy). The downside will be to handle an LSM hook
>>>>>>> program which is not triggered by a seccomp-filter, but this should be
>>>>>>> needed anyway to handle interruptions.
>>>>>>
>>>>>> what do you mean 'not triggered by seccomp' ?
>>>>>> You're not suggesting that this lsm has to enable seccomp to be functional?
>>>>>> imo that's non starter due to overhead.
>>>>>
>>>>> Yes, for now, it is triggered by a new seccomp filter return value
>>>>> RET_LANDLOCK, which can take a 16-bit value called cookie. This must not
>>>>> be needed but could be useful to bind a seccomp filter security policy
>>>>> with a Landlock one. Waiting for Kees's point of view…
>>>>>
>>>>
>>>> I'm not Kees, but I'd be okay with that.  I still think that doing
>>>> this by process hierarchy a la seccomp will be easier to use and to
>>>> understand (which is quite important for this kind of work) than doing
>>>> it by cgroup.
>>>>
>>>> A feature I've wanted to add for a while is to have an fd that
>>>> represents a seccomp layer, the idea being that you would set up your
>>>> seccomp layer (with syscall filter, landlock hooks, etc) and then you
>>>> would have a syscall to install that layer.  Then an unprivileged
>>>> sandbox manager could set up its layer and still be able to inject new
>>>> processes into it later on, no cgroups needed.
>>>
>>> A nice thing I didn't highlight about Landlock is that a process can
>>> prepare a layer of rules (arraymap of handles + Landlock programs) and
>>> pass the file descriptors of the Landlock programs to another process.
>>> This process could then apply this programs to get sandboxed. However,
>>> for now, because a Landlock program is only triggered by a seccomp
>>> filter (which do not follow the Landlock programs as a FD), they will be
>>> useless.
>>>
>>> The FD referring to an arraymap of handles can also be used to update a
>>> map and change the behavior of a Landlock program. A master process can
>>> then add or remove restrictions to another process hierarchy on the fly.
>>
>> Maybe this could be extended a little bit.  The fd could hold the
>> seccomp filter *and* the LSM hook filters.  FMODE_EXECUTE could give
>> the ability to install it and FMODE_WRITE could give the ability to
>> modify it.
>>
>
> This is interesting! It should be possible to append the seccomp stack
> of a source process to the seccomp stack of the target process when a
> Landlock program is passed and then activated through seccomp(2).
>
> For the FMODE_EXECUTE/FMODE_WRITE, are you suggesting to manage
> permission of the eBPF program FD in a specific way?
>

This wouldn't be an eBPF program FD -- it would be an FD encapsulating
an entire configuration including seccomp BPF program, whatever
landlock stuff is associated, and eventual seccomp monitor
configuration (once I write that code), etc.

You wouldn't say "attach this process's seccomp stack to me" -- you'd
say "attach this seccomp layer to me".

A decision that we'd have to make would be whether the FD links to the
parent layer or whether it can be attached without regard to what the
parent layer is.

--Andy

[toc] | [prev] | [next] | [standalone]

#1472857 — Re: [RFC v2 09/10] landlock: Handle cgroups (performance)

[Multipart message — attachments visible in raw view] — view raw

On 30/08/2016 22:23, Andy Lutomirski wrote:
> On Tue, Aug 30, 2016 at 1:20 PM, Mickaël Salaün <mic@digikod.net> wrote:
>>
>> On 30/08/2016 20:55, Andy Lutomirski wrote:
>>> On Sun, Aug 28, 2016 at 2:42 AM, Mickaël Salaün <mic@digikod.net> wrote:
>>>>
>>>>
>>>> On 28/08/2016 10:13, Andy Lutomirski wrote:
>>>>> On Aug 27, 2016 11:14 PM, "Mickaël Salaün" <mic@digikod.net> wrote:
>>>>>>
>>>>>>
>>>>>> On 27/08/2016 22:43, Alexei Starovoitov wrote:
>>>>>>> On Sat, Aug 27, 2016 at 09:35:14PM +0200, Mickaël Salaün wrote:
>>>>>>>> On 27/08/2016 20:06, Alexei Starovoitov wrote:
>>>>>>>>> On Sat, Aug 27, 2016 at 04:06:38PM +0200, Mickaël Salaün wrote:
>>>>>>>>>> As said above, Landlock will not run an eBPF programs when not strictly
>>>>>>>>>> needed. Attaching to a cgroup will have the same performance impact as
>>>>>>>>>> attaching to a process hierarchy.
>>>>>>>>>
>>>>>>>>> Having a prog per cgroup per lsm_hook is the only scalable way I
>>>>>>>>> could come up with. If you see another way, please propose.
>>>>>>>>> current->seccomp.landlock_prog is not the answer.
>>>>>>>>
>>>>>>>> Hum, I don't see the difference from a performance point of view between
>>>>>>>> a cgroup-based or a process hierarchy-based system.
>>>>>>>>
>>>>>>>> Maybe a better option should be to use an array of pointers with N
>>>>>>>> entries, one for each supported hook, instead of a unique pointer list?
>>>>>>>
>>>>>>> yes, clearly array dereference is faster than link list walk.
>>>>>>> Now the question is where to keep this prog_array[num_lsm_hooks] ?
>>>>>>> Since we cannot keep it inside task_struct, we have to allocate it.
>>>>>>> Every time the task is creted then. What to do on the fork? That
>>>>>>> will require changes all over. Then the obvious optimization would be
>>>>>>> to share this allocated array of prog pointers across multiple tasks...
>>>>>>> and little by little this new facility will look like cgroup.
>>>>>>> Hence the suggestion to put this array into cgroup from the start.
>>>>>>
>>>>>> I see your point :)
>>>>>>
>>>>>>>
>>>>>>>> Anyway, being able to attach an LSM hook program to a cgroup thanks to
>>>>>>>> the new BPF_PROG_ATTACH seems a good idea (while keeping the possibility
>>>>>>>> to use a process hierarchy). The downside will be to handle an LSM hook
>>>>>>>> program which is not triggered by a seccomp-filter, but this should be
>>>>>>>> needed anyway to handle interruptions.
>>>>>>>
>>>>>>> what do you mean 'not triggered by seccomp' ?
>>>>>>> You're not suggesting that this lsm has to enable seccomp to be functional?
>>>>>>> imo that's non starter due to overhead.
>>>>>>
>>>>>> Yes, for now, it is triggered by a new seccomp filter return value
>>>>>> RET_LANDLOCK, which can take a 16-bit value called cookie. This must not
>>>>>> be needed but could be useful to bind a seccomp filter security policy
>>>>>> with a Landlock one. Waiting for Kees's point of view…
>>>>>>
>>>>>
>>>>> I'm not Kees, but I'd be okay with that.  I still think that doing
>>>>> this by process hierarchy a la seccomp will be easier to use and to
>>>>> understand (which is quite important for this kind of work) than doing
>>>>> it by cgroup.
>>>>>
>>>>> A feature I've wanted to add for a while is to have an fd that
>>>>> represents a seccomp layer, the idea being that you would set up your
>>>>> seccomp layer (with syscall filter, landlock hooks, etc) and then you
>>>>> would have a syscall to install that layer.  Then an unprivileged
>>>>> sandbox manager could set up its layer and still be able to inject new
>>>>> processes into it later on, no cgroups needed.
>>>>
>>>> A nice thing I didn't highlight about Landlock is that a process can
>>>> prepare a layer of rules (arraymap of handles + Landlock programs) and
>>>> pass the file descriptors of the Landlock programs to another process.
>>>> This process could then apply this programs to get sandboxed. However,
>>>> for now, because a Landlock program is only triggered by a seccomp
>>>> filter (which do not follow the Landlock programs as a FD), they will be
>>>> useless.
>>>>
>>>> The FD referring to an arraymap of handles can also be used to update a
>>>> map and change the behavior of a Landlock program. A master process can
>>>> then add or remove restrictions to another process hierarchy on the fly.
>>>
>>> Maybe this could be extended a little bit.  The fd could hold the
>>> seccomp filter *and* the LSM hook filters.  FMODE_EXECUTE could give
>>> the ability to install it and FMODE_WRITE could give the ability to
>>> modify it.
>>>
>>
>> This is interesting! It should be possible to append the seccomp stack
>> of a source process to the seccomp stack of the target process when a
>> Landlock program is passed and then activated through seccomp(2).
>>
>> For the FMODE_EXECUTE/FMODE_WRITE, are you suggesting to manage
>> permission of the eBPF program FD in a specific way?
>>
> 
> This wouldn't be an eBPF program FD -- it would be an FD encapsulating
> an entire configuration including seccomp BPF program, whatever
> landlock stuff is associated, and eventual seccomp monitor
> configuration (once I write that code), etc.
> 
> You wouldn't say "attach this process's seccomp stack to me" -- you'd
> say "attach this seccomp layer to me".
> 
> A decision that we'd have to make would be whether the FD links to the
> parent layer or whether it can be attached without regard to what the
> parent layer is.

OK, I like that, but I think it could be done on a second time. :)

[toc] | [prev] | [next] | [standalone]

#1472868 — Re: [RFC v2 09/10] landlock: Handle cgroups (performance)

On Tue, Aug 30, 2016 at 10:33:31PM +0200, Mickaël Salaün wrote:
> 
> 
> On 30/08/2016 22:23, Andy Lutomirski wrote:
> > On Tue, Aug 30, 2016 at 1:20 PM, Mickaël Salaün <mic@digikod.net> wrote:
> >>
> >> On 30/08/2016 20:55, Andy Lutomirski wrote:
> >>> On Sun, Aug 28, 2016 at 2:42 AM, Mickaël Salaün <mic@digikod.net> wrote:
> >>>>
> >>>>
> >>>> On 28/08/2016 10:13, Andy Lutomirski wrote:
> >>>>> On Aug 27, 2016 11:14 PM, "Mickaël Salaün" <mic@digikod.net> wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 27/08/2016 22:43, Alexei Starovoitov wrote:
> >>>>>>> On Sat, Aug 27, 2016 at 09:35:14PM +0200, Mickaël Salaün wrote:
> >>>>>>>> On 27/08/2016 20:06, Alexei Starovoitov wrote:
> >>>>>>>>> On Sat, Aug 27, 2016 at 04:06:38PM +0200, Mickaël Salaün wrote:
> >>>>>>>>>> As said above, Landlock will not run an eBPF programs when not strictly
> >>>>>>>>>> needed. Attaching to a cgroup will have the same performance impact as
> >>>>>>>>>> attaching to a process hierarchy.
> >>>>>>>>>
> >>>>>>>>> Having a prog per cgroup per lsm_hook is the only scalable way I
> >>>>>>>>> could come up with. If you see another way, please propose.
> >>>>>>>>> current->seccomp.landlock_prog is not the answer.
> >>>>>>>>
> >>>>>>>> Hum, I don't see the difference from a performance point of view between
> >>>>>>>> a cgroup-based or a process hierarchy-based system.
> >>>>>>>>
> >>>>>>>> Maybe a better option should be to use an array of pointers with N
> >>>>>>>> entries, one for each supported hook, instead of a unique pointer list?
> >>>>>>>
> >>>>>>> yes, clearly array dereference is faster than link list walk.
> >>>>>>> Now the question is where to keep this prog_array[num_lsm_hooks] ?
> >>>>>>> Since we cannot keep it inside task_struct, we have to allocate it.
> >>>>>>> Every time the task is creted then. What to do on the fork? That
> >>>>>>> will require changes all over. Then the obvious optimization would be
> >>>>>>> to share this allocated array of prog pointers across multiple tasks...
> >>>>>>> and little by little this new facility will look like cgroup.
> >>>>>>> Hence the suggestion to put this array into cgroup from the start.
> >>>>>>
> >>>>>> I see your point :)
> >>>>>>
> >>>>>>>
> >>>>>>>> Anyway, being able to attach an LSM hook program to a cgroup thanks to
> >>>>>>>> the new BPF_PROG_ATTACH seems a good idea (while keeping the possibility
> >>>>>>>> to use a process hierarchy). The downside will be to handle an LSM hook
> >>>>>>>> program which is not triggered by a seccomp-filter, but this should be
> >>>>>>>> needed anyway to handle interruptions.
> >>>>>>>
> >>>>>>> what do you mean 'not triggered by seccomp' ?
> >>>>>>> You're not suggesting that this lsm has to enable seccomp to be functional?
> >>>>>>> imo that's non starter due to overhead.
> >>>>>>
> >>>>>> Yes, for now, it is triggered by a new seccomp filter return value
> >>>>>> RET_LANDLOCK, which can take a 16-bit value called cookie. This must not
> >>>>>> be needed but could be useful to bind a seccomp filter security policy
> >>>>>> with a Landlock one. Waiting for Kees's point of view…
> >>>>>>
> >>>>>
> >>>>> I'm not Kees, but I'd be okay with that.  I still think that doing
> >>>>> this by process hierarchy a la seccomp will be easier to use and to
> >>>>> understand (which is quite important for this kind of work) than doing
> >>>>> it by cgroup.
> >>>>>
> >>>>> A feature I've wanted to add for a while is to have an fd that
> >>>>> represents a seccomp layer, the idea being that you would set up your
> >>>>> seccomp layer (with syscall filter, landlock hooks, etc) and then you
> >>>>> would have a syscall to install that layer.  Then an unprivileged
> >>>>> sandbox manager could set up its layer and still be able to inject new
> >>>>> processes into it later on, no cgroups needed.
> >>>>
> >>>> A nice thing I didn't highlight about Landlock is that a process can
> >>>> prepare a layer of rules (arraymap of handles + Landlock programs) and
> >>>> pass the file descriptors of the Landlock programs to another process.
> >>>> This process could then apply this programs to get sandboxed. However,
> >>>> for now, because a Landlock program is only triggered by a seccomp
> >>>> filter (which do not follow the Landlock programs as a FD), they will be
> >>>> useless.
> >>>>
> >>>> The FD referring to an arraymap of handles can also be used to update a
> >>>> map and change the behavior of a Landlock program. A master process can
> >>>> then add or remove restrictions to another process hierarchy on the fly.
> >>>
> >>> Maybe this could be extended a little bit.  The fd could hold the
> >>> seccomp filter *and* the LSM hook filters.  FMODE_EXECUTE could give
> >>> the ability to install it and FMODE_WRITE could give the ability to
> >>> modify it.
> >>>
> >>
> >> This is interesting! It should be possible to append the seccomp stack
> >> of a source process to the seccomp stack of the target process when a
> >> Landlock program is passed and then activated through seccomp(2).
> >>
> >> For the FMODE_EXECUTE/FMODE_WRITE, are you suggesting to manage
> >> permission of the eBPF program FD in a specific way?
> >>
> > 
> > This wouldn't be an eBPF program FD -- it would be an FD encapsulating
> > an entire configuration including seccomp BPF program, whatever
> > landlock stuff is associated, and eventual seccomp monitor
> > configuration (once I write that code), etc.
> > 
> > You wouldn't say "attach this process's seccomp stack to me" -- you'd
> > say "attach this seccomp layer to me".
> > 
> > A decision that we'd have to make would be whether the FD links to the
> > parent layer or whether it can be attached without regard to what the
> > parent layer is.
> 
> OK, I like that, but I think it could be done on a second time. :)

I don't. Single FD that is a collection of objects seems an odd abstraction
to me. I also don't see what it actually solves.
I think lsm and seccomp should be orthogonal and not tied into each other.

[toc] | [prev] | [next] | [standalone]

#1472892 — Re: [RFC v2 09/10] landlock: Handle cgroups (performance)

On Aug 30, 2016 1:56 PM, "Alexei Starovoitov"
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Aug 30, 2016 at 10:33:31PM +0200, Mickaël Salaün wrote:
> >
> >
> > On 30/08/2016 22:23, Andy Lutomirski wrote:
> > > On Tue, Aug 30, 2016 at 1:20 PM, Mickaël Salaün <mic@digikod.net> wrote:
> > >>
> > >> On 30/08/2016 20:55, Andy Lutomirski wrote:
> > >>> On Sun, Aug 28, 2016 at 2:42 AM, Mickaël Salaün <mic@digikod.net> wrote:
> > >>>>
> > >>>>
> > >>>> On 28/08/2016 10:13, Andy Lutomirski wrote:
> > >>>>> On Aug 27, 2016 11:14 PM, "Mickaël Salaün" <mic@digikod.net> wrote:
> > >>>>>>
> > >>>>>>
> > >>>>>> On 27/08/2016 22:43, Alexei Starovoitov wrote:
> > >>>>>>> On Sat, Aug 27, 2016 at 09:35:14PM +0200, Mickaël Salaün wrote:
> > >>>>>>>> On 27/08/2016 20:06, Alexei Starovoitov wrote:
> > >>>>>>>>> On Sat, Aug 27, 2016 at 04:06:38PM +0200, Mickaël Salaün wrote:
> > >>>>>>>>>> As said above, Landlock will not run an eBPF programs when not strictly
> > >>>>>>>>>> needed. Attaching to a cgroup will have the same performance impact as
> > >>>>>>>>>> attaching to a process hierarchy.
> > >>>>>>>>>
> > >>>>>>>>> Having a prog per cgroup per lsm_hook is the only scalable way I
> > >>>>>>>>> could come up with. If you see another way, please propose.
> > >>>>>>>>> current->seccomp.landlock_prog is not the answer.
> > >>>>>>>>
> > >>>>>>>> Hum, I don't see the difference from a performance point of view between
> > >>>>>>>> a cgroup-based or a process hierarchy-based system.
> > >>>>>>>>
> > >>>>>>>> Maybe a better option should be to use an array of pointers with N
> > >>>>>>>> entries, one for each supported hook, instead of a unique pointer list?
> > >>>>>>>
> > >>>>>>> yes, clearly array dereference is faster than link list walk.
> > >>>>>>> Now the question is where to keep this prog_array[num_lsm_hooks] ?
> > >>>>>>> Since we cannot keep it inside task_struct, we have to allocate it.
> > >>>>>>> Every time the task is creted then. What to do on the fork? That
> > >>>>>>> will require changes all over. Then the obvious optimization would be
> > >>>>>>> to share this allocated array of prog pointers across multiple tasks...
> > >>>>>>> and little by little this new facility will look like cgroup.
> > >>>>>>> Hence the suggestion to put this array into cgroup from the start.
> > >>>>>>
> > >>>>>> I see your point :)
> > >>>>>>
> > >>>>>>>
> > >>>>>>>> Anyway, being able to attach an LSM hook program to a cgroup thanks to
> > >>>>>>>> the new BPF_PROG_ATTACH seems a good idea (while keeping the possibility
> > >>>>>>>> to use a process hierarchy). The downside will be to handle an LSM hook
> > >>>>>>>> program which is not triggered by a seccomp-filter, but this should be
> > >>>>>>>> needed anyway to handle interruptions.
> > >>>>>>>
> > >>>>>>> what do you mean 'not triggered by seccomp' ?
> > >>>>>>> You're not suggesting that this lsm has to enable seccomp to be functional?
> > >>>>>>> imo that's non starter due to overhead.
> > >>>>>>
> > >>>>>> Yes, for now, it is triggered by a new seccomp filter return value
> > >>>>>> RET_LANDLOCK, which can take a 16-bit value called cookie. This must not
> > >>>>>> be needed but could be useful to bind a seccomp filter security policy
> > >>>>>> with a Landlock one. Waiting for Kees's point of view…
> > >>>>>>
> > >>>>>
> > >>>>> I'm not Kees, but I'd be okay with that.  I still think that doing
> > >>>>> this by process hierarchy a la seccomp will be easier to use and to
> > >>>>> understand (which is quite important for this kind of work) than doing
> > >>>>> it by cgroup.
> > >>>>>
> > >>>>> A feature I've wanted to add for a while is to have an fd that
> > >>>>> represents a seccomp layer, the idea being that you would set up your
> > >>>>> seccomp layer (with syscall filter, landlock hooks, etc) and then you
> > >>>>> would have a syscall to install that layer.  Then an unprivileged
> > >>>>> sandbox manager could set up its layer and still be able to inject new
> > >>>>> processes into it later on, no cgroups needed.
> > >>>>
> > >>>> A nice thing I didn't highlight about Landlock is that a process can
> > >>>> prepare a layer of rules (arraymap of handles + Landlock programs) and
> > >>>> pass the file descriptors of the Landlock programs to another process.
> > >>>> This process could then apply this programs to get sandboxed. However,
> > >>>> for now, because a Landlock program is only triggered by a seccomp
> > >>>> filter (which do not follow the Landlock programs as a FD), they will be
> > >>>> useless.
> > >>>>
> > >>>> The FD referring to an arraymap of handles can also be used to update a
> > >>>> map and change the behavior of a Landlock program. A master process can
> > >>>> then add or remove restrictions to another process hierarchy on the fly.
> > >>>
> > >>> Maybe this could be extended a little bit.  The fd could hold the
> > >>> seccomp filter *and* the LSM hook filters.  FMODE_EXECUTE could give
> > >>> the ability to install it and FMODE_WRITE could give the ability to
> > >>> modify it.
> > >>>
> > >>
> > >> This is interesting! It should be possible to append the seccomp stack
> > >> of a source process to the seccomp stack of the target process when a
> > >> Landlock program is passed and then activated through seccomp(2).
> > >>
> > >> For the FMODE_EXECUTE/FMODE_WRITE, are you suggesting to manage
> > >> permission of the eBPF program FD in a specific way?
> > >>
> > >
> > > This wouldn't be an eBPF program FD -- it would be an FD encapsulating
> > > an entire configuration including seccomp BPF program, whatever
> > > landlock stuff is associated, and eventual seccomp monitor
> > > configuration (once I write that code), etc.
> > >
> > > You wouldn't say "attach this process's seccomp stack to me" -- you'd
> > > say "attach this seccomp layer to me".
> > >
> > > A decision that we'd have to make would be whether the FD links to the
> > > parent layer or whether it can be attached without regard to what the
> > > parent layer is.
> >
> > OK, I like that, but I think it could be done on a second time. :)
>
> I don't. Single FD that is a collection of objects seems an odd abstraction
> to me. I also don't see what it actually solves.
> I think lsm and seccomp should be orthogonal and not tied into each other.
>

It's not a random collection of objects.  It's a fully configured
sandboxing layer.

One might argue that landlock shouldn't be tied to seccomp (in theory,
attached progs could be given access to syscall_get_xyz()), but I
think that the seccomp attachment mechanism is the right way to
install unprivileged filters.  It handles the no_new_privs stuff, it
allows TSYNC, it's totally independent of systemwide policy, etc.

Trying to use cgroups or similar for this is going to be much nastier.
Some tighter sandboxes (Sandstorm, etc) aren't even going to dream of
putting cgroupfs in their containers, so requiring cgroups or similar
would be a mess for that type of application.

--Andy

[toc] | [prev] | [next] | [standalone]

#1472971 — Re: [RFC v2 09/10] landlock: Handle cgroups (performance)

On Tue, Aug 30, 2016 at 02:45:14PM -0700, Andy Lutomirski wrote:
> 
> One might argue that landlock shouldn't be tied to seccomp (in theory,
> attached progs could be given access to syscall_get_xyz()), but I

proposed lsm is way more powerful than syscall_get_xyz.
no need to dumb it down.

> think that the seccomp attachment mechanism is the right way to
> install unprivileged filters.  It handles the no_new_privs stuff, it
> allows TSYNC, it's totally independent of systemwide policy, etc.
> 
> Trying to use cgroups or similar for this is going to be much nastier.
> Some tighter sandboxes (Sandstorm, etc) aren't even going to dream of
> putting cgroupfs in their containers, so requiring cgroups or similar
> would be a mess for that type of application.

I don't see why it is a 'mess'. cgroups are already used by majority
of the systems, so I don't see why requiring a cgroup is such a big deal.
But let's say we don't do them. How implementation is going to look like
for task based hierarchy? Note that we need an array of bpf_prog pointers.
One for each lsm hook. Where this array is going to be stored?
We cannot put in task_struct, since it's too large. Cannot put it
into 'struct seccomp' directly either, unless it will become a pointer.
Is that the proposal?
So now we will be wasting extra 1kbyte of memory per task. Not great.
We'd want to optimize it by sharing this such struct seccomp with prog array
across threads of the same task? Or dynimically allocating it when
landlock is in use? May sound nice, but how to account for that kernel
memory? I guess also solvable by charging memlock.
With cgroup based approach we don't need to worry about all that.

[toc] | [prev] | [next] | [standalone]

#1473001 — Re: [RFC v2 09/10] landlock: Handle cgroups (performance)

On Tue, Aug 30, 2016 at 6:36 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Tue, Aug 30, 2016 at 02:45:14PM -0700, Andy Lutomirski wrote:
>>
>> One might argue that landlock shouldn't be tied to seccomp (in theory,
>> attached progs could be given access to syscall_get_xyz()), but I
>
> proposed lsm is way more powerful than syscall_get_xyz.
> no need to dumb it down.

I think you're misunderstanding me.

Mickaël's code allows one to make the LSM hook filters depend on the
syscall using SECCOMP_RET_LANDLOCK.  I'm suggesting that a similar
effect could be achieved by allowing the eBPF LSM hook to call
syscall_get_xyz() if it wants to.

>
>> think that the seccomp attachment mechanism is the right way to
>> install unprivileged filters.  It handles the no_new_privs stuff, it
>> allows TSYNC, it's totally independent of systemwide policy, etc.
>>
>> Trying to use cgroups or similar for this is going to be much nastier.
>> Some tighter sandboxes (Sandstorm, etc) aren't even going to dream of
>> putting cgroupfs in their containers, so requiring cgroups or similar
>> would be a mess for that type of application.
>
> I don't see why it is a 'mess'. cgroups are already used by majority
> of the systems, so I don't see why requiring a cgroup is such a big deal.

Requiring cgroup to be configured in isn't a big deal.  Requiring

> But let's say we don't do them. How implementation is going to look like
> for task based hierarchy? Note that we need an array of bpf_prog pointers.
> One for each lsm hook. Where this array is going to be stored?
> We cannot put in task_struct, since it's too large. Cannot put it
> into 'struct seccomp' directly either, unless it will become a pointer.
> Is that the proposal?

It would go in struct seccomp_filter or in something pointed to from there.

> So now we will be wasting extra 1kbyte of memory per task. Not great.
> We'd want to optimize it by sharing this such struct seccomp with prog array
> across threads of the same task? Or dynimically allocating it when
> landlock is in use? May sound nice, but how to account for that kernel
> memory? I guess also solvable by charging memlock.
> With cgroup based approach we don't need to worry about all that.
>

The considerations are essentially identical either way.

With cgroups, if you want to share the memory between multiple
separate sandboxes (Firejail instances, Sandstorm grains, Chromium
instances, xdg-apps, etc), you'd need to get them to all coordinate to
share a cgroup.  With a seccomp-like interface, you'd need to get them
to coordinate to share an installed layer (using my FD idea or
similar).

There would *not* be any duplication of this memory just because a
sandboxed process called fork().

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

[toc] | [prev] | [next] | [standalone]

#1471189 — Re: [RFC v2 09/10] landlock: Handle cgroups (netfilter match)

[Multipart message — attachments visible in raw view] — view raw

On 27/08/2016 01:05, Alexei Starovoitov wrote:
> On Fri, Aug 26, 2016 at 05:10:40PM +0200, Mickaël Salaün wrote:
>> To sum up, there is four related patchsets:
>> * "Landlock LSM: Unprivileged sandboxing" (this series)
>> * "Add Checmate, BPF-driven minor LSM" (Sargun Dhillon)
>> * "Networking cgroup controller" (Anoop Naravaram)
>> * "Add eBPF hooks for cgroups" (Daniel Mack)

>>> Anoop Naravaram's use case is to control the ports the applications
>>> under cgroup can bind and listen on.
>>> Such use case can be solved by such 'lsm cgroup controller' by
>>> attaching bpf program to security_socket_bind lsm hook and
>>> filtering sockaddr.
>>> Furthermore Sargun's use case is to allow further sockaddr rewrites
>>> from the bpf program which can be done as natural extension
>>> of such mechanism.
>>>
>>> If I understood Daniel's Anoop's Sargun's and yours use cases
>>> correctly the common piece of kernel infrastructure that can solve
>>> them all can start from Daniel's current set of patches that
>>> establish a mechanism of attaching bpf program to a cgroup.
>>> Then adding lsm hooks to it and later allowing argument rewrite
>>> (since they're already in the kernel and no ToCToU problems exist)

>> For the network-related series, I think it make more sense to simply
>> create a netfilter rule matching a cgroup and then add more features to
>> netfilter (restrict port ranges and so on) thanks to eBPF programs.
>> Containers are (usually) in a dedicated network namespace, which open
>> the possibility to not only rely on cgroups (e.g. match UID,
>> netmask...). It would also be more flexible to be able to load a BPF
>> program in netfilter and update its maps on the fly to make dynamic
>> rules, like ipset does, but in a more generic way.

What do the netdev folks think about this design?

[toc] | [prev] | [next] | [standalone]

#1471238 — Re: [RFC v2 09/10] landlock: Handle cgroups (netfilter match)

On Sat, Aug 27, 2016 at 04:19:05PM +0200, Mickaël Salaün wrote:
> 
> On 27/08/2016 01:05, Alexei Starovoitov wrote:
> > On Fri, Aug 26, 2016 at 05:10:40PM +0200, Mickaël Salaün wrote:
> >> To sum up, there is four related patchsets:
> >> * "Landlock LSM: Unprivileged sandboxing" (this series)
> >> * "Add Checmate, BPF-driven minor LSM" (Sargun Dhillon)
> >> * "Networking cgroup controller" (Anoop Naravaram)
> >> * "Add eBPF hooks for cgroups" (Daniel Mack)
> 
> >>> Anoop Naravaram's use case is to control the ports the applications
> >>> under cgroup can bind and listen on.
> >>> Such use case can be solved by such 'lsm cgroup controller' by
> >>> attaching bpf program to security_socket_bind lsm hook and
> >>> filtering sockaddr.
> >>> Furthermore Sargun's use case is to allow further sockaddr rewrites
> >>> from the bpf program which can be done as natural extension
> >>> of such mechanism.
> >>>
> >>> If I understood Daniel's Anoop's Sargun's and yours use cases
> >>> correctly the common piece of kernel infrastructure that can solve
> >>> them all can start from Daniel's current set of patches that
> >>> establish a mechanism of attaching bpf program to a cgroup.
> >>> Then adding lsm hooks to it and later allowing argument rewrite
> >>> (since they're already in the kernel and no ToCToU problems exist)
> 
> >> For the network-related series, I think it make more sense to simply
> >> create a netfilter rule matching a cgroup and then add more features to
> >> netfilter (restrict port ranges and so on) thanks to eBPF programs.
> >> Containers are (usually) in a dedicated network namespace, which open
> >> the possibility to not only rely on cgroups (e.g. match UID,
> >> netmask...). It would also be more flexible to be able to load a BPF
> >> program in netfilter and update its maps on the fly to make dynamic
> >> rules, like ipset does, but in a more generic way.
> 
> What do the netdev folks think about this design?

such design doesn't scale when used for container management and
that's what we need to solve.
netns has its overhead and management issues. There are proposals to
solve that but that is orthogonal to containers in general.
A lot of them don't use netns.

[toc] | [prev] | [next] | [standalone]

#1471190 — Re: [RFC v2 09/10] landlock: Handle cgroups (program types)

[Multipart message — attachments visible in raw view] — view raw

On 27/08/2016 01:05, Alexei Starovoitov wrote:
> On Fri, Aug 26, 2016 at 05:10:40PM +0200, Mickaël Salaün wrote:
>
>>> As far as safety and type checking that bpf programs has to do,
>>> I like the approach of patch 06/10:
>>> +LANDLOCK_HOOK2(file_open, FILE_OPEN,
>>> +       PTR_TO_STRUCT_FILE, struct file *, file,
>>> +       PTR_TO_STRUCT_CRED, const struct cred *, cred
>>> +)
>>> teaching verifier to recognize struct file, cred, sockaddr
>>> will let bpf program access them naturally without any overhead.
>>> Though:
>>> @@ -102,6 +102,9 @@ enum bpf_prog_type {
>>>         BPF_PROG_TYPE_SCHED_CLS,
>>>         BPF_PROG_TYPE_SCHED_ACT,
>>>         BPF_PROG_TYPE_TRACEPOINT,
>>> +       BPF_PROG_TYPE_LANDLOCK_FILE_OPEN,
>>> +       BPF_PROG_TYPE_LANDLOCK_FILE_PERMISSION,
>>> +       BPF_PROG_TYPE_LANDLOCK_MMAP_FILE,
>>>  };
>>> is a bit of overkill.
>>> I think it would be cleaner to have single
>>> BPF_PROG_TYPE_LSM and at program load time pass
>>> lsm_hook_id as well, so that verifier can do safety checks
>>> based on type info provided in LANDLOCK_HOOKs
>>
>> I first started with a unique BPF_PROG_TYPE but, the thing is, the BPF
>> verifier check programs according to their types. If we need to check
>> specific context value types (e.g. PTR_TO_STRUCT_FILE), we need a
>> dedicated program types. I don't see any other way to do it with the
>> current verifier code. Moreover it's the purpose of program types, right?
> 
> Adding new bpf program type for every lsm hook is not acceptable.
> Either do one new program type + pass lsm_hook_id as suggested
> or please come up with an alternative approach.

OK, so we have to modify the verifier to not only rely on the program
type but on another value to check the context accesses. Do you have a
hint from where this value could come from? Do we need to add a new bpf
command to associate a program to a subtype?

[toc] | [prev] | [next] | [standalone]

#1471237 — Re: [RFC v2 09/10] landlock: Handle cgroups (program types)

On Sat, Aug 27, 2016 at 04:34:55PM +0200, Mickaël Salaün wrote:
> 
> On 27/08/2016 01:05, Alexei Starovoitov wrote:
> > On Fri, Aug 26, 2016 at 05:10:40PM +0200, Mickaël Salaün wrote:
> >
> >>> As far as safety and type checking that bpf programs has to do,
> >>> I like the approach of patch 06/10:
> >>> +LANDLOCK_HOOK2(file_open, FILE_OPEN,
> >>> +       PTR_TO_STRUCT_FILE, struct file *, file,
> >>> +       PTR_TO_STRUCT_CRED, const struct cred *, cred
> >>> +)
> >>> teaching verifier to recognize struct file, cred, sockaddr
> >>> will let bpf program access them naturally without any overhead.
> >>> Though:
> >>> @@ -102,6 +102,9 @@ enum bpf_prog_type {
> >>>         BPF_PROG_TYPE_SCHED_CLS,
> >>>         BPF_PROG_TYPE_SCHED_ACT,
> >>>         BPF_PROG_TYPE_TRACEPOINT,
> >>> +       BPF_PROG_TYPE_LANDLOCK_FILE_OPEN,
> >>> +       BPF_PROG_TYPE_LANDLOCK_FILE_PERMISSION,
> >>> +       BPF_PROG_TYPE_LANDLOCK_MMAP_FILE,
> >>>  };
> >>> is a bit of overkill.
> >>> I think it would be cleaner to have single
> >>> BPF_PROG_TYPE_LSM and at program load time pass
> >>> lsm_hook_id as well, so that verifier can do safety checks
> >>> based on type info provided in LANDLOCK_HOOKs
> >>
> >> I first started with a unique BPF_PROG_TYPE but, the thing is, the BPF
> >> verifier check programs according to their types. If we need to check
> >> specific context value types (e.g. PTR_TO_STRUCT_FILE), we need a
> >> dedicated program types. I don't see any other way to do it with the
> >> current verifier code. Moreover it's the purpose of program types, right?
> > 
> > Adding new bpf program type for every lsm hook is not acceptable.
> > Either do one new program type + pass lsm_hook_id as suggested
> > or please come up with an alternative approach.
> 
> OK, so we have to modify the verifier to not only rely on the program
> type but on another value to check the context accesses. Do you have a
> hint from where this value could come from? Do we need to add a new bpf
> command to associate a program to a subtype?

It's another field prog_subtype (or prog_hook_id) in union bpf_attr.
Both prog_type and prog_hook_id are used during verification.
prog_type distinguishes the main aspects whereas prog_hook_id selects
which lsm_hook's argument definition to apply.
At the time of attaching to a hook, the prog_hook_id passed at the
load time should match lsm's hook_id.

[toc] | [prev] | [next] | [standalone]

#1471245 — Re: [RFC v2 09/10] landlock: Handle cgroups (program types)

[Multipart message — attachments visible in raw view] — view raw

On 27/08/2016 20:19, Alexei Starovoitov wrote:
> On Sat, Aug 27, 2016 at 04:34:55PM +0200, Mickaël Salaün wrote:
>>
>> On 27/08/2016 01:05, Alexei Starovoitov wrote:
>>> On Fri, Aug 26, 2016 at 05:10:40PM +0200, Mickaël Salaün wrote:
>>>
>>>>> As far as safety and type checking that bpf programs has to do,
>>>>> I like the approach of patch 06/10:
>>>>> +LANDLOCK_HOOK2(file_open, FILE_OPEN,
>>>>> +       PTR_TO_STRUCT_FILE, struct file *, file,
>>>>> +       PTR_TO_STRUCT_CRED, const struct cred *, cred
>>>>> +)
>>>>> teaching verifier to recognize struct file, cred, sockaddr
>>>>> will let bpf program access them naturally without any overhead.
>>>>> Though:
>>>>> @@ -102,6 +102,9 @@ enum bpf_prog_type {
>>>>>         BPF_PROG_TYPE_SCHED_CLS,
>>>>>         BPF_PROG_TYPE_SCHED_ACT,
>>>>>         BPF_PROG_TYPE_TRACEPOINT,
>>>>> +       BPF_PROG_TYPE_LANDLOCK_FILE_OPEN,
>>>>> +       BPF_PROG_TYPE_LANDLOCK_FILE_PERMISSION,
>>>>> +       BPF_PROG_TYPE_LANDLOCK_MMAP_FILE,
>>>>>  };
>>>>> is a bit of overkill.
>>>>> I think it would be cleaner to have single
>>>>> BPF_PROG_TYPE_LSM and at program load time pass
>>>>> lsm_hook_id as well, so that verifier can do safety checks
>>>>> based on type info provided in LANDLOCK_HOOKs
>>>>
>>>> I first started with a unique BPF_PROG_TYPE but, the thing is, the BPF
>>>> verifier check programs according to their types. If we need to check
>>>> specific context value types (e.g. PTR_TO_STRUCT_FILE), we need a
>>>> dedicated program types. I don't see any other way to do it with the
>>>> current verifier code. Moreover it's the purpose of program types, right?
>>>
>>> Adding new bpf program type for every lsm hook is not acceptable.
>>> Either do one new program type + pass lsm_hook_id as suggested
>>> or please come up with an alternative approach.
>>
>> OK, so we have to modify the verifier to not only rely on the program
>> type but on another value to check the context accesses. Do you have a
>> hint from where this value could come from? Do we need to add a new bpf
>> command to associate a program to a subtype?
> 
> It's another field prog_subtype (or prog_hook_id) in union bpf_attr.
> Both prog_type and prog_hook_id are used during verification.
> prog_type distinguishes the main aspects whereas prog_hook_id selects
> which lsm_hook's argument definition to apply.
> At the time of attaching to a hook, the prog_hook_id passed at the
> load time should match lsm's hook_id.

OK, so this new prog_subtype field should be use/set by a new bpf_cmd,
right? Something like BPF_PROG_SUBTYPE or BPF_PROG_METADATA?

[toc] | [prev] | [next] | [standalone]

#1471248 — Re: [RFC v2 09/10] landlock: Handle cgroups (program types)

On Sat, Aug 27, 2016 at 09:55:01PM +0200, Mickaël Salaün wrote:
> 
> On 27/08/2016 20:19, Alexei Starovoitov wrote:
> > On Sat, Aug 27, 2016 at 04:34:55PM +0200, Mickaël Salaün wrote:
> >>
> >> On 27/08/2016 01:05, Alexei Starovoitov wrote:
> >>> On Fri, Aug 26, 2016 at 05:10:40PM +0200, Mickaël Salaün wrote:
> >>>
> >>>>> As far as safety and type checking that bpf programs has to do,
> >>>>> I like the approach of patch 06/10:
> >>>>> +LANDLOCK_HOOK2(file_open, FILE_OPEN,
> >>>>> +       PTR_TO_STRUCT_FILE, struct file *, file,
> >>>>> +       PTR_TO_STRUCT_CRED, const struct cred *, cred
> >>>>> +)
> >>>>> teaching verifier to recognize struct file, cred, sockaddr
> >>>>> will let bpf program access them naturally without any overhead.
> >>>>> Though:
> >>>>> @@ -102,6 +102,9 @@ enum bpf_prog_type {
> >>>>>         BPF_PROG_TYPE_SCHED_CLS,
> >>>>>         BPF_PROG_TYPE_SCHED_ACT,
> >>>>>         BPF_PROG_TYPE_TRACEPOINT,
> >>>>> +       BPF_PROG_TYPE_LANDLOCK_FILE_OPEN,
> >>>>> +       BPF_PROG_TYPE_LANDLOCK_FILE_PERMISSION,
> >>>>> +       BPF_PROG_TYPE_LANDLOCK_MMAP_FILE,
> >>>>>  };
> >>>>> is a bit of overkill.
> >>>>> I think it would be cleaner to have single
> >>>>> BPF_PROG_TYPE_LSM and at program load time pass
> >>>>> lsm_hook_id as well, so that verifier can do safety checks
> >>>>> based on type info provided in LANDLOCK_HOOKs
> >>>>
> >>>> I first started with a unique BPF_PROG_TYPE but, the thing is, the BPF
> >>>> verifier check programs according to their types. If we need to check
> >>>> specific context value types (e.g. PTR_TO_STRUCT_FILE), we need a
> >>>> dedicated program types. I don't see any other way to do it with the
> >>>> current verifier code. Moreover it's the purpose of program types, right?
> >>>
> >>> Adding new bpf program type for every lsm hook is not acceptable.
> >>> Either do one new program type + pass lsm_hook_id as suggested
> >>> or please come up with an alternative approach.
> >>
> >> OK, so we have to modify the verifier to not only rely on the program
> >> type but on another value to check the context accesses. Do you have a
> >> hint from where this value could come from? Do we need to add a new bpf
> >> command to associate a program to a subtype?
> > 
> > It's another field prog_subtype (or prog_hook_id) in union bpf_attr.
> > Both prog_type and prog_hook_id are used during verification.
> > prog_type distinguishes the main aspects whereas prog_hook_id selects
> > which lsm_hook's argument definition to apply.
> > At the time of attaching to a hook, the prog_hook_id passed at the
> > load time should match lsm's hook_id.
> 
> OK, so this new prog_subtype field should be use/set by a new bpf_cmd,
> right? Something like BPF_PROG_SUBTYPE or BPF_PROG_METADATA?

No new command. It will be an optional field to existing BPF_PROG_LOAD.
In other words instead of replicating everything that different
bpf_prog_type-s need, we can pass this hook_id field to fine tune
the purpose (and applicability) of the program.
And one BPF_PROG_TYPE_LANDLOCK type will cover all lsm hooks.
For example existing BPF_PROG_TYPE_TRACEPOINT checks the safety
for all tracepoints while they all have different arguments.
For tracepoints it's easier, since the only difference between them
is the range of ctx access. Here we need strong type safety
of arguments therefore need extra hook_id at load time.

[toc] | [prev] | [next] | [standalone]

#1471251 — Re: [RFC v2 09/10] landlock: Handle cgroups (program types)

[Multipart message — attachments visible in raw view] — view raw

On 27/08/2016 22:56, Alexei Starovoitov wrote:
> On Sat, Aug 27, 2016 at 09:55:01PM +0200, Mickaël Salaün wrote:
>>
>> On 27/08/2016 20:19, Alexei Starovoitov wrote:
>>> On Sat, Aug 27, 2016 at 04:34:55PM +0200, Mickaël Salaün wrote:
>>>>
>>>> On 27/08/2016 01:05, Alexei Starovoitov wrote:
>>>>> On Fri, Aug 26, 2016 at 05:10:40PM +0200, Mickaël Salaün wrote:
>>>>>
>>>>>>> As far as safety and type checking that bpf programs has to do,
>>>>>>> I like the approach of patch 06/10:
>>>>>>> +LANDLOCK_HOOK2(file_open, FILE_OPEN,
>>>>>>> +       PTR_TO_STRUCT_FILE, struct file *, file,
>>>>>>> +       PTR_TO_STRUCT_CRED, const struct cred *, cred
>>>>>>> +)
>>>>>>> teaching verifier to recognize struct file, cred, sockaddr
>>>>>>> will let bpf program access them naturally without any overhead.
>>>>>>> Though:
>>>>>>> @@ -102,6 +102,9 @@ enum bpf_prog_type {
>>>>>>>         BPF_PROG_TYPE_SCHED_CLS,
>>>>>>>         BPF_PROG_TYPE_SCHED_ACT,
>>>>>>>         BPF_PROG_TYPE_TRACEPOINT,
>>>>>>> +       BPF_PROG_TYPE_LANDLOCK_FILE_OPEN,
>>>>>>> +       BPF_PROG_TYPE_LANDLOCK_FILE_PERMISSION,
>>>>>>> +       BPF_PROG_TYPE_LANDLOCK_MMAP_FILE,
>>>>>>>  };
>>>>>>> is a bit of overkill.
>>>>>>> I think it would be cleaner to have single
>>>>>>> BPF_PROG_TYPE_LSM and at program load time pass
>>>>>>> lsm_hook_id as well, so that verifier can do safety checks
>>>>>>> based on type info provided in LANDLOCK_HOOKs
>>>>>>
>>>>>> I first started with a unique BPF_PROG_TYPE but, the thing is, the BPF
>>>>>> verifier check programs according to their types. If we need to check
>>>>>> specific context value types (e.g. PTR_TO_STRUCT_FILE), we need a
>>>>>> dedicated program types. I don't see any other way to do it with the
>>>>>> current verifier code. Moreover it's the purpose of program types, right?
>>>>>
>>>>> Adding new bpf program type for every lsm hook is not acceptable.
>>>>> Either do one new program type + pass lsm_hook_id as suggested
>>>>> or please come up with an alternative approach.
>>>>
>>>> OK, so we have to modify the verifier to not only rely on the program
>>>> type but on another value to check the context accesses. Do you have a
>>>> hint from where this value could come from? Do we need to add a new bpf
>>>> command to associate a program to a subtype?
>>>
>>> It's another field prog_subtype (or prog_hook_id) in union bpf_attr.
>>> Both prog_type and prog_hook_id are used during verification.
>>> prog_type distinguishes the main aspects whereas prog_hook_id selects
>>> which lsm_hook's argument definition to apply.
>>> At the time of attaching to a hook, the prog_hook_id passed at the
>>> load time should match lsm's hook_id.
>>
>> OK, so this new prog_subtype field should be use/set by a new bpf_cmd,
>> right? Something like BPF_PROG_SUBTYPE or BPF_PROG_METADATA?
> 
> No new command. It will be an optional field to existing BPF_PROG_LOAD.
> In other words instead of replicating everything that different
> bpf_prog_type-s need, we can pass this hook_id field to fine tune
> the purpose (and applicability) of the program.
> And one BPF_PROG_TYPE_LANDLOCK type will cover all lsm hooks.
> For example existing BPF_PROG_TYPE_TRACEPOINT checks the safety
> for all tracepoints while they all have different arguments.
> For tracepoints it's easier, since the only difference between them
> is the range of ctx access. Here we need strong type safety
> of arguments therefore need extra hook_id at load time.

OK, I will do it.

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web