* Re: Preferred subj= with multiple LSMs
From: Simon McVittie @ 2019-07-23 14:06 UTC (permalink / raw)
To: Paul Moore
Cc: Casey Schaufler, James Morris, Steve Grubb, Richard Guy Briggs,
linux-audit@redhat.com, Linux Security Module list
In-Reply-To: <CAHC9VhSbg7BxPSA4NkQV3_1zx6cj3ej25e6fJ2FBWX9fU104rg@mail.gmail.com>
On Mon, 22 Jul 2019 at 18:30:35 -0400, Paul Moore wrote:
> On Mon, Jul 22, 2019 at 6:01 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> > I suggest that if supporting dbus well is assisted by
> > making reasonable restrictions on what constitutes a valid LSM
> > "context" that we have a good reason.
>
> I continue to believe that restrictions on the label format are a bad
> idea
Does this include the restriction "the label does not include \0",
which is an assumption that dbus is already relying on since I checked
it in the thread around
<https://marc.info/?l=linux-security-module&m=142323508321029&w=2>?
Or is that restriction so fundamental that it's considered OK?
(Other user-space tools like ls -Z and ps -Z also rely on that assumption
by printing security contexts with %s, as far as I know.)
dbus does not require a way to multiplex multiple LSMs' labels in a
printable text string, so from my point of view, multiplexed labels do
not necessarily have to be in what Casey calls the "Hideous" format,
or in any text format at all: anything with documented rules for parsing
(including the unescaping that readers are expected to apply, if there
is any) would be fine. Based on the assumption of no "\0", I previously
suggested a "\0"-delimited encoding similar to /proc/self/cmdline, which
would not need any escaping/unescaping:
"apparmor\0" <apparmor label> "\0"
"selinux\0" <SELinux label> "\0"
...
"\0" (or this could be omitted since it's redundant with the length)
which would be fine (indeed, actually easier than the "Hideous" format)
from dbus' point of view.
dbus does not strictly need reading security labels for sockets or
processes to be atomic, either: it would be OK if we can get the complete
list of LSM labels *somehow*, possibly in O(number of LSMs) rather than
O(1) syscalls (although I'd prefer O(1)).
However, the getsockopt() interface only lets the kernel return one thing
per socket option, and I assume the networking maintainers probably don't
want to have to add SO_PEERAPPARMOR, SO_PEERSELINUX... for each LSM -
so this part at least would probably be easier as a single blob in some
text or binary format.
smcv
^ permalink raw reply
* Re: [PATCH 02/10] vfs: syscall: Add move_mount(2) to move mounts around
From: Tetsuo Handa @ 2019-07-23 13:45 UTC (permalink / raw)
To: Al Viro
Cc: John Johansen, Eric W. Biederman, David Howells, linux-api,
linux-fsdevel, torvalds, linux-security-module
In-Reply-To: <e75d4a66-cfcf-2ce8-e82a-fdc80f01723d@canonical.com>
Al, will you send this patch to 5.3-rcX via vfs.git tree?
On 2019/07/23 13:16, John Johansen wrote:
> On 7/22/19 3:12 AM, Tetsuo Handa wrote:
>> I did not see AppArmor patch for this problem in 5.3-rc1.
>> John, are you OK with this patch for 5.2-stable and 5.3 ?
>>
> yes, I have some larger mount rework and clean-up to do and an apparmor
> patch for this is waiting on that. Looking at the thread I am wondering
> where my previous reply is, probably lost in a mail client crash, I have
> had a few of those lately.
>
> Acked-by: John Johansen <john.johansen@canonical.com>
>
>
>> On 2019/07/09 19:51, Tetsuo Handa wrote:
>>> For now, can we apply this patch for 5.2-stable ?
>>>
>>>
>>> >From dd62fab0592e02580fd5a34222a2d11bfc179f61 Mon Sep 17 00:00:00 2001
>>> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>>> Date: Tue, 9 Jul 2019 19:05:49 +0900
>>> Subject: [PATCH] LSM: Disable move_mount() syscall when TOMOYO or AppArmor is enabled.
>>>
>>> Commit 2db154b3ea8e14b0 ("vfs: syscall: Add move_mount(2) to move mounts
>>> around") introduced security_move_mount() LSM hook, but we missed that
>>> TOMOYO and AppArmor did not implement hooks for checking move_mount(2).
>>> For pathname based access controls like TOMOYO and AppArmor, unchecked
>>> mount manipulation is not acceptable. Therefore, until TOMOYO and AppArmor
>>> implement hooks, in order to avoid unchecked mount manipulation, pretend
>>> as if move_mount(2) is unavailable when either TOMOYO or AppArmor is
>>> enabled.
>>>
>>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>>> Fixes: 2db154b3ea8e14b0 ("vfs: syscall: Add move_mount(2) to move mounts around")
>>> Cc: stable@vger.kernel.org # 5.2
>>> ---
>>> include/linux/lsm_hooks.h | 6 ++++++
>>> security/apparmor/lsm.c | 1 +
>>> security/tomoyo/tomoyo.c | 1 +
>>> 3 files changed, 8 insertions(+)
>>>
>>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>>> index 47f58cf..cd411b7 100644
>>> --- a/include/linux/lsm_hooks.h
>>> +++ b/include/linux/lsm_hooks.h
>>> @@ -2142,4 +2142,10 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
>>>
>>> extern int lsm_inode_alloc(struct inode *inode);
>>>
>>> +static inline int no_move_mount(const struct path *from_path,
>>> + const struct path *to_path)
>>> +{
>>> + return -ENOSYS;
>>> +}
>>> +
>>> #endif /* ! __LINUX_LSM_HOOKS_H */
>>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>>> index ec3a928..5cdf63b 100644
>>> --- a/security/apparmor/lsm.c
>>> +++ b/security/apparmor/lsm.c
>>> @@ -1158,6 +1158,7 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
>>> LSM_HOOK_INIT(capable, apparmor_capable),
>>>
>>> LSM_HOOK_INIT(sb_mount, apparmor_sb_mount),
>>> + LSM_HOOK_INIT(move_mount, no_move_mount),
>>> LSM_HOOK_INIT(sb_umount, apparmor_sb_umount),
>>> LSM_HOOK_INIT(sb_pivotroot, apparmor_sb_pivotroot),
>>>
>>> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
>>> index 716c92e..be1b1a1 100644
>>> --- a/security/tomoyo/tomoyo.c
>>> +++ b/security/tomoyo/tomoyo.c
>>> @@ -558,6 +558,7 @@ static void tomoyo_task_free(struct task_struct *task)
>>> LSM_HOOK_INIT(path_chown, tomoyo_path_chown),
>>> LSM_HOOK_INIT(path_chroot, tomoyo_path_chroot),
>>> LSM_HOOK_INIT(sb_mount, tomoyo_sb_mount),
>>> + LSM_HOOK_INIT(move_mount, no_move_mount),
>>> LSM_HOOK_INIT(sb_umount, tomoyo_sb_umount),
>>> LSM_HOOK_INIT(sb_pivotroot, tomoyo_sb_pivotroot),
>>> LSM_HOOK_INIT(socket_bind, tomoyo_socket_bind),
>>>
>>
>
>
^ permalink raw reply
* [PATCH] security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb()
From: Jia-Ju Bai @ 2019-07-23 10:00 UTC (permalink / raw)
To: casey, jmorris, serge; +Cc: linux-security-module, linux-kernel, Jia-Ju Bai
In smack_socket_sock_rcv_skb(), there is an if statement
on line 3920 to check whether skb is NULL:
if (skb && skb->secmark != 0)
This check indicates skb can be NULL in some cases.
But on lines 3931 and 3932, skb is used:
ad.a.u.net->netif = skb->skb_iif;
ipv6_skb_to_auditdata(skb, &ad.a, NULL);
Thus, possible null-pointer dereferences may occur when skb is NULL.
To fix these possible bugs, an if statement is added to check skb.
These bugs are found by a static analysis tool STCheck written by us.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
security/smack/smack_lsm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 4c5e5a438f8b..5c9fc8ba6e57 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3925,6 +3925,8 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
skp = smack_ipv6host_label(&sadd);
if (skp == NULL)
skp = smack_net_ambient;
+ if (skb == NULL)
+ break;
#ifdef CONFIG_AUDIT
smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
ad.a.u.net->family = family;
--
2.17.0
^ permalink raw reply related
* Re: [PATCH] selinux: check sidtab limit before adding a new entry
From: Ondrej Mosnacek @ 2019-07-23 6:48 UTC (permalink / raw)
To: Kees Cook
Cc: SElinux list, Paul Moore, NitinGote, kernel-hardening,
Linux Security Module list
In-Reply-To: <201907220949.AFB5B68@keescook>
On Mon, Jul 22, 2019 at 6:50 PM Kees Cook <keescook@chromium.org> wrote:
> On Mon, Jul 22, 2019 at 03:21:11PM +0200, Ondrej Mosnacek wrote:
> > We need to error out when trying to add an entry above SIDTAB_MAX in
> > sidtab_reverse_lookup() to avoid overflow on the odd chance that this
> > happens.
> >
> > Fixes: ee1a84fdfeed ("selinux: overhaul sidtab to fix bug and improve performance")
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>
> Is this reachable from unprivileged userspace?
>
> > ---
> > security/selinux/ss/sidtab.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
> > index e63a90ff2728..54c1ba1e79ab 100644
> > --- a/security/selinux/ss/sidtab.c
> > +++ b/security/selinux/ss/sidtab.c
> > @@ -286,6 +286,11 @@ static int sidtab_reverse_lookup(struct sidtab *s, struct context *context,
> > ++count;
> > }
> >
> > + /* bail out if we already reached max entries */
> > + rc = -ENOMEM;
> > + if (count == SIDTAB_MAX)
>
> Do you want to use >= here instead?
Makes sense. Also staged for v2.
>
> -Kees
>
> > + goto out_unlock;
> > +
> > /* insert context into new entry */
> > rc = -ENOMEM;
> > dst = sidtab_do_lookup(s, count, 1);
> > --
> > 2.21.0
> >
>
> --
> Kees Cook
Thanks,
--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.
^ permalink raw reply
* Re: [PATCH 02/10] vfs: syscall: Add move_mount(2) to move mounts around
From: John Johansen @ 2019-07-23 4:16 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Eric W. Biederman, Al Viro, David Howells, linux-api,
linux-fsdevel, torvalds, linux-security-module
In-Reply-To: <1698ec76-f56c-1e65-2f11-318c0ed225bb@i-love.sakura.ne.jp>
On 7/22/19 3:12 AM, Tetsuo Handa wrote:
> I did not see AppArmor patch for this problem in 5.3-rc1.
> John, are you OK with this patch for 5.2-stable and 5.3 ?
>
yes, I have some larger mount rework and clean-up to do and an apparmor
patch for this is waiting on that. Looking at the thread I am wondering
where my previous reply is, probably lost in a mail client crash, I have
had a few of those lately.
Acked-by: John Johansen <john.johansen@canonical.com>
> On 2019/07/09 19:51, Tetsuo Handa wrote:
>> For now, can we apply this patch for 5.2-stable ?
>>
>>
>> >From dd62fab0592e02580fd5a34222a2d11bfc179f61 Mon Sep 17 00:00:00 2001
>> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> Date: Tue, 9 Jul 2019 19:05:49 +0900
>> Subject: [PATCH] LSM: Disable move_mount() syscall when TOMOYO or AppArmor is enabled.
>>
>> Commit 2db154b3ea8e14b0 ("vfs: syscall: Add move_mount(2) to move mounts
>> around") introduced security_move_mount() LSM hook, but we missed that
>> TOMOYO and AppArmor did not implement hooks for checking move_mount(2).
>> For pathname based access controls like TOMOYO and AppArmor, unchecked
>> mount manipulation is not acceptable. Therefore, until TOMOYO and AppArmor
>> implement hooks, in order to avoid unchecked mount manipulation, pretend
>> as if move_mount(2) is unavailable when either TOMOYO or AppArmor is
>> enabled.
>>
>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> Fixes: 2db154b3ea8e14b0 ("vfs: syscall: Add move_mount(2) to move mounts around")
>> Cc: stable@vger.kernel.org # 5.2
>> ---
>> include/linux/lsm_hooks.h | 6 ++++++
>> security/apparmor/lsm.c | 1 +
>> security/tomoyo/tomoyo.c | 1 +
>> 3 files changed, 8 insertions(+)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 47f58cf..cd411b7 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -2142,4 +2142,10 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
>>
>> extern int lsm_inode_alloc(struct inode *inode);
>>
>> +static inline int no_move_mount(const struct path *from_path,
>> + const struct path *to_path)
>> +{
>> + return -ENOSYS;
>> +}
>> +
>> #endif /* ! __LINUX_LSM_HOOKS_H */
>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>> index ec3a928..5cdf63b 100644
>> --- a/security/apparmor/lsm.c
>> +++ b/security/apparmor/lsm.c
>> @@ -1158,6 +1158,7 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
>> LSM_HOOK_INIT(capable, apparmor_capable),
>>
>> LSM_HOOK_INIT(sb_mount, apparmor_sb_mount),
>> + LSM_HOOK_INIT(move_mount, no_move_mount),
>> LSM_HOOK_INIT(sb_umount, apparmor_sb_umount),
>> LSM_HOOK_INIT(sb_pivotroot, apparmor_sb_pivotroot),
>>
>> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
>> index 716c92e..be1b1a1 100644
>> --- a/security/tomoyo/tomoyo.c
>> +++ b/security/tomoyo/tomoyo.c
>> @@ -558,6 +558,7 @@ static void tomoyo_task_free(struct task_struct *task)
>> LSM_HOOK_INIT(path_chown, tomoyo_path_chown),
>> LSM_HOOK_INIT(path_chroot, tomoyo_path_chroot),
>> LSM_HOOK_INIT(sb_mount, tomoyo_sb_mount),
>> + LSM_HOOK_INIT(move_mount, no_move_mount),
>> LSM_HOOK_INIT(sb_umount, tomoyo_sb_umount),
>> LSM_HOOK_INIT(sb_pivotroot, tomoyo_sb_pivotroot),
>> LSM_HOOK_INIT(socket_bind, tomoyo_socket_bind),
>>
>
^ permalink raw reply
* Re: [PATCH] selinux: check sidtab limit before adding a new entry
From: Paul Moore @ 2019-07-23 0:36 UTC (permalink / raw)
To: Kees Cook
Cc: Ondrej Mosnacek, selinux, NitinGote, kernel-hardening,
linux-security-module
In-Reply-To: <201907220949.AFB5B68@keescook>
On Mon, Jul 22, 2019 at 12:50 PM Kees Cook <keescook@chromium.org> wrote:
> On Mon, Jul 22, 2019 at 03:21:11PM +0200, Ondrej Mosnacek wrote:
> > We need to error out when trying to add an entry above SIDTAB_MAX in
> > sidtab_reverse_lookup() to avoid overflow on the odd chance that this
> > happens.
> >
> > Fixes: ee1a84fdfeed ("selinux: overhaul sidtab to fix bug and improve performance")
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>
> Is this reachable from unprivileged userspace?
I believe it's reachable via selinuxfs under /sys/fs/selinux/context,
and the DAC permissions are for the relevant files are 0666, but the
SELinux policy might restrict that.
> > ---
> > security/selinux/ss/sidtab.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
> > index e63a90ff2728..54c1ba1e79ab 100644
> > --- a/security/selinux/ss/sidtab.c
> > +++ b/security/selinux/ss/sidtab.c
> > @@ -286,6 +286,11 @@ static int sidtab_reverse_lookup(struct sidtab *s, struct context *context,
> > ++count;
> > }
> >
> > + /* bail out if we already reached max entries */
> > + rc = -ENOMEM;
> > + if (count == SIDTAB_MAX)
>
> Do you want to use >= here instead?
Yes, definitely.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: Preferred subj= with multiple LSMs
From: Casey Schaufler @ 2019-07-23 0:11 UTC (permalink / raw)
To: Paul Moore
Cc: James Morris, Steve Grubb, Richard Guy Briggs,
linux-audit@redhat.com, Linux Security Module list,
Simon McVittie, casey
In-Reply-To: <CAHC9VhSbg7BxPSA4NkQV3_1zx6cj3ej25e6fJ2FBWX9fU104rg@mail.gmail.com>
On 7/22/2019 3:30 PM, Paul Moore wrote:
> On Mon, Jul 22, 2019 at 6:01 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 7/22/2019 1:50 PM, James Morris wrote:
>>> On Fri, 19 Jul 2019, Paul Moore wrote:
>>>
>>>>> We've never had to think about having general rules on
>>>>> what security modules do before, because with only one
>>>>> active each could do whatever it wanted without fear of
>>>>> conflict. If there is already a character that none of
>>>>> the existing modules use, how would it be wrong to
>>>>> reserve it?
>>>> "We've never had to think about having general rules on what security
>>>> modules do before..."
>>>>
>>>> We famously haven't imposed restrictions on the label format before
>>>> now, and this seems like a pretty poor reason to start.
>>> Agreed.
>> In a follow on thread
>>
>> https://www.spinics.net/lists/linux-security-module/msg29996.html
>>
>> we've been discussing the needs of dbus-daemon in a multiple LSM
>> environment. I suggest that if supporting dbus well is assisted by
>> making reasonable restrictions on what constitutes a valid LSM
>> "context" that we have a good reason. While there are ways to
>> present groups of arbitrary hunks of data, why would we want to?
> I continue to believe that restrictions on the label format are a bad
> idea, and I further believe that multiplexing the labels is going to
> be a major problem that will haunt us for many, many years. If we are
> going to support multiple simultaneous LSMs I think we need to find a
> way to represent those labels independently.
Let's review the bidding:
Audit wants to maintain backward compatibility while also getting
the information about multiple subject and object labels. The current
proposal:
... subj=a:b:c:d \
... obj=e:f:g:h obj_selinux=e:f:g:h obj_mumble=Crivens \
... subj_selinux=a:b:c:d subj_mumble=Feegle \
...
where obj_<lsm> and subj_<lsm> are only provided if there's more than
one active "display" LSM.
Dbus wants an atomic fetch of the security attributes from a socket
and from a /proc entry. We don't want to break compatibility, so new
interfaces are provided:
SO_PEERCONTEXT - packet label in the "Hideous" format
/proc/.../attr/context - process label in the "Hideous" format
Legacy programs want the security attributes from a socket
and from a /proc entry. Since they don't know how to differentiate
which security module is active, these are controlled by the
"display", which defaults to the first module loaded that provides
a secid_to_secctx() hook. (not quite the definition, but close enough)
SO_PEERSEC - "display" LSM packet label in module native format
/proc/.../attr/display - set/get the "display" value
/proc/.../attr/current - "display" LSM process label in module native format
/proc/.../attr/smack/current - Smack process label in module native format
A classic Android, Tizen, Fedora or Ubuntu system will continue to use these
interfaces and see no difference in behavior.
A system that really wants to use multiple "display"ing modules will
have the same issues that dbus has, and will likely convert to the new,
"hideous" interfaces, especially if a liblsm (NOT libsecurity!) is
provided.
^ permalink raw reply
* Re: Preferred subj= with multiple LSMs
From: Paul Moore @ 2019-07-22 22:30 UTC (permalink / raw)
To: Casey Schaufler
Cc: James Morris, Steve Grubb, Richard Guy Briggs,
linux-audit@redhat.com, Linux Security Module list
In-Reply-To: <ca22ea45-3f3b-4f79-8d77-7528877b8b36@schaufler-ca.com>
On Mon, Jul 22, 2019 at 6:01 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 7/22/2019 1:50 PM, James Morris wrote:
> > On Fri, 19 Jul 2019, Paul Moore wrote:
> >
> >>> We've never had to think about having general rules on
> >>> what security modules do before, because with only one
> >>> active each could do whatever it wanted without fear of
> >>> conflict. If there is already a character that none of
> >>> the existing modules use, how would it be wrong to
> >>> reserve it?
> >> "We've never had to think about having general rules on what security
> >> modules do before..."
> >>
> >> We famously haven't imposed restrictions on the label format before
> >> now, and this seems like a pretty poor reason to start.
> > Agreed.
>
> In a follow on thread
>
> https://www.spinics.net/lists/linux-security-module/msg29996.html
>
> we've been discussing the needs of dbus-daemon in a multiple LSM
> environment. I suggest that if supporting dbus well is assisted by
> making reasonable restrictions on what constitutes a valid LSM
> "context" that we have a good reason. While there are ways to
> present groups of arbitrary hunks of data, why would we want to?
I continue to believe that restrictions on the label format are a bad
idea, and I further believe that multiplexing the labels is going to
be a major problem that will haunt us for many, many years. If we are
going to support multiple simultaneous LSMs I think we need to find a
way to represent those labels independently.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: Preferred subj= with multiple LSMs
From: Casey Schaufler @ 2019-07-22 22:01 UTC (permalink / raw)
To: James Morris, Paul Moore
Cc: Steve Grubb, Richard Guy Briggs, linux-audit@redhat.com,
Linux Security Module list, casey
In-Reply-To: <alpine.LRH.2.21.1907230649460.18217@namei.org>
On 7/22/2019 1:50 PM, James Morris wrote:
> On Fri, 19 Jul 2019, Paul Moore wrote:
>
>>> We've never had to think about having general rules on
>>> what security modules do before, because with only one
>>> active each could do whatever it wanted without fear of
>>> conflict. If there is already a character that none of
>>> the existing modules use, how would it be wrong to
>>> reserve it?
>> "We've never had to think about having general rules on what security
>> modules do before..."
>>
>> We famously haven't imposed restrictions on the label format before
>> now, and this seems like a pretty poor reason to start.
> Agreed.
In a follow on thread
https://www.spinics.net/lists/linux-security-module/msg29996.html
we've been discussing the needs of dbus-daemon in a multiple LSM
environment. I suggest that if supporting dbus well is assisted by
making reasonable restrictions on what constitutes a valid LSM
"context" that we have a good reason. While there are ways to
present groups of arbitrary hunks of data, why would we want to?
^ permalink raw reply
* Re: Preferred subj= with multiple LSMs
From: James Morris @ 2019-07-22 20:50 UTC (permalink / raw)
To: Paul Moore
Cc: Casey Schaufler, Steve Grubb, Richard Guy Briggs,
linux-audit@redhat.com, Linux Security Module list
In-Reply-To: <CAHC9VhTf0yYDZBxOtfv2E5=GtrBYonoqr46hyBy7qdNdjVLoVg@mail.gmail.com>
On Fri, 19 Jul 2019, Paul Moore wrote:
> > We've never had to think about having general rules on
> > what security modules do before, because with only one
> > active each could do whatever it wanted without fear of
> > conflict. If there is already a character that none of
> > the existing modules use, how would it be wrong to
> > reserve it?
>
> "We've never had to think about having general rules on what security
> modules do before..."
>
> We famously haven't imposed restrictions on the label format before
> now, and this seems like a pretty poor reason to start.
Agreed.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH] [RESEND v2] structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK
From: Kees Cook @ 2019-07-22 17:23 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Morton, James Morris, Serge E. Hallyn, Ard Biesheuvel,
Masahiro Yamada, Alexander Potapenko, linux-security-module,
linux-kernel
In-Reply-To: <20190722114134.3123901-1-arnd@arndb.de>
On Mon, Jul 22, 2019 at 01:41:20PM +0200, Arnd Bergmann wrote:
> The combination of KASAN_STACK and GCC_PLUGIN_STRUCTLEAK_BYREF
> leads to much larger kernel stack usage, as seen from the warnings
> about functions that now exceed the 2048 byte limit:
>
> drivers/media/i2c/tvp5150.c:253:1: error: the frame size of 3936 bytes is larger than 2048 bytes
> drivers/media/tuners/r820t.c:1327:1: error: the frame size of 2816 bytes is larger than 2048 bytes
> drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:16552:1: error: the frame size of 3144 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> fs/ocfs2/aops.c:1892:1: error: the frame size of 2088 bytes is larger than 2048 bytes
> fs/ocfs2/dlm/dlmrecovery.c:737:1: error: the frame size of 2088 bytes is larger than 2048 bytes
> fs/ocfs2/namei.c:1677:1: error: the frame size of 2584 bytes is larger than 2048 bytes
> fs/ocfs2/super.c:1186:1: error: the frame size of 2640 bytes is larger than 2048 bytes
> fs/ocfs2/xattr.c:3678:1: error: the frame size of 2176 bytes is larger than 2048 bytes
> net/bluetooth/l2cap_core.c:7056:1: error: the frame size of 2144 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> net/bluetooth/l2cap_core.c: In function 'l2cap_recv_frame':
> net/bridge/br_netlink.c:1505:1: error: the frame size of 2448 bytes is larger than 2048 bytes
> net/ieee802154/nl802154.c:548:1: error: the frame size of 2232 bytes is larger than 2048 bytes
> net/wireless/nl80211.c:1726:1: error: the frame size of 2224 bytes is larger than 2048 bytes
> net/wireless/nl80211.c:2357:1: error: the frame size of 4584 bytes is larger than 2048 bytes
> net/wireless/nl80211.c:5108:1: error: the frame size of 2760 bytes is larger than 2048 bytes
> net/wireless/nl80211.c:6472:1: error: the frame size of 2112 bytes is larger than 2048 bytes
>
> The structleak plugin was previously disabled for CONFIG_COMPILE_TEST,
> but meant we missed some bugs, so this time we should address them.
>
> The frame size warnings are distracting, and risking a kernel stack
> overflow is generally not beneficial to performance, so it may be best
> to disallow that particular combination. This can be done by turning
> off either one. I picked the dependency in GCC_PLUGIN_STRUCTLEAK_BYREF
> and GCC_PLUGIN_STRUCTLEAK_BYREF_ALL, as this option is designed to
> make uninitialized stack usage less harmful when enabled on its own,
> but it also prevents KASAN from detecting those cases in which it was
> in fact needed.
>
> KASAN_STACK is currently implied by KASAN on gcc, but could be made a
> user selectable option if we want to allow combining (non-stack) KASAN
> with GCC_PLUGIN_STRUCTLEAK_BYREF.
>
> Note that it would be possible to specifically address the files that
> print the warning, but presumably the overall stack usage is still
> significantly higher than in other configurations, so this would not
> address the full problem.
>
> I could not test this with CONFIG_INIT_STACK_ALL, which may or may not
> suffer from a similar problem.
>
> Fixes: 81a56f6dcd20 ("gcc-plugins: structleak: Generalize to all variable types")
> Acked-by: Kees Cook <keescook@chromium.org>
> Link: https://lore.kernel.org/lkml/20190628123819.2785504-1-arnd@arndb.de/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> [v2] do it for both GCC_PLUGIN_STRUCTLEAK_BYREF and GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
>
> Andrew, can you pick this up in -mm? It looks like nobody else
> wanted it in their trees even though there was agreement on the
> patch itself.
Andrew, if you don't want it, I can take it via my gcc-plugins tree?
-Kees
> ---
> security/Kconfig.hardening | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
> index a1ffe2eb4d5f..af4c979b38ee 100644
> --- a/security/Kconfig.hardening
> +++ b/security/Kconfig.hardening
> @@ -61,6 +61,7 @@ choice
> config GCC_PLUGIN_STRUCTLEAK_BYREF
> bool "zero-init structs passed by reference (strong)"
> depends on GCC_PLUGINS
> + depends on !(KASAN && KASAN_STACK=1)
> select GCC_PLUGIN_STRUCTLEAK
> help
> Zero-initialize any structures on the stack that may
> @@ -70,9 +71,15 @@ choice
> exposures, like CVE-2017-1000410:
> https://git.kernel.org/linus/06e7e776ca4d3654
>
> + As a side-effect, this keeps a lot of variables on the
> + stack that can otherwise be optimized out, so combining
> + this with CONFIG_KASAN_STACK can lead to a stack overflow
> + and is disallowed.
> +
> config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
> bool "zero-init anything passed by reference (very strong)"
> depends on GCC_PLUGINS
> + depends on !(KASAN && KASAN_STACK=1)
> select GCC_PLUGIN_STRUCTLEAK
> help
> Zero-initialize any stack variables that may be passed
> --
> 2.20.0
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH] [RESEND v2] structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK
From: Arnd Bergmann @ 2019-07-22 17:14 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Dmitriy Vyukov, Marco Elver, Andrew Morton, James Morris,
Serge E. Hallyn, Kees Cook, Ard Biesheuvel, Masahiro Yamada,
linux-security-module, LKML
In-Reply-To: <CAG_fn=U60gv-zWiVZS5Z++1fMcLwDmOgoF8gpJY3c+4dGHBhvw@mail.gmail.com>
On Mon, Jul 22, 2019 at 5:23 PM Alexander Potapenko <glider@google.com> wrote:
> On Mon, Jul 22, 2019 at 4:26 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Mon, Jul 22, 2019 at 3:43 PM Alexander Potapenko <glider@google.com> wrote:
> > > On Mon, Jul 22, 2019 at 1:41 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > Doesn't that just limit the usefulness of KASAN, as you no longer catch
> > actual accesses to unintialized variables that KASAN is designed to find?
> KASAN (unlike KMSAN) doesn't detect accesses to uninitialized variables.
> It can of course detect bugs induced by e.g. treating an uninitialized
> variable as a pointer or an array index.
> Depending on the pattern used to initialize those variables, this can
> indeed mask some bugs, but OTOH others will become more reproducible.
>
> I'm not really sure KASAN+CONFIG_INIT_STACK_ALL is a problem right
> now, will need to take a look.
See below for the slightly trimmed warning output of a 32-bit ARM
allmodconfig build with KASAN_STACK enabled. In the default
allmodconfig build (no KASAN_STACK, with CONFIG_INIT_STACK_ALL)
I see no warnings.
With KASAN_STACK, some really bad ones start to appear, such as
drivers/gpu/drm/panel/panel-sitronix-st7789v.c:197:12: 14944 bytes
'st7789v_prepare'
drivers/usb/misc/sisusbvga/sisusb.c:1878:12: 12480 bytes 'sisusb_init_gfxcore'.
drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c:108:12:
7904 bytes 'lb035q02_connect'
drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c:163:13: 9600
bytes 'td028ttec1_panel_enable'
drivers/media/dvb-frontends/stv0367.c:995:12: 7488 bytes 'stv0367ter_algo'
drivers/media/i2c/mt9t112.c:670:12: 7584 bytes 'mt9t112_init_camera'
drivers/usb/misc/sisusbvga/sisusb.c:1750:13: 8096 bytes
'sisusb_set_default_mode'
In most functions, the frame sizes stay the same or go up 32 bytes.
In a few files, the warnings change drastically, and but it goes both ways,
smaller or larger with init_stack_all added in, presumably as clang makes
different inlining decisions, the worst ones are now
drivers/gpu/drm/panel/panel-sitronix-st7789v.c:197:12: 14944 bytes
'st7789v_prepare'
drivers/media/i2c/mt9t112.c:670:12: 10080 bytes 'mt9t112_init_camera'
drivers/usb/misc/sisusbvga/sisusb.c:1878:12: 11072 bytes 'sisusb_init_gfxcore'
drivers/firmware/broadcom/bcm47xx_sprom.c:187:13: 9152 bytes
'bcm47xx_sprom_fill_auto'
drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c:163:13: 9600
bytes 'td028ttec1_panel_enable'
drivers/usb/misc/sisusbvga/sisusb.c:1750:13: 8096 bytes
'sisusb_set_default_mode'
See below for the full list.
Overall, I'd say there is probably no harm in allowing CONFIG_INIT_STACK_ALL
in combination with KASAN_STACK when building with clang, as KASAN_STACK
by itself is the actual problem. With gcc, CONFIG_INIT_STACK_ALL+KASAN_STACK
is dangerous, while KASAN_STACK by itself is not a problem any more on
recent gcc versions.
Arnd
--- allmodconfig-arm+kasan-stack 2019-07-22 18:57:29.970466686 +0200
+++ allmodconfig-arm+kasan_stack+init_stack_all 2019-07-22
18:57:31.434493204 +0200
@@ -1,262 +1,260 @@
crypto/asymmetric_keys/asym_tpm.c:720:12: 1376 bytes 'tpm_key_eds_op'
crypto/crypto_user_stat.c:298:5: 1920 bytes 'crypto_reportstat'
-drivers/block/drbd/drbd_receiver.c:921:12: 1632 bytes 'conn_connect'
+drivers/block/drbd/drbd_receiver.c:921:12: 1664 bytes 'conn_connect'
drivers/block/loop.c:1569:12: 1824 bytes 'lo_ioctl'
drivers/cdrom/cdrom.c:1149:5: 1600 bytes 'cdrom_open'
drivers/crypto/ccree/cc_cipher.c:383:12: 1536 bytes 'cc_cipher_setkey'
-drivers/firmware/broadcom/bcm47xx_sprom.c:187:13: 3424 bytes
'bcm47xx_sprom_fill_auto'
+drivers/firmware/broadcom/bcm47xx_sprom.c:187:13: 9152 bytes
'bcm47xx_sprom_fill_auto'
+drivers/firmware/broadcom/bcm47xx_sprom.c:408:13: 1536 bytes
'bcm47xx_fill_sprom_path_r4589'
drivers/fpga/machxo2-spi.c:186:12: 1952 bytes 'machxo2_write_init'
drivers/fpga/machxo2-spi.c:286:12: 1856 bytes 'machxo2_write_complete'
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:291:6: 1376 bytes
'amdgpu_atombios_get_connector_info_from_object_table'
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:2993:6:
1312 bytes 'bw_calcs'
+drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:1361:17: 1312
bytes 'link_create'
drivers/gpu/drm/amd/amdgpu/../display/dc/dce112/dce112_resource.c:1328:23:
1568 bytes 'dce112_create_resource_pool'
drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:1553:6:
2112 bytes 'mod_color_calculate_regamma_params'
drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/vega10_hwmgr.c:2932:12:
1504 bytes 'vega10_enable_dpm_tasks'
-drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/vegam_smumgr.c:1923:12:
1504 bytes 'vegam_init_smc_table'
+drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/vegam_smumgr.c:1923:12:
1536 bytes 'vegam_init_smc_table'
drivers/gpu/drm/bridge/parade-ps8622.c:352:13: 1632 bytes 'ps8622_pre_enable'
-drivers/gpu/drm/i2c/tda998x_drv.c:1419:13: 1376 bytes 'tda998x_bridge_mode_set'
-drivers/gpu/drm/i2c/tda998x_drv.c:2058:1: 1312 bytes 'tda998x_probe'
drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf100.c:567:1: 3456 bytes
'gf100_ram_new_'
-drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c:1521:1: 5472 bytes
'gk104_ram_new_'
+drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c:1521:1: 5504 bytes
'gk104_ram_new_'
drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c:940:1: 2592 bytes
'gt215_ram_new'
-drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c:108:12:
7904 bytes 'lb035q02_connect'
drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c:163:13: 9600
bytes 'td028ttec1_panel_enable'
drivers/gpu/drm/panel/panel-sitronix-st7789v.c:197:12: 14944 bytes
'st7789v_prepare'
drivers/gpu/drm/radeon/ci_dpm.c:5165:5: 1408 bytes 'ci_dpm_enable'
drivers/gpu/drm/radeon/radeon_atombios.c:521:6: 1440 bytes
'radeon_get_atom_connector_info_from_object_table'
drivers/gpu/ipu-v3/ipu-di.c:557:5: 1472 bytes 'ipu_di_init_sync_panel'
-drivers/hid/hid-logitech-hidpp.c:3327:13: 2336 bytes 'hidpp_connect_event'
+drivers/hid/hid-logitech-hidpp.c:3327:13: 1664 bytes 'hidpp_connect_event'
drivers/iio/accel/sca3000.c:1106:12: 1312 bytes 'sca3000_read_event_config'
drivers/iio/accel/sca3000.c:1244:12: 3712 bytes 'sca3000_write_event_config'
drivers/iio/accel/sca3000.c:1453:12: 4480 bytes 'sca3000_probe'
drivers/iio/accel/sca3000.c:709:12: 1600 bytes 'sca3000_read_raw'
drivers/iio/accel/sca3000.c:776:12: 2336 bytes 'sca3000_write_raw'
drivers/iio/adc/ti-ads124s08.c:268:20: 1376 bytes 'ads124s_trigger_handler'
drivers/iio/dac/ad5758.c:840:12: 4864 bytes 'ad5758_probe'
drivers/iio/frequency/ad9523.c:509:16: 1440 bytes 'ad9523_store'
drivers/iio/frequency/ad9523.c:972:12: 2720 bytes 'ad9523_probe'
drivers/iio/proximity/as3935.c:320:12: 1408 bytes 'as3935_resume'
drivers/infiniband/hw/cxgb3/iwch_cm.c:1428:12: 1312 bytes 'peer_close'
drivers/infiniband/hw/i40iw/i40iw_cm.c:4176:13: 2848 bytes
'i40iw_cm_event_handler'
drivers/infiniband/hw/mlx4/mcg.c:642:13: 2016 bytes 'mlx4_ib_mcg_work_handler'
-drivers/infiniband/hw/mthca/mthca_main.c:897:12: 1440 bytes '__mthca_init_one'
+drivers/infiniband/hw/mthca/mthca_main.c:897:12: 1472 bytes '__mthca_init_one'
drivers/infiniband/sw/rxe/rxe_req.c:583:5: 1536 bytes 'rxe_requester'
drivers/input/touchscreen/hideep.c:616:12: 1632 bytes 'hideep_update_firmware'
-drivers/md/dm-integrity.c:2141:13: 1344 bytes 'do_journal_write'
+drivers/md/dm-integrity.c:2141:13: 1312 bytes 'do_journal_write'
drivers/media/common/cx2341x.c:1574:5: 1888 bytes 'cx2341x_handler_init'
drivers/media/dvb-frontends/cx24110.c:522:12: 1344 bytes 'cx24110_set_frontend'
-drivers/media/dvb-frontends/cx24120.c:1266:12: 1920 bytes 'cx24120_init'
-drivers/media/dvb-frontends/cx24123.c:900:12: 1600 bytes 'cx24123_set_frontend'
+drivers/media/dvb-frontends/cx24120.c:1266:12: 1984 bytes 'cx24120_init'
+drivers/media/dvb-frontends/cx24123.c:900:12: 1632 bytes 'cx24123_set_frontend'
drivers/media/dvb-frontends/dib3000mc.c:323:12: 1920 bytes 'dib3000mc_init'
-drivers/media/dvb-frontends/drxd_hard.c:1897:12: 4544 bytes 'DRX_Start'
+drivers/media/dvb-frontends/drxd_hard.c:1897:12: 4608 bytes 'DRX_Start'
drivers/media/dvb-frontends/drxd_hard.c:2801:12: 2272 bytes 'drxd_init'
drivers/media/dvb-frontends/ds3000.c:883:12: 1440 bytes 'ds3000_set_frontend'
-drivers/media/dvb-frontends/itd1000.c:251:12: 2112 bytes
'itd1000_set_parameters'
-drivers/media/dvb-frontends/lgs8gl5.c:302:1: 1536 bytes 'lgs8gl5_set_frontend'
+drivers/media/dvb-frontends/itd1000.c:251:12: 2144 bytes
'itd1000_set_parameters'
+drivers/media/dvb-frontends/lgs8gl5.c:302:1: 1568 bytes 'lgs8gl5_set_frontend'
drivers/media/dvb-frontends/m88rs2000.c:596:12: 1312 bytes
'm88rs2000_set_frontend'
drivers/media/dvb-frontends/mb86a16.c:997:12: 2912 bytes 'mb86a16_set_fe'
drivers/media/dvb-frontends/mxl5xx.c:1837:22: 2368 bytes 'mxl5xx_attach'
+drivers/media/dvb-frontends/nxt200x.c:525:12: 1440 bytes
'nxt200x_setup_frontend_parameters'
drivers/media/dvb-frontends/nxt200x.c:930:12: 1408 bytes 'nxt2004_init'
drivers/media/dvb-frontends/s5h1420.c:628:12: 1376 bytes 's5h1420_set_frontend'
-drivers/media/dvb-frontends/s5h1432.c:168:12: 1376 bytes 's5h1432_set_frontend'
-drivers/media/dvb-frontends/sp887x.c:520:12: 1472 bytes 'sp887x_init'
-drivers/media/dvb-frontends/stv0297.c:398:12: 2240 bytes 'stv0297_set_frontend'
-drivers/media/dvb-frontends/stv0367.c:2274:12: 1440 bytes 'stv0367cab_init'
-drivers/media/dvb-frontends/stv0367.c:2540:12: 3360 bytes
'stv0367cab_set_frontend'
-drivers/media/dvb-frontends/stv0367.c:995:12: 7488 bytes 'stv0367ter_algo'
-drivers/media/dvb-frontends/stv0900_core.c:1331:30: 3072 bytes
'stv0900_init_internal'
-drivers/media/dvb-frontends/stv0900_core.c:956:6: 1344 bytes
'stv0900_start_search'
+drivers/media/dvb-frontends/sp8870.c:451:12: 1472 bytes 'sp8870_set_frontend'
+drivers/media/dvb-frontends/sp887x.c:520:12: 1952 bytes 'sp887x_init'
+drivers/media/dvb-frontends/stv0297.c:398:12: 2688 bytes 'stv0297_set_frontend'
+drivers/media/dvb-frontends/stv0900_core.c:1331:30: 2944 bytes
'stv0900_init_internal'
+drivers/media/dvb-frontends/stv0900_core.c:956:6: 1376 bytes
'stv0900_start_search'
drivers/media/dvb-frontends/stv0910.c:1019:12: 3680 bytes 'start'
drivers/media/dvb-frontends/stv0910.c:1379:12: 1664 bytes 'read_status'
drivers/media/dvb-frontends/stv0910.c:1776:22: 1920 bytes 'stv0910_attach'
drivers/media/dvb-frontends/tda10086.c:93:12: 1536 bytes 'tda10086_init'
-drivers/media/i2c/mt9t112.c:670:12: 7584 bytes 'mt9t112_init_camera'
-drivers/media/i2c/mt9v111.c:410:12: 1312 bytes '__mt9v111_sw_reset'
-drivers/media/i2c/mt9v111.c:534:12: 2400 bytes 'mt9v111_hw_config'
-drivers/media/i2c/mt9v111.c:996:12: 1888 bytes 'mt9v111_s_ctrl'
-drivers/media/i2c/ov5640.c:1791:12: 3200 bytes 'ov5640_set_mode'
-drivers/media/i2c/ov5640.c:2588:12: 1536 bytes 'ov5640_s_ctrl'
+drivers/media/i2c/mt9t112.c:670:12: 10080 bytes 'mt9t112_init_camera'
+drivers/media/i2c/mt9v111.c:534:12: 2176 bytes 'mt9v111_hw_config'
+drivers/media/i2c/mt9v111.c:996:12: 1632 bytes 'mt9v111_s_ctrl'
+drivers/media/i2c/ov5640.c:1791:12: 3552 bytes 'ov5640_set_mode'
+drivers/media/i2c/ov5640.c:2588:12: 1504 bytes 'ov5640_s_ctrl'
drivers/media/i2c/ov7670.c:1577:12: 1408 bytes 'ov7670_s_ctrl'
-drivers/media/i2c/s5k4ecgx.c:742:12: 1632 bytes '__s5k4ecgx_s_params'
-drivers/media/pci/cx23885/cx23885-dvb.c:1187:12: 2016 bytes 'dvb_register'
-drivers/media/pci/ddbridge/ddbridge-core.c:2365:6: 1664 bytes 'ddb_ports_init'
+drivers/media/i2c/s5k4ecgx.c:742:12: 1728 bytes '__s5k4ecgx_s_params'
+drivers/media/i2c/tvp5150.c:121:12: 1312 bytes 'tvp5150_log_status'
+drivers/media/pci/cx23885/cx23885-dvb.c:1187:12: 2080 bytes 'dvb_register'
+drivers/media/pci/ddbridge/ddbridge-core.c:2365:6: 1536 bytes 'ddb_ports_init'
drivers/media/pci/ngene/ngene-core.c:1664:5: 1408 bytes 'ngene_probe'
-drivers/media/pci/ttpci/av7110_v4l.c:693:5: 1376 bytes
'av7110_init_analog_module'
-drivers/media/tuners/fc0011.c:163:12: 1824 bytes 'fc0011_set_params'
+drivers/media/pci/ttpci/av7110_v4l.c:693:5: 1344 bytes
'av7110_init_analog_module'
+drivers/media/tuners/fc0011.c:163:12: 2240 bytes 'fc0011_set_params'
drivers/media/tuners/fc0013.c:209:12: 1440 bytes 'fc0013_set_params'
drivers/media/tuners/msi001.c:83:12: 1504 bytes 'msi001_set_tuner'
drivers/media/usb/cpia2/cpia2_core.c:615:5: 3552 bytes 'cpia2_reset_camera'
-drivers/media/usb/dvb-usb-v2/af9035.c:1200:12: 1632 bytes
'it930x_frontend_attach'
-drivers/media/usb/dvb-usb-v2/af9035.c:1340:12: 2592 bytes 'af9035_tuner_attach'
-drivers/media/usb/dvb-usb-v2/af9035.c:719:12: 1344 bytes
'af9035_download_firmware'
drivers/media/usb/dvb-usb-v2/rtl28xxu.c:350:12: 1920 bytes
'rtl2832u_read_config'
drivers/media/usb/em28xx/em28xx-dvb.c:1407:12: 2176 bytes 'em28xx_dvb_init'
drivers/media/usb/go7007/go7007-fw.c:1494:31: 1376 bytes 'do_special'
drivers/media/usb/gspca/sn9c2028.c:802:12: 3168 bytes 'sd_start'
drivers/media/usb/pvrusb2/pvrusb2-hdw.c:4585:12: 1376 bytes
'state_eval_pipeline_config'
-drivers/message/fusion/mptctl.c:621:1: 1472 bytes '__mptctl_ioctl'
+drivers/message/fusion/mptctl.c:621:1: 1504 bytes '__mptctl_ioctl'
drivers/mfd/ezx-pcap.c:173:13: 1600 bytes 'pcap_isr_work'
drivers/mtd/chips/cfi_cmdset_0001.c:1868:12: 2336 bytes 'cfi_intelext_writev'
-drivers/mtd/chips/cfi_cmdset_0020.c:421:12: 1600 bytes 'do_write_buffer'
-drivers/mtd/chips/cfi_cmdset_0020.c:734:19: 1664 bytes 'do_erase_oneblock'
+drivers/mtd/chips/cfi_cmdset_0020.c:248:19: 1312 bytes 'do_read_onechip'
+drivers/mtd/chips/cfi_cmdset_0020.c:421:12: 1664 bytes 'do_write_buffer'
+drivers/mtd/chips/cfi_cmdset_0020.c:734:19: 1696 bytes 'do_erase_oneblock'
drivers/mtd/mtdchar.c:634:12: 1472 bytes 'mtdchar_ioctl'
-drivers/mtd/nftlcore.c:231:12: 1472 bytes 'NFTL_foldchain'
+drivers/mtd/nftlcore.c:231:12: 1504 bytes 'NFTL_foldchain'
drivers/mtd/spi-nor/spi-nor.c:4104:5: 1312 bytes 'spi_nor_scan'
drivers/net/can/janz-ican3.c:1882:12: 2208 bytes 'ican3_probe'
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:5662:13: 1440 bytes
'bnx2x_sp_task'
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c:1587:5: 1376 bytes
'bnxt_tc_setup_flower'
drivers/net/ethernet/broadcom/cnic.c:2792:12: 1344 bytes
'cnic_submit_bnx2x_kwqes'
drivers/net/ethernet/broadcom/cnic.c:4705:12: 2592 bytes 'cnic_start_bnx2_hw'
drivers/net/ethernet/chelsio/cxgb4/cudbg_lib.c:1079:12: 1824 bytes
'cudbg_collect_mem_region'
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c:5654:12: 1472 bytes 'init_one'
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:1068:5:
1696 bytes 'hclge_dbg_run_cmd'
-drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c:555:6: 1568
bytes 'hclge_mbx_handler'
+drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c:555:6: 1632
bytes 'hclge_mbx_handler'
drivers/net/ethernet/intel/i40e/i40e_ddp.c:264:5: 1312 bytes 'i40e_ddp_load'
-drivers/net/ethernet/intel/i40e/i40e_main.c:14691:12: 1312 bytes 'i40e_probe'
-drivers/net/ethernet/netronome/nfp/bpf/jit.c:2843:1: 1504 bytes 'mem_ldx'
+drivers/net/ethernet/intel/i40e/i40e_main.c:14691:12: 1344 bytes 'i40e_probe'
+drivers/net/ethernet/microchip/enc28j60.c:649:12: 1312 bytes 'enc28j60_hw_init'
drivers/net/ethernet/netronome/nfp/bpf/jit.c:3353:12: 1312 bytes 'call'
drivers/net/ethernet/netronome/nfp/bpf/jit.c:4452:5: 1344 bytes 'nfp_bpf_jit'
drivers/net/ethernet/qlogic/qede/qede_filter.c:1266:6: 2496 bytes
'qede_config_rx_mode'
drivers/net/ethernet/qlogic/qed/qed_int.c:1213:6: 1696 bytes 'qed_int_sp_dpc'
drivers/net/ethernet/qlogic/qed/qed_sriov.c:3861:13: 1856 bytes
'qed_iov_process_mbx_req'
drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c:504:12: 1408
bytes 'qlcnic_sriov_pf_init'
drivers/net/ieee802154/ca8210.c:2128:12: 1440 bytes 'ca8210_set_hw_addr_filt'
drivers/net/ieee802154/ca8210.c:2286:12: 1440 bytes 'ca8210_set_csma_params'
-drivers/net/ieee802154/ca8210.c:2485:16: 2528 bytes
'ca8210_test_int_user_write'
+drivers/net/ieee802154/ca8210.c:2485:16: 2560 bytes
'ca8210_test_int_user_write'
drivers/net/ieee802154/ca8210.c:3108:12: 5600 bytes 'ca8210_probe'
-drivers/net/usb/r8152.c:3303:13: 1408 bytes 'r8153_hw_phy_cfg'
-drivers/net/usb/r8152.c:3376:13: 1504 bytes 'r8153b_hw_phy_cfg'
+drivers/net/usb/r8152.c:3376:13: 1344 bytes 'r8153b_hw_phy_cfg'
+drivers/net/usb/smsc75xx.c:818:12: 1408 bytes 'smsc75xx_phy_initialize'
drivers/net/wan/slic_ds26522.c:205:12: 6944 bytes 'slic_ds26522_probe'
drivers/net/wimax/i2400m/fw.c:1480:5: 1568 bytes 'i2400m_fw_bootstrap'
drivers/net/wimax/i2400m/rx.c:1244:5: 1792 bytes 'i2400m_rx'
drivers/net/wimax/i2400m/usb-rx.c:328:5: 1344 bytes 'i2400mu_rxd'
drivers/net/wireless/ath/ar5523/ar5523.c:1574:12: 1536 bytes 'ar5523_probe'
drivers/net/wireless/ath/ar5523/ar5523.c:987:12: 1344 bytes 'ar5523_start'
-drivers/net/wireless/atmel/atmel.c:1156:20: 2240 bytes 'service_interrupt'
-drivers/net/wireless/atmel/atmel.c:1307:5: 1568 bytes 'atmel_open'
+drivers/net/wireless/atmel/atmel.c:1156:20: 1664 bytes 'service_interrupt'
+drivers/net/wireless/atmel/atmel.c:1307:5: 1440 bytes 'atmel_open'
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c:2621:1:
1440 bytes 'wlc_lcnphy_tx_iqlo_cal'
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c:4814:6:
1952 bytes 'wlc_phy_init_lcnphy'
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:15430:13:
2304 bytes 'wlc_phy_workarounds_nphy_gainctrl'
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:17019:13:
6560 bytes 'wlc_phy_workarounds_nphy'
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:19205:6:
1888 bytes 'wlc_phy_init_nphy'
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:25662:1:
2848 bytes 'wlc_phy_cal_txiqlo_nphy'
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:27723:1:
1312 bytes 'wlc_phy_cal_rxiq_nphy'
-drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:28334:1:
2080 bytes 'wlc_phy_txpwr_index_nphy'
+drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:28334:1:
2112 bytes 'wlc_phy_txpwr_index_nphy'
drivers/net/wireless/cisco/airo.c:3050:12: 3968 bytes 'airo_thread'
drivers/net/wireless/cisco/airo.c:3793:12: 1568 bytes 'setup_card'
drivers/net/wireless/intel/ipw2x00/ipw2100.c:1710:12: 3616 bytes 'ipw2100_up'
drivers/net/wireless/intel/ipw2x00/ipw2100.c:5473:12: 2656 bytes
'ipw2100_configure_security'
drivers/net/wireless/intel/ipw2x00/ipw2100.c:7332:12: 2016 bytes
'ipw2100_wx_set_retry'
drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c:1168:12: 1504 bytes
'iwl_mvm_mac_ctx_send'
-drivers/net/wireless/intersil/p54/p54spi.c:134:12: 1888 bytes
'p54spi_spi_write_dma'
-drivers/net/wireless/intersil/p54/p54spi.c:326:12: 2592 bytes 'p54spi_rx'
-drivers/net/wireless/intersil/p54/p54spi.c:478:13: 3456 bytes 'p54spi_work'
-drivers/net/wireless/intersil/p54/p54spi.c:517:12: 2304 bytes 'p54spi_op_start'
+drivers/net/wireless/intersil/p54/p54spi.c:478:13: 1440 bytes 'p54spi_work'
drivers/net/wireless/mediatek/mt76/mt76x0/phy.c:908:5: 1760 bytes
'mt76x0_phy_set_channel'
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:3349:13: 2048 bytes
'rt2800_config_channel_rf55xx'
-drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4011:13: 3360 bytes
'rt2800_config_channel'
-drivers/net/wireless/ralink/rt2x00/rt2800lib.c:6918:13: 4640 bytes
'rt2800_init_bbp_6352'
+drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4011:13: 3392 bytes
'rt2800_config_channel'
+drivers/net/wireless/ralink/rt2x00/rt2800lib.c:6918:13: 4672 bytes
'rt2800_init_bbp_6352'
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:7122:13: 4832 bytes
'rt2800_init_bbp'
-drivers/net/wireless/ralink/rt2x00/rt2800lib.c:8033:13: 1440 bytes
'rt2800_init_rfcsr_3883'
+drivers/net/wireless/ralink/rt2x00/rt2800lib.c:8033:13: 1472 bytes
'rt2800_init_rfcsr_3883'
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:8723:13: 6816 bytes
'rt2800_init_rfcsr_6352'
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:8991:13: 4832 bytes
'rt2800_init_rfcsr'
-drivers/net/wireless/ralink/rt2x00/rt73usb.c:1407:12: 1920 bytes
'rt73usb_set_device_state'
+drivers/net/wireless/ralink/rt2x00/rt73usb.c:1407:12: 1888 bytes
'rt73usb_set_device_state'
drivers/net/wireless/realtek/rtl818x/rtl8180/rtl8225se.c:299:6: 1312
bytes 'rtl8225se_rf_init'
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c:3345:13:
2016 bytes 'btc8723b2ant_run_coexist_mechanism'
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3380:13:
2048 bytes 'btc8821a2ant_run_coexist_mechanism'
+drivers/power/supply/bq2415x_charger.c:301:12: 2496 bytes
'bq2415x_exec_command'
drivers/rtc/rtc-mcp795.c:160:12: 1824 bytes 'mcp795_update_alarm'
drivers/rtc/rtc-mcp795.c:183:12: 2880 bytes 'mcp795_set_time'
drivers/rtc/rtc-mcp795.c:381:12: 1856 bytes 'mcp795_probe'
drivers/rtc/rtc-r9701.c:95:12: 1600 bytes 'r9701_set_datetime'
-drivers/scsi/bfa/bfad_bsg.c:3551:1: 1760 bytes 'bfad_im_bsg_request'
-drivers/scsi/gdth.c:4020:13: 2560 bytes 'gdth_unlocked_ioctl'
+drivers/scsi/gdth.c:4020:13: 2624 bytes 'gdth_unlocked_ioctl'
drivers/scsi/mpt3sas/mpt3sas_ctl.c:2255:1: 1600 bytes '_ctl_ioctl_main'
drivers/scsi/mpt3sas/mpt3sas_scsih.c:9266:1: 2336 bytes '_mpt3sas_fw_work'
-drivers/scsi/pm8001/pm8001_hwi.c:4005:13: 1408 bytes 'process_one_iomb'
-drivers/scsi/pm8001/pm80xx_hwi.c:3556:13: 2688 bytes 'process_one_iomb'
-drivers/scsi/qla2xxx/qla_bsg.c:2434:1: 1408 bytes
'qla2x00_process_vendor_specific'
+drivers/scsi/pm8001/pm8001_hwi.c:4005:13: 1344 bytes 'process_one_iomb'
+drivers/scsi/pm8001/pm80xx_hwi.c:3556:13: 2624 bytes 'process_one_iomb'
+drivers/scsi/qla2xxx/qla_bsg.c:2434:1: 1472 bytes
'qla2x00_process_vendor_specific'
+drivers/scsi/qla4xxx/ql4_nx.c:3242:6: 1536 bytes 'qla4_8xxx_get_minidump'
drivers/staging/fbtft/fbtft-core.c:989:5: 1376 bytes 'fbtft_init_display'
drivers/staging/iio/adc/ad7280a.c:914:12: 1856 bytes 'ad7280_probe'
+drivers/staging/isdn/avm/b1.c:490:13: 1408 bytes 'b1_interrupt'
+drivers/staging/isdn/avm/b1dma.c:291:5: 1696 bytes 't1pci_detect'
drivers/staging/pi433/rf69.c:171:5: 1824 bytes 'rf69_set_modulation_shaping'
drivers/staging/pi433/rf69.c:703:5: 1824 bytes 'rf69_set_sync_values'
drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:91:23: 1376 bytes
'translate_scan'
drivers/staging/rtl8723bs/hal/HalBtc8723b2Ant.c:2667:13: 1472 bytes
'halbtc8723b2ant_RunCoexistMechanism'
-drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c:1700:5:
1344 bytes 'vchiq_mmal_component_finalise'
+drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c:1700:5:
1376 bytes 'vchiq_mmal_component_finalise'
drivers/staging/wlan-ng/cfg80211.c:272:12: 1376 bytes 'prism2_scan'
drivers/staging/wlan-ng/cfg80211.c:433:12: 3808 bytes 'prism2_connect'
drivers/tty/rocket.c:1275:12: 1472 bytes 'rp_ioctl'
drivers/tty/serial/8250/serial_cs.c:610:12: 4384 bytes 'serial_config'
drivers/usb/core/devio.c:2392:13: 1408 bytes 'usbdev_do_ioctl'
drivers/usb/host/max3421-hcd.c:1395:1: 5280 bytes 'max3421_spi_thread'
drivers/usb/host/max3421-hcd.c:604:1: 1600 bytes 'max3421_next_transfer'
drivers/usb/misc/sisusbvga/sisusb.c:1750:13: 8096 bytes
'sisusb_set_default_mode'
-drivers/usb/misc/sisusbvga/sisusb.c:1878:12: 12480 bytes 'sisusb_init_gfxcore'
-drivers/usb/misc/sisusbvga/sisusb.c:2203:12: 1376 bytes 'sisusb_init_gfxdevice'
-drivers/usb/typec/tcpm/tcpm.c:2838:13: 1408 bytes 'run_state_machine'
-drivers/vhost/vhost.c:3033:6: 1344 bytes 'vhost_enable_notify'
+drivers/usb/misc/sisusbvga/sisusb.c:1878:12: 11072 bytes 'sisusb_init_gfxcore'
+drivers/usb/misc/sisusbvga/sisusb.c:2203:12: 1408 bytes 'sisusb_init_gfxdevice'
+drivers/usb/typec/tcpm/tcpm.c:2838:13: 1472 bytes 'run_state_machine'
+drivers/usb/wusbcore/crypto.c:242:9: 1408 bytes 'wusb_prf'
+drivers/vhost/vhost.c:3033:6: 1376 bytes 'vhost_enable_notify'
drivers/video/backlight/ams369fg06.c:209:12: 1664 bytes '_ams369fg06_gamma_ctl'
drivers/video/backlight/lms501kf03.c:221:12: 5856 bytes 'lms501kf03_power_on'
-drivers/video/fbdev/aty/aty128fb.c:2072:12: 1408 bytes 'aty128_probe'
+drivers/video/fbdev/aty/aty128fb.c:2072:12: 1440 bytes 'aty128_probe'
fs/btrfs/check-integrity.c:943:31: 1408 bytes 'btrfsic_process_metablock'
fs/btrfs/disk-io.c:2616:5: 1664 bytes 'open_ctree'
fs/btrfs/ioctl.c:5550:6: 1760 bytes 'btrfs_ioctl'
fs/btrfs/tree-log.c:5079:12: 1504 bytes 'btrfs_log_inode'
-fs/crypto/keyinfo.c:502:5: 1536 bytes 'fscrypt_get_encryption_info'
+fs/crypto/keyinfo.c:502:5: 1664 bytes 'fscrypt_get_encryption_info'
fs/ext4/extents.c:1948:5: 1664 bytes 'ext4_ext_insert_extent'
fs/ext4/inode.c:53:14: 2560 bytes 'ext4_inode_csum'
fs/ext4/namei.c:102:28: 1408 bytes '__ext4_read_dirblock'
-fs/ext4/namei.c:3977:12: 1664 bytes 'ext4_rename2'
fs/ext4/namei.c:483:13: 1408 bytes 'ext4_dx_csum_set'
fs/ext4/super.c:2380:15: 1920 bytes 'ext4_group_desc_csum'
fs/ext4/super.c:3604:12: 1664 bytes 'ext4_fill_super'
fs/ext4/xattr.c:128:15: 1920 bytes 'ext4_xattr_block_csum'
+fs/f2fs/checkpoint.c:1549:5: 1408 bytes 'f2fs_write_checkpoint'
fs/f2fs/inode.c:158:6: 2304 bytes 'f2fs_inode_chksum_verify'
fs/f2fs/inode.c:185:6: 2304 bytes 'f2fs_inode_chksum_set'
fs/f2fs/super.c:3048:12: 1536 bytes 'f2fs_fill_super'
-fs/gfs2/ops_fstype.c:1235:23: 1376 bytes 'gfs2_mount'
fs/jbd2/commit.c:355:6: 1792 bytes 'jbd2_journal_commit_transaction'
fs/jbd2/recovery.c:416:12: 2432 bytes 'do_one_pass'
fs/ocfs2/alloc.c:4239:12: 1504 bytes 'ocfs2_do_insert_extent'
-fs/ocfs2/aops.c:1658:5: 1728 bytes 'ocfs2_write_begin_nolock'
-fs/ocfs2/cluster/heartbeat.c:1201:12: 1696 bytes 'o2hb_thread'
+fs/ocfs2/aops.c:1658:5: 1760 bytes 'ocfs2_write_begin_nolock'
+fs/ocfs2/cluster/heartbeat.c:1201:12: 1728 bytes 'o2hb_thread'
fs/ocfs2/cluster/tcp.c:1424:13: 1312 bytes 'o2net_rx_until_empty'
-fs/ocfs2/dir.c:3148:12: 1792 bytes 'ocfs2_extend_dir'
-fs/ocfs2/dir.c:4236:5: 1952 bytes 'ocfs2_prepare_dir_for_insert'
+fs/ocfs2/dir.c:3148:12: 1728 bytes 'ocfs2_extend_dir'
+fs/ocfs2/dir.c:4236:5: 1856 bytes 'ocfs2_prepare_dir_for_insert'
fs/ocfs2/dlm/dlmdomain.c:2106:19: 2784 bytes 'dlm_register_domain'
-fs/ocfs2/dlm/dlmmaster.c:703:28: 2048 bytes 'dlm_get_lock_resource'
-fs/ocfs2/dlm/dlmrecovery.c:286:12: 3424 bytes 'dlm_recovery_thread'
+fs/ocfs2/dlm/dlmmaster.c:2766:5: 1312 bytes 'dlm_empty_lockres'
+fs/ocfs2/dlm/dlmmaster.c:703:28: 2080 bytes 'dlm_get_lock_resource'
+fs/ocfs2/dlm/dlmrecovery.c:286:12: 3488 bytes 'dlm_recovery_thread'
fs/ocfs2/inode.c:1214:6: 1728 bytes 'ocfs2_evict_inode'
fs/ocfs2/ioctl.c:836:6: 2016 bytes 'ocfs2_ioctl'
fs/ocfs2/move_extents.c:985:5: 1856 bytes 'ocfs2_ioctl_move_extents'
fs/ocfs2/namei.c:1192:12: 1696 bytes 'ocfs2_rename'
fs/ocfs2/namei.c:1781:12: 1472 bytes 'ocfs2_symlink'
-fs/ocfs2/refcounttree.c:4418:5: 1952 bytes 'ocfs2_reflink_ioctl'
+fs/ocfs2/refcounttree.c:4418:5: 1984 bytes 'ocfs2_reflink_ioctl'
fs/ocfs2/super.c:972:12: 3168 bytes 'ocfs2_fill_super'
fs/ocfs2/xattr.c:2952:12: 1472 bytes 'ocfs2_xattr_block_set'
+fs/ocfs2/xattr.c:3527:5: 1344 bytes 'ocfs2_xattr_set'
fs/ocfs2/xattr.c:7133:5: 1312 bytes 'ocfs2_reflink_xattrs'
fs/quota/quota.c:823:5: 3360 bytes 'kernel_quotactl'
fs/reiserfs/namei.c:1304:12: 1696 bytes 'reiserfs_rename'
fs/select.c:621:5: 1312 bytes 'core_sys_select'
fs/ubifs/auth.c:78:5: 1920 bytes 'ubifs_prepare_auth_node'
fs/ubifs/master.c:332:5: 1408 bytes 'ubifs_read_master'
-include/linux/module.h:76:12: 2400 bytes 'init_module'
+include/linux/module.h:76:12: 2368 bytes 'init_module'
kernel/bpf/verifier.c:7459:12: 2496 bytes 'do_check'
-kernel/locking/lockdep.c:3737:12: 2112 bytes '__lock_acquire'
-net/caif/cfctrl.c:350:12: 1600 bytes 'cfctrl_recv'
+net/caif/cfctrl.c:350:12: 1632 bytes 'cfctrl_recv'
net/core/ethtool.c:2553:5: 2976 bytes 'dev_ethtool'
net/dcb/dcbnl.c:1031:12: 1504 bytes 'dcbnl_ieee_fill'
net/mac80211/mlme.c:4047:6: 2336 bytes 'ieee80211_sta_rx_queued_mgmt'
net/netfilter/ipvs/ip_vs_nfct.c:140:13: 1440 bytes 'ip_vs_nfct_expect_callback'
net/sctp/socket.c:4605:12: 1664 bytes 'sctp_setsockopt'
net/sctp/socket.c:7765:12: 2272 bytes 'sctp_getsockopt'
net/sunrpc/auth_gss/svcauth_gss.c:1437:1: 1664 bytes 'svcauth_gss_accept'
net/wireless/nl80211.c:1826:12: 2016 bytes 'nl80211_send_wiphy'
security/integrity/ima/ima_crypto.c:401:5: 1408 bytes 'ima_calc_file_hash'
security/integrity/ima/ima_crypto.c:504:5: 1408 bytes
'ima_calc_field_array_hash'
security/keys/encrypted-keys/encrypted.c:793:12: 1536 bytes
'encrypted_instantiate'
security/keys/encrypted-keys/encrypted.c:912:13: 1408 bytes 'encrypted_read'
-sound/pci/ac97/ac97_codec.c:1998:5: 2464 bytes 'snd_ac97_mixer'
+sound/pci/ac97/ac97_codec.c:1998:5: 2496 bytes 'snd_ac97_mixer'
sound/pci/emu10k1/emumixer.c:1775:5: 1472 bytes 'snd_emu10k1_mixer'
sound/pci/hda/patch_ca0132.c:6363:12: 1632 bytes 'ca0132_build_controls'
+sound/pci/hda/patch_ca0132.c:6950:13: 2144 bytes 'sbz_chipio_startup_data'
+sound/pci/hda/patch_ca0132.c:8269:12: 1312 bytes 'ca0132_init'
sound/spi/at73c213.c:1012:12: 2656 bytes 'snd_at73c213_remove'
sound/spi/at73c213.c:882:12: 3424 bytes 'snd_at73c213_dev_init'
^ permalink raw reply
* Re: [PATCH] selinux: check sidtab limit before adding a new entry
From: Kees Cook @ 2019-07-22 16:50 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: selinux, Paul Moore, NitinGote, kernel-hardening,
linux-security-module
In-Reply-To: <20190722132111.25743-1-omosnace@redhat.com>
On Mon, Jul 22, 2019 at 03:21:11PM +0200, Ondrej Mosnacek wrote:
> We need to error out when trying to add an entry above SIDTAB_MAX in
> sidtab_reverse_lookup() to avoid overflow on the odd chance that this
> happens.
>
> Fixes: ee1a84fdfeed ("selinux: overhaul sidtab to fix bug and improve performance")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Is this reachable from unprivileged userspace?
> ---
> security/selinux/ss/sidtab.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
> index e63a90ff2728..54c1ba1e79ab 100644
> --- a/security/selinux/ss/sidtab.c
> +++ b/security/selinux/ss/sidtab.c
> @@ -286,6 +286,11 @@ static int sidtab_reverse_lookup(struct sidtab *s, struct context *context,
> ++count;
> }
>
> + /* bail out if we already reached max entries */
> + rc = -ENOMEM;
> + if (count == SIDTAB_MAX)
Do you want to use >= here instead?
-Kees
> + goto out_unlock;
> +
> /* insert context into new entry */
> rc = -ENOMEM;
> dst = sidtab_do_lookup(s, count, 1);
> --
> 2.21.0
>
--
Kees Cook
^ permalink raw reply
* Re: Dbus and multiple LSMs (was Preferred subj= with multiple LSMs)
From: Casey Schaufler @ 2019-07-22 16:04 UTC (permalink / raw)
To: Simon McVittie
Cc: Paul Moore, Steve Grubb, Richard Guy Briggs,
linux-audit@redhat.com, Linux Security Module list, SELinux,
casey
In-Reply-To: <20190722113636.GA12250@horizon>
On 7/22/2019 4:36 AM, Simon McVittie wrote:
> On Fri, 19 Jul 2019 at 13:02:24 -0700, Casey Schaufler wrote:
>> On 7/19/2019 11:47 AM, Simon McVittie wrote:
>>> I was hoping the syscall wrappers in glibc would be a viable user-space
>>> interface to the small amount of LSM stuff that dbus needs to use in an
>>> LSM-agnostic way.
>> I don't see how to do that without making the Fedora and Ubuntu user space
>> environments [not] remain functional.
> What I was thinking of was a second, parallel kernel <-> user-space
> interface (like the SO_PEERSECLABELS that I suggested) for future/updated
> user-space components. SO_PEERSEC would continue to return some
> hopefully-backwards-compatible thing, but would be deprecated, because it
> cannot fully represent the reality of LSM stacking while remaining
> backwards-compatible.
I will propose SO_PEERCONTEXT and /proc/.../attr/stack/context,
both of which will use the Hideous format, in the next round. I
appreciate the suggestion and discussion.
>> I see display being used in scripts:
>>
>> echo apparmor > /proc/self/attr/display
>> apparmor-do-stuff --options --deamon
>>
>> much more than inside new or updated programs.
> Note that this implicitly relies on echo being a shell builtin, which
> is common but not guaranteed (I don't think). It would work in bash or
> dash, though.
Yes, echo being built-in can't be guaranteed. Most shells have some
way of doing the equivalent.
> If apparmor-do-stuff no longer works, and you have to wrap a shell
> script around it, isn't that the same amount of user-space breakage as
> if apparmor-do-stuff no longer works and you have to install a newer
> version that does work?
True when there is such a newer version. I'm sure you're aware
of how much system software out there hasn't been updated in this
century.
> Either way, the sysadmin must take action to
> change user-space components. I think the attr/display thing only reduces
> the magnitude of the user-space changes required to catch up, and doesn't
> eliminate the fact that those changes were needed.
Agreed. It's a tool for the times of transition.
>>> Lots of programs (including dbus-daemon) fork-and-exec arbitrary
>>> child processes that come from a different codebase not under our
>>> control and aren't necessarily LSM-stacking-aware. I don't really want
>>> to have to reset /proc/self/attr/display in our increasingly crowded
>>> after-fork-but-before-exec code path
>> My hope is that new and updated programs will have to tools
>> they need to get it right, and that those that don't won't
>> fall over on a well configured system.
> The problem I see here is that if we assume dbus-daemon is a new/updated
> program that has set /proc/self/attr/display = "hideous" in order to get
> the full stack of labels for its peer processes, then it will be causing
> side-effects on its separately-maintained child processes - they will
> no longer be able to benefit from the backwards-compatility thing where
> /proc/self/attr/display (effectively) defaults to the first LSM that
> has labels, because dbus-daemon overrode that (unless dbus-daemon takes
> action to reverse it between fork and exec). This partially defeats the
> semi-backwards-compatible handling of the existing kernel interfaces.
Point. /proc/self/attr/stack/context and SO_PEERCONTEXT comprise a better,
more reliable solution.
> If dbus-daemon could read SO_PEERSECLABELS instead of SO_PEERSEC and
> read /proc/<pid>/attr/current_stack instead of /proc/<pid>/attr/current,
> leaving /proc/self/attr/display untouched, then this concern would go away.
I agree.
> Similarly, dbus-daemon can be linked to libselinux and/or libapparmor
> (on Debian it's linked to both, even in the non-stackable present,
> and the right one for the kernel configuration is chosen at runtime).
> If one of those libraries wrote to /proc/self/attr/display, then the rest
> of dbus-daemon's main thread and all child processes would implicitly be
> getting the result of that - even if dbus-daemon itself had not yet been
> updated for stacked LSMs (in which case it cannot be expected to reverse
> their action between fork and exec, because it's an older codebase that
> doesn't yet know that "big" LSMs can be stacked).
Yes.
> So I think libselinux and libapparmor should be enhanced to use
> new kernel interfaces that get the label they want to get (either
> just that label, or all the labels), instead of being enhanced to
> write /proc/self/attr/display to change the meaning of old kernel
> interfaces. Otherwise they can break other code in their process or
> their subprocesses.
The AppArmor team is already moving away from using the /proc/self/attr
intefaces. /proc/self/attr/smack is already there, and the transition
begun. The SELinux developers seem firmly set in the position that there
is no reason they should ever change. In the long term I think we'll get
the conflict sorted out. It's hard to say what value of "long term"
we're looking at.
>>> instead of repurposing /proc/<pid>/attr/current
>>> and SO_PEERSEC to have contents that vary according to ambient process
>>> state in their reader?
>> In addition, yes. Instead of? I don't think that we can have a
>> backward compatibility story that flies without it.
> Consider only SELinux and AppArmor for a moment (I know there are other
> "big" LSMs like Smack, but this same reasoning applies to any pair, with
> appropriate search-and-replace on their names).
>
> Neither SELinux nor AppArmor: there are no labels, nothing changed.
>
> AppArmor is the only "big" LSM in the stack (Ubuntu): previously,
> the label was the AppArmor label; now, if attr/display is not altered,
> the label is the one used by the first "big" LSM in the stack, which is
> AppArmor. Nothing changed.
>
> SELinux is the only "big" LSM in the stack (Red Hat): same as for AppArmor
> being the only "big" LSM in the stack, but with s/AppArmor/SELinux/.
>
> SELinux and AppArmor stacked: this is a situation that could not exist
> before, so distro/sysadmin action must have been necessary to make it
> happen. However much ambient process state is invented, I don't see any
> way to make both SELinux and AppArmor user-space work without modifications:
> at least one of them (the one that is second in the stack) has to use new
> kernel interfaces, or alter attr/display to change the meaning of the old
> kernel interfaces, or something similar, to get the second LSM's labels.
> So distro/sysadmin action in user-space is also going to be necessary here
> whatever happens - backward compatibility has already been broken, it's
> only a question of how intrusive the user-space changes are. Is it really
> so much worse if the distro/sysadmin action taken to update user-space
> has to take the form of using new kernel interfaces that always do the
> same thing, rather than changing the meaning of old kernel interfaces?
In addition to the big name distros/systems like RedHat, Ubuntu and
Android there are a bunch of smaller players who don't have the
expertise and/or staffing and/or upstream clout to update system
services. Some of these are targets for stacked LSMs. They will be
delighted to get updated programs, but will muddle through with the
compatibility mechanisms if they have to.
> smcv
Thank you again for your insights on this topic. My next round
should provide what you've suggested.
^ permalink raw reply
* Re: [PATCH] [RESEND v2] structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK
From: Alexander Potapenko @ 2019-07-22 15:23 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Dmitriy Vyukov, Marco Elver, Andrew Morton, James Morris,
Serge E. Hallyn, Kees Cook, Ard Biesheuvel, Masahiro Yamada,
linux-security-module, LKML
In-Reply-To: <CAK8P3a28c5V5VnsFrttgtCC5+i87yuAT-G5xx50KOsKUJ6-VKg@mail.gmail.com>
On Mon, Jul 22, 2019 at 4:26 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Jul 22, 2019 at 3:43 PM Alexander Potapenko <glider@google.com> wrote:
> > On Mon, Jul 22, 2019 at 1:41 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > KASAN_STACK is currently implied by KASAN on gcc, but could be made a
> > > user selectable option if we want to allow combining (non-stack) KASAN
> > > with GCC_PLUGIN_STRUCTLEAK_BYREF.
> > >
> > > Note that it would be possible to specifically address the files that
> > > print the warning, but presumably the overall stack usage is still
> > > significantly higher than in other configurations, so this would not
> > > address the full problem.
> > >
> > > I could not test this with CONFIG_INIT_STACK_ALL, which may or may not
> > > suffer from a similar problem.
> > We would love to be able to run KASAN together with
> > CONFIG_INIT_STACK_ALL on syzbot, as this will potentially reduce the
> > number of flaky errors.
>
> Doesn't that just limit the usefulness of KASAN, as you no longer catch
> actual accesses to unintialized variables that KASAN is designed to find?
KASAN (unlike KMSAN) doesn't detect accesses to uninitialized variables.
It can of course detect bugs induced by e.g. treating an uninitialized
variable as a pointer or an array index.
Depending on the pattern used to initialize those variables, this can
indeed mask some bugs, but OTOH others will become more reproducible.
I'm not really sure KASAN+CONFIG_INIT_STACK_ALL is a problem right
now, will need to take a look.
> > Given that we already increase the stack size in KASAN builds, how big
> > of a problem are these warnings?
> > Perhaps it's better to disable them in this configuration, or push the limit up?
>
> I'm really hoping to lower the per-function limit for 'allmodconfig' builds,
> since both a high limit and lots of bogus warnings prevent us from
> noticing any newly introduced functions that use a lot of kernel stack
> without KASAN.
>
> An allmodconfig build (and ideally also any randconfig build) should always
> complete without warnings to be useful for compile testing.
>
> Arnd
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply
* Re: [PATCH] [RESEND v2] structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK
From: Arnd Bergmann @ 2019-07-22 14:26 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Dmitriy Vyukov, Marco Elver, Andrew Morton, James Morris,
Serge E. Hallyn, Kees Cook, Ard Biesheuvel, Masahiro Yamada,
linux-security-module, LKML
In-Reply-To: <CAG_fn=UxowACw5w+erKaAPRr4SWk3WbLTfAgJj=cOL4HgZHK=Q@mail.gmail.com>
On Mon, Jul 22, 2019 at 3:43 PM Alexander Potapenko <glider@google.com> wrote:
> On Mon, Jul 22, 2019 at 1:41 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > KASAN_STACK is currently implied by KASAN on gcc, but could be made a
> > user selectable option if we want to allow combining (non-stack) KASAN
> > with GCC_PLUGIN_STRUCTLEAK_BYREF.
> >
> > Note that it would be possible to specifically address the files that
> > print the warning, but presumably the overall stack usage is still
> > significantly higher than in other configurations, so this would not
> > address the full problem.
> >
> > I could not test this with CONFIG_INIT_STACK_ALL, which may or may not
> > suffer from a similar problem.
> We would love to be able to run KASAN together with
> CONFIG_INIT_STACK_ALL on syzbot, as this will potentially reduce the
> number of flaky errors.
Doesn't that just limit the usefulness of KASAN, as you no longer catch
actual accesses to unintialized variables that KASAN is designed to find?
> Given that we already increase the stack size in KASAN builds, how big
> of a problem are these warnings?
> Perhaps it's better to disable them in this configuration, or push the limit up?
I'm really hoping to lower the per-function limit for 'allmodconfig' builds,
since both a high limit and lots of bogus warnings prevent us from
noticing any newly introduced functions that use a lot of kernel stack
without KASAN.
An allmodconfig build (and ideally also any randconfig build) should always
complete without warnings to be useful for compile testing.
Arnd
^ permalink raw reply
* Re: [PATCH] [RESEND v2] structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK
From: Alexander Potapenko @ 2019-07-22 13:42 UTC (permalink / raw)
To: Arnd Bergmann, Dmitriy Vyukov, Marco Elver
Cc: Andrew Morton, James Morris, Serge E. Hallyn, Kees Cook,
Ard Biesheuvel, Masahiro Yamada, linux-security-module, LKML
In-Reply-To: <20190722114134.3123901-1-arnd@arndb.de>
On Mon, Jul 22, 2019 at 1:41 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> The combination of KASAN_STACK and GCC_PLUGIN_STRUCTLEAK_BYREF
> leads to much larger kernel stack usage, as seen from the warnings
> about functions that now exceed the 2048 byte limit:
>
> drivers/media/i2c/tvp5150.c:253:1: error: the frame size of 3936 bytes is larger than 2048 bytes
> drivers/media/tuners/r820t.c:1327:1: error: the frame size of 2816 bytes is larger than 2048 bytes
> drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:16552:1: error: the frame size of 3144 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> fs/ocfs2/aops.c:1892:1: error: the frame size of 2088 bytes is larger than 2048 bytes
> fs/ocfs2/dlm/dlmrecovery.c:737:1: error: the frame size of 2088 bytes is larger than 2048 bytes
> fs/ocfs2/namei.c:1677:1: error: the frame size of 2584 bytes is larger than 2048 bytes
> fs/ocfs2/super.c:1186:1: error: the frame size of 2640 bytes is larger than 2048 bytes
> fs/ocfs2/xattr.c:3678:1: error: the frame size of 2176 bytes is larger than 2048 bytes
> net/bluetooth/l2cap_core.c:7056:1: error: the frame size of 2144 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> net/bluetooth/l2cap_core.c: In function 'l2cap_recv_frame':
> net/bridge/br_netlink.c:1505:1: error: the frame size of 2448 bytes is larger than 2048 bytes
> net/ieee802154/nl802154.c:548:1: error: the frame size of 2232 bytes is larger than 2048 bytes
> net/wireless/nl80211.c:1726:1: error: the frame size of 2224 bytes is larger than 2048 bytes
> net/wireless/nl80211.c:2357:1: error: the frame size of 4584 bytes is larger than 2048 bytes
> net/wireless/nl80211.c:5108:1: error: the frame size of 2760 bytes is larger than 2048 bytes
> net/wireless/nl80211.c:6472:1: error: the frame size of 2112 bytes is larger than 2048 bytes
>
> The structleak plugin was previously disabled for CONFIG_COMPILE_TEST,
> but meant we missed some bugs, so this time we should address them.
>
> The frame size warnings are distracting, and risking a kernel stack
> overflow is generally not beneficial to performance, so it may be best
> to disallow that particular combination. This can be done by turning
> off either one. I picked the dependency in GCC_PLUGIN_STRUCTLEAK_BYREF
> and GCC_PLUGIN_STRUCTLEAK_BYREF_ALL, as this option is designed to
> make uninitialized stack usage less harmful when enabled on its own,
> but it also prevents KASAN from detecting those cases in which it was
> in fact needed.
>
> KASAN_STACK is currently implied by KASAN on gcc, but could be made a
> user selectable option if we want to allow combining (non-stack) KASAN
> with GCC_PLUGIN_STRUCTLEAK_BYREF.
>
> Note that it would be possible to specifically address the files that
> print the warning, but presumably the overall stack usage is still
> significantly higher than in other configurations, so this would not
> address the full problem.
>
> I could not test this with CONFIG_INIT_STACK_ALL, which may or may not
> suffer from a similar problem.
We would love to be able to run KASAN together with
CONFIG_INIT_STACK_ALL on syzbot, as this will potentially reduce the
number of flaky errors.
Given that we already increase the stack size in KASAN builds, how big
of a problem are these warnings?
Perhaps it's better to disable them in this configuration, or push the limit up?
> Fixes: 81a56f6dcd20 ("gcc-plugins: structleak: Generalize to all variable types")
> Acked-by: Kees Cook <keescook@chromium.org>
> Link: https://lore.kernel.org/lkml/20190628123819.2785504-1-arnd@arndb.de/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> [v2] do it for both GCC_PLUGIN_STRUCTLEAK_BYREF and GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
>
> Andrew, can you pick this up in -mm? It looks like nobody else
> wanted it in their trees even though there was agreement on the
> patch itself.
> ---
> security/Kconfig.hardening | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
> index a1ffe2eb4d5f..af4c979b38ee 100644
> --- a/security/Kconfig.hardening
> +++ b/security/Kconfig.hardening
> @@ -61,6 +61,7 @@ choice
> config GCC_PLUGIN_STRUCTLEAK_BYREF
> bool "zero-init structs passed by reference (strong)"
> depends on GCC_PLUGINS
> + depends on !(KASAN && KASAN_STACK=1)
> select GCC_PLUGIN_STRUCTLEAK
> help
> Zero-initialize any structures on the stack that may
> @@ -70,9 +71,15 @@ choice
> exposures, like CVE-2017-1000410:
> https://git.kernel.org/linus/06e7e776ca4d3654
>
> + As a side-effect, this keeps a lot of variables on the
> + stack that can otherwise be optimized out, so combining
> + this with CONFIG_KASAN_STACK can lead to a stack overflow
> + and is disallowed.
> +
> config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
> bool "zero-init anything passed by reference (very strong)"
> depends on GCC_PLUGINS
> + depends on !(KASAN && KASAN_STACK=1)
> select GCC_PLUGIN_STRUCTLEAK
> help
> Zero-initialize any stack variables that may be passed
> --
> 2.20.0
>
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply
* [PATCH] [RESEND v2] structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK
From: Arnd Bergmann @ 2019-07-22 11:41 UTC (permalink / raw)
To: Andrew Morton, James Morris, Serge E. Hallyn
Cc: Arnd Bergmann, Kees Cook, Ard Biesheuvel, Masahiro Yamada,
Alexander Potapenko, linux-security-module, linux-kernel
The combination of KASAN_STACK and GCC_PLUGIN_STRUCTLEAK_BYREF
leads to much larger kernel stack usage, as seen from the warnings
about functions that now exceed the 2048 byte limit:
drivers/media/i2c/tvp5150.c:253:1: error: the frame size of 3936 bytes is larger than 2048 bytes
drivers/media/tuners/r820t.c:1327:1: error: the frame size of 2816 bytes is larger than 2048 bytes
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:16552:1: error: the frame size of 3144 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
fs/ocfs2/aops.c:1892:1: error: the frame size of 2088 bytes is larger than 2048 bytes
fs/ocfs2/dlm/dlmrecovery.c:737:1: error: the frame size of 2088 bytes is larger than 2048 bytes
fs/ocfs2/namei.c:1677:1: error: the frame size of 2584 bytes is larger than 2048 bytes
fs/ocfs2/super.c:1186:1: error: the frame size of 2640 bytes is larger than 2048 bytes
fs/ocfs2/xattr.c:3678:1: error: the frame size of 2176 bytes is larger than 2048 bytes
net/bluetooth/l2cap_core.c:7056:1: error: the frame size of 2144 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
net/bluetooth/l2cap_core.c: In function 'l2cap_recv_frame':
net/bridge/br_netlink.c:1505:1: error: the frame size of 2448 bytes is larger than 2048 bytes
net/ieee802154/nl802154.c:548:1: error: the frame size of 2232 bytes is larger than 2048 bytes
net/wireless/nl80211.c:1726:1: error: the frame size of 2224 bytes is larger than 2048 bytes
net/wireless/nl80211.c:2357:1: error: the frame size of 4584 bytes is larger than 2048 bytes
net/wireless/nl80211.c:5108:1: error: the frame size of 2760 bytes is larger than 2048 bytes
net/wireless/nl80211.c:6472:1: error: the frame size of 2112 bytes is larger than 2048 bytes
The structleak plugin was previously disabled for CONFIG_COMPILE_TEST,
but meant we missed some bugs, so this time we should address them.
The frame size warnings are distracting, and risking a kernel stack
overflow is generally not beneficial to performance, so it may be best
to disallow that particular combination. This can be done by turning
off either one. I picked the dependency in GCC_PLUGIN_STRUCTLEAK_BYREF
and GCC_PLUGIN_STRUCTLEAK_BYREF_ALL, as this option is designed to
make uninitialized stack usage less harmful when enabled on its own,
but it also prevents KASAN from detecting those cases in which it was
in fact needed.
KASAN_STACK is currently implied by KASAN on gcc, but could be made a
user selectable option if we want to allow combining (non-stack) KASAN
with GCC_PLUGIN_STRUCTLEAK_BYREF.
Note that it would be possible to specifically address the files that
print the warning, but presumably the overall stack usage is still
significantly higher than in other configurations, so this would not
address the full problem.
I could not test this with CONFIG_INIT_STACK_ALL, which may or may not
suffer from a similar problem.
Fixes: 81a56f6dcd20 ("gcc-plugins: structleak: Generalize to all variable types")
Acked-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/lkml/20190628123819.2785504-1-arnd@arndb.de/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
[v2] do it for both GCC_PLUGIN_STRUCTLEAK_BYREF and GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
Andrew, can you pick this up in -mm? It looks like nobody else
wanted it in their trees even though there was agreement on the
patch itself.
---
security/Kconfig.hardening | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index a1ffe2eb4d5f..af4c979b38ee 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -61,6 +61,7 @@ choice
config GCC_PLUGIN_STRUCTLEAK_BYREF
bool "zero-init structs passed by reference (strong)"
depends on GCC_PLUGINS
+ depends on !(KASAN && KASAN_STACK=1)
select GCC_PLUGIN_STRUCTLEAK
help
Zero-initialize any structures on the stack that may
@@ -70,9 +71,15 @@ choice
exposures, like CVE-2017-1000410:
https://git.kernel.org/linus/06e7e776ca4d3654
+ As a side-effect, this keeps a lot of variables on the
+ stack that can otherwise be optimized out, so combining
+ this with CONFIG_KASAN_STACK can lead to a stack overflow
+ and is disallowed.
+
config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
bool "zero-init anything passed by reference (very strong)"
depends on GCC_PLUGINS
+ depends on !(KASAN && KASAN_STACK=1)
select GCC_PLUGIN_STRUCTLEAK
help
Zero-initialize any stack variables that may be passed
--
2.20.0
^ permalink raw reply related
* Re: Dbus and multiple LSMs (was Preferred subj= with multiple LSMs)
From: Simon McVittie @ 2019-07-22 11:36 UTC (permalink / raw)
To: Casey Schaufler
Cc: Paul Moore, Steve Grubb, Richard Guy Briggs,
linux-audit@redhat.com, Linux Security Module list, SELinux
In-Reply-To: <700301b3-b607-3234-15ae-b958df2b59d3@schaufler-ca.com>
On Fri, 19 Jul 2019 at 13:02:24 -0700, Casey Schaufler wrote:
> On 7/19/2019 11:47 AM, Simon McVittie wrote:
> > I was hoping the syscall wrappers in glibc would be a viable user-space
> > interface to the small amount of LSM stuff that dbus needs to use in an
> > LSM-agnostic way.
>
> I don't see how to do that without making the Fedora and Ubuntu user space
> environments [not] remain functional.
What I was thinking of was a second, parallel kernel <-> user-space
interface (like the SO_PEERSECLABELS that I suggested) for future/updated
user-space components. SO_PEERSEC would continue to return some
hopefully-backwards-compatible thing, but would be deprecated, because it
cannot fully represent the reality of LSM stacking while remaining
backwards-compatible.
> I see display being used in scripts:
>
> echo apparmor > /proc/self/attr/display
> apparmor-do-stuff --options --deamon
>
> much more than inside new or updated programs.
Note that this implicitly relies on echo being a shell builtin, which
is common but not guaranteed (I don't think). It would work in bash or
dash, though.
If apparmor-do-stuff no longer works, and you have to wrap a shell
script around it, isn't that the same amount of user-space breakage as
if apparmor-do-stuff no longer works and you have to install a newer
version that does work? Either way, the sysadmin must take action to
change user-space components. I think the attr/display thing only reduces
the magnitude of the user-space changes required to catch up, and doesn't
eliminate the fact that those changes were needed.
> > Lots of programs (including dbus-daemon) fork-and-exec arbitrary
> > child processes that come from a different codebase not under our
> > control and aren't necessarily LSM-stacking-aware. I don't really want
> > to have to reset /proc/self/attr/display in our increasingly crowded
> > after-fork-but-before-exec code path
>
> My hope is that new and updated programs will have to tools
> they need to get it right, and that those that don't won't
> fall over on a well configured system.
The problem I see here is that if we assume dbus-daemon is a new/updated
program that has set /proc/self/attr/display = "hideous" in order to get
the full stack of labels for its peer processes, then it will be causing
side-effects on its separately-maintained child processes - they will
no longer be able to benefit from the backwards-compatility thing where
/proc/self/attr/display (effectively) defaults to the first LSM that
has labels, because dbus-daemon overrode that (unless dbus-daemon takes
action to reverse it between fork and exec). This partially defeats the
semi-backwards-compatible handling of the existing kernel interfaces.
If dbus-daemon could read SO_PEERSECLABELS instead of SO_PEERSEC and
read /proc/<pid>/attr/current_stack instead of /proc/<pid>/attr/current,
leaving /proc/self/attr/display untouched, then this concern would go away.
Similarly, dbus-daemon can be linked to libselinux and/or libapparmor
(on Debian it's linked to both, even in the non-stackable present,
and the right one for the kernel configuration is chosen at runtime).
If one of those libraries wrote to /proc/self/attr/display, then the rest
of dbus-daemon's main thread and all child processes would implicitly be
getting the result of that - even if dbus-daemon itself had not yet been
updated for stacked LSMs (in which case it cannot be expected to reverse
their action between fork and exec, because it's an older codebase that
doesn't yet know that "big" LSMs can be stacked).
So I think libselinux and libapparmor should be enhanced to use
new kernel interfaces that get the label they want to get (either
just that label, or all the labels), instead of being enhanced to
write /proc/self/attr/display to change the meaning of old kernel
interfaces. Otherwise they can break other code in their process or
their subprocesses.
> > instead of repurposing /proc/<pid>/attr/current
> > and SO_PEERSEC to have contents that vary according to ambient process
> > state in their reader?
>
> In addition, yes. Instead of? I don't think that we can have a
> backward compatibility story that flies without it.
Consider only SELinux and AppArmor for a moment (I know there are other
"big" LSMs like Smack, but this same reasoning applies to any pair, with
appropriate search-and-replace on their names).
Neither SELinux nor AppArmor: there are no labels, nothing changed.
AppArmor is the only "big" LSM in the stack (Ubuntu): previously,
the label was the AppArmor label; now, if attr/display is not altered,
the label is the one used by the first "big" LSM in the stack, which is
AppArmor. Nothing changed.
SELinux is the only "big" LSM in the stack (Red Hat): same as for AppArmor
being the only "big" LSM in the stack, but with s/AppArmor/SELinux/.
SELinux and AppArmor stacked: this is a situation that could not exist
before, so distro/sysadmin action must have been necessary to make it
happen. However much ambient process state is invented, I don't see any
way to make both SELinux and AppArmor user-space work without modifications:
at least one of them (the one that is second in the stack) has to use new
kernel interfaces, or alter attr/display to change the meaning of the old
kernel interfaces, or something similar, to get the second LSM's labels.
So distro/sysadmin action in user-space is also going to be necessary here
whatever happens - backward compatibility has already been broken, it's
only a question of how intrusive the user-space changes are. Is it really
so much worse if the distro/sysadmin action taken to update user-space
has to take the form of using new kernel interfaces that always do the
same thing, rather than changing the meaning of old kernel interfaces?
smcv
^ permalink raw reply
* Re: [PATCH 02/10] vfs: syscall: Add move_mount(2) to move mounts around
From: Tetsuo Handa @ 2019-07-22 10:12 UTC (permalink / raw)
To: John Johansen
Cc: Eric W. Biederman, Al Viro, David Howells, linux-api,
linux-fsdevel, torvalds, linux-security-module
In-Reply-To: <5802b8b1-f734-1670-f83b-465eda133936@i-love.sakura.ne.jp>
I did not see AppArmor patch for this problem in 5.3-rc1.
John, are you OK with this patch for 5.2-stable and 5.3 ?
On 2019/07/09 19:51, Tetsuo Handa wrote:
> For now, can we apply this patch for 5.2-stable ?
>
>
>>From dd62fab0592e02580fd5a34222a2d11bfc179f61 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Tue, 9 Jul 2019 19:05:49 +0900
> Subject: [PATCH] LSM: Disable move_mount() syscall when TOMOYO or AppArmor is enabled.
>
> Commit 2db154b3ea8e14b0 ("vfs: syscall: Add move_mount(2) to move mounts
> around") introduced security_move_mount() LSM hook, but we missed that
> TOMOYO and AppArmor did not implement hooks for checking move_mount(2).
> For pathname based access controls like TOMOYO and AppArmor, unchecked
> mount manipulation is not acceptable. Therefore, until TOMOYO and AppArmor
> implement hooks, in order to avoid unchecked mount manipulation, pretend
> as if move_mount(2) is unavailable when either TOMOYO or AppArmor is
> enabled.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Fixes: 2db154b3ea8e14b0 ("vfs: syscall: Add move_mount(2) to move mounts around")
> Cc: stable@vger.kernel.org # 5.2
> ---
> include/linux/lsm_hooks.h | 6 ++++++
> security/apparmor/lsm.c | 1 +
> security/tomoyo/tomoyo.c | 1 +
> 3 files changed, 8 insertions(+)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 47f58cf..cd411b7 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2142,4 +2142,10 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
>
> extern int lsm_inode_alloc(struct inode *inode);
>
> +static inline int no_move_mount(const struct path *from_path,
> + const struct path *to_path)
> +{
> + return -ENOSYS;
> +}
> +
> #endif /* ! __LINUX_LSM_HOOKS_H */
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index ec3a928..5cdf63b 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1158,6 +1158,7 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
> LSM_HOOK_INIT(capable, apparmor_capable),
>
> LSM_HOOK_INIT(sb_mount, apparmor_sb_mount),
> + LSM_HOOK_INIT(move_mount, no_move_mount),
> LSM_HOOK_INIT(sb_umount, apparmor_sb_umount),
> LSM_HOOK_INIT(sb_pivotroot, apparmor_sb_pivotroot),
>
> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
> index 716c92e..be1b1a1 100644
> --- a/security/tomoyo/tomoyo.c
> +++ b/security/tomoyo/tomoyo.c
> @@ -558,6 +558,7 @@ static void tomoyo_task_free(struct task_struct *task)
> LSM_HOOK_INIT(path_chown, tomoyo_path_chown),
> LSM_HOOK_INIT(path_chroot, tomoyo_path_chroot),
> LSM_HOOK_INIT(sb_mount, tomoyo_sb_mount),
> + LSM_HOOK_INIT(move_mount, no_move_mount),
> LSM_HOOK_INIT(sb_umount, tomoyo_sb_umount),
> LSM_HOOK_INIT(sb_pivotroot, tomoyo_sb_pivotroot),
> LSM_HOOK_INIT(socket_bind, tomoyo_socket_bind),
>
^ permalink raw reply
* [PATCH bpf-next v10 06/10] bpf,landlock: Add a new map type: inode
From: Mickaël Salaün @ 2019-07-21 21:31 UTC (permalink / raw)
To: linux-kernel
Cc: Mickaël Salaün, Alexander Viro, Alexei Starovoitov,
Andrew Morton, Andy Lutomirski, Arnaldo Carvalho de Melo,
Casey Schaufler, Daniel Borkmann, David Drysdale,
David S . Miller, Eric W . Biederman, James Morris, Jann Horn,
John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
Mickaël Salaün, Paul Moore, Sargun Dhillon,
Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
Tetsuo Handa, Thomas Graf, Tycho Andersen, Will Drewry,
kernel-hardening, linux-api, linux-fsdevel, linux-security-module,
netdev
In-Reply-To: <20190721213116.23476-1-mic@digikod.net>
FIXME: 64-bits in the doc
This new map store arbitrary values referenced by inode keys. The map
can be updated from user space with file descriptor pointing to inodes
tied to a file system. From an eBPF (Landlock) program point of view,
such a map is read-only and can only be used to retrieved a value tied
to a given inode. This is useful to recognize an inode tagged by user
space, without access right to this inode (i.e. no need to have a write
access to this inode).
Add dedicated BPF functions to handle this type of map:
* bpf_inode_htab_map_update_elem()
* bpf_inode_htab_map_lookup_elem()
* bpf_inode_htab_map_delete_elem()
This new map require a dedicated helper inode_map_lookup_elem() because
of the key which is a pointer to an opaque data (only provided by the
kernel). This act like a (physical or cryptographic) key, which is why
it is also not allowed to get the next key.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Jann Horn <jann@thejh.net>
---
Changes since v9:
* use a hash map for the inode map: integrate inodemap.c into hashtab.c
* add map_put_key() to struct bpf_map_ops to enable to put an inode
reference used as key
* allow arbitrary value size instead of 64-bits
* handle inode and map lifetime with LSM hooks
* check access for inode lookup via syscall: similar to adding xattr,
except it does not touch the file system (which is handy for read-only
ones)
* force read-only inode map for Landlock programs
* rename inode_map_lookup() into inode_map_lookup_elem()
* fix inode and mnt checks (suggested by Al Viro)
Changes since v8:
* remove prog chaining and object tagging to ease review
* use bpf_map_init_from_attr()
Changes since v7:
* new design with a dedicated map and a BPF function to tie a value to
an inode
* add the ability to set or get a tag on an inode from a Landlock
program
Changes since v6:
* remove WARN_ON() for missing dentry->d_inode
* refactor bpf_landlock_func_proto() (suggested by Kees Cook)
Changes since v5:
* cosmetic fixes and rebase
Changes since v4:
* use a file abstraction (handle) to wrap inode, dentry, path and file
structs
* remove bpf_landlock_cmp_fs_beneath()
* rename the BPF helper and move it to kernel/bpf/
* tighten helpers accessible by a Landlock rule
Changes since v3:
* remove bpf_landlock_cmp_fs_prop() (suggested by Alexei Starovoitov)
* add hooks dealing with struct inode and struct path pointers:
inode_permission and inode_getattr
* add abstraction over eBPF helper arguments thanks to wrapping structs
* add bpf_landlock_get_fs_mode() helper to check file type and mode
* merge WARN_ON() (suggested by Kees Cook)
* fix and update bpf_helpers.h
* use BPF_CALL_* for eBPF helpers (suggested by Alexei Starovoitov)
* make handle arraymap safe (RCU) and remove buggy synchronize_rcu()
* factor out the arraymay walk
* use size_t to index array (suggested by Jann Horn)
Changes since v2:
* add MNT_INTERNAL check to only add file handle from user-visible FS
(e.g. no anonymous inode)
* replace struct file* with struct path* in map_landlock_handle
* add BPF protos
* fix bpf_landlock_cmp_fs_prop_with_struct_file()
---
include/linux/bpf.h | 16 +++
include/linux/bpf_types.h | 3 +
include/linux/landlock.h | 4 +
include/uapi/linux/bpf.h | 12 +-
kernel/bpf/core.c | 2 +
kernel/bpf/hashtab.c | 253 +++++++++++++++++++++++++++++++++
kernel/bpf/syscall.c | 27 +++-
kernel/bpf/verifier.c | 14 ++
security/landlock/common.h | 14 ++
security/landlock/hooks_fs.c | 85 +++++++++++
security/landlock/init.c | 13 ++
tools/include/uapi/linux/bpf.h | 12 +-
tools/lib/bpf/libbpf_probes.c | 1 +
13 files changed, 453 insertions(+), 3 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 6d9c7a08713e..c507438e56b5 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -47,6 +47,7 @@ struct bpf_map_ops {
void *(*map_fd_get_ptr)(struct bpf_map *map, struct file *map_file,
int fd);
void (*map_fd_put_ptr)(void *ptr);
+ void (*map_put_key)(void *key);
u32 (*map_gen_lookup)(struct bpf_map *map, struct bpf_insn *insn_buf);
u32 (*map_fd_sys_lookup_elem)(void *ptr);
void (*map_seq_show_elem)(struct bpf_map *map, void *key,
@@ -208,6 +209,8 @@ enum bpf_arg_type {
ARG_PTR_TO_INT, /* pointer to int */
ARG_PTR_TO_LONG, /* pointer to long */
ARG_PTR_TO_SOCKET, /* pointer to bpf_sock (fullsock) */
+
+ ARG_PTR_TO_INODE, /* pointer to a struct inode */
};
/* type of values returned from helper functions */
@@ -278,6 +281,7 @@ enum bpf_reg_type {
PTR_TO_TCP_SOCK_OR_NULL, /* reg points to struct tcp_sock or NULL */
PTR_TO_TP_BUFFER, /* reg points to a writable raw tp's buffer */
PTR_TO_XDP_SOCK, /* reg points to struct xdp_sock */
+ PTR_TO_INODE, /* reg points to struct inode */
};
/* The information passed from prog-specific *_is_valid_access
@@ -479,6 +483,7 @@ struct bpf_event_entry {
struct rcu_head rcu;
};
+
bool bpf_prog_array_compatible(struct bpf_array *array, const struct bpf_prog *fp);
int bpf_prog_calc_tag(struct bpf_prog *fp);
@@ -684,6 +689,16 @@ int bpf_fd_array_map_lookup_elem(struct bpf_map *map, void *key, u32 *value);
int bpf_fd_htab_map_update_elem(struct bpf_map *map, struct file *map_file,
void *key, void *value, u64 map_flags);
int bpf_fd_htab_map_lookup_elem(struct bpf_map *map, void *key, u32 *value);
+int bpf_inode_fd_htab_map_lookup_elem(struct bpf_map *map, int *key, void *value);
+int bpf_inode_fd_htab_map_delete_elem(struct bpf_map *map, int *key);
+int bpf_inode_ptr_unlocked_htab_map_delete_elem(struct bpf_map *map,
+ struct inode **key,
+ bool remove_in_inode);
+int bpf_inode_ptr_locked_htab_map_delete_elem(struct bpf_map *map,
+ struct inode **key,
+ bool remove_in_inode);
+int bpf_inode_fd_htab_map_update_elem(struct bpf_map *map, int *key,
+ void *value, u64 map_flags);
int bpf_get_file_flag(int flags);
int bpf_check_uarg_tail_zero(void __user *uaddr, size_t expected_size,
@@ -1055,6 +1070,7 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto;
extern const struct bpf_func_proto bpf_strtol_proto;
extern const struct bpf_func_proto bpf_strtoul_proto;
extern const struct bpf_func_proto bpf_tcp_sock_proto;
+extern const struct bpf_func_proto bpf_inode_map_lookup_elem_proto;
/* Shared helpers among cBPF and eBPF. */
void bpf_user_rnd_init_once(void);
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 2ab647323f3a..ea177818d67e 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -80,3 +80,6 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_REUSEPORT_SOCKARRAY, reuseport_array_ops)
#endif
BPF_MAP_TYPE(BPF_MAP_TYPE_QUEUE, queue_map_ops)
BPF_MAP_TYPE(BPF_MAP_TYPE_STACK, stack_map_ops)
+#ifdef CONFIG_SECURITY_LANDLOCK
+BPF_MAP_TYPE(BPF_MAP_TYPE_INODE, htab_inode_ops)
+#endif
diff --git a/include/linux/landlock.h b/include/linux/landlock.h
index 8ac7942f50fc..731b89cdf977 100644
--- a/include/linux/landlock.h
+++ b/include/linux/landlock.h
@@ -9,6 +9,7 @@
#ifndef _LINUX_LANDLOCK_H
#define _LINUX_LANDLOCK_H
+#include <linux/bpf.h>
#include <linux/errno.h>
#include <linux/sched.h> /* task_struct */
@@ -31,4 +32,7 @@ static inline void get_seccomp_landlock(struct task_struct *tsk)
}
#endif /* CONFIG_SECCOMP_FILTER && CONFIG_SECURITY_LANDLOCK */
+int landlock_inode_add_map(struct inode *inode, struct bpf_map *map);
+void landlock_inode_remove_map(struct inode *inode, const struct bpf_map *map);
+
#endif /* _LINUX_LANDLOCK_H */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index d68613f737f3..2da054ca9c8b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -134,6 +134,7 @@ enum bpf_map_type {
BPF_MAP_TYPE_QUEUE,
BPF_MAP_TYPE_STACK,
BPF_MAP_TYPE_SK_STORAGE,
+ BPF_MAP_TYPE_INODE,
};
/* Note that tracing related programs such as
@@ -2717,6 +2718,14 @@ union bpf_attr {
* **-EPERM** if no permission to send the *sig*.
*
* **-EAGAIN** if bpf program can try again.
+ *
+ * void *bpf_inode_map_lookup_elem(struct bpf_map *map, const void *key)
+ * Description
+ * Perform a lookup in *map* for an entry associated to an inode
+ * *key*.
+ * Return
+ * Map value associated to *key*, or **NULL** if no entry was
+ * found.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -2828,7 +2837,8 @@ union bpf_attr {
FN(strtoul), \
FN(sk_storage_get), \
FN(sk_storage_delete), \
- FN(send_signal),
+ FN(send_signal), \
+ FN(inode_map_lookup_elem),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 16079550db6d..4177c818e5cd 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2040,6 +2040,8 @@ const struct bpf_func_proto bpf_get_current_comm_proto __weak;
const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
const struct bpf_func_proto bpf_get_local_storage_proto __weak;
+const struct bpf_func_proto bpf_inode_map_update_proto __weak;
+
const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
{
return NULL;
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 22066a62c8c9..4fc7755042f0 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -1,13 +1,21 @@
// SPDX-License-Identifier: GPL-2.0-only
/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
* Copyright (c) 2016 Facebook
+ * Copyright (c) 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright (c) 2019 ANSSI
*/
+#include <asm/resource.h> /* RLIMIT_NOFILE */
#include <linux/bpf.h>
#include <linux/btf.h>
+#include <linux/err.h>
#include <linux/jhash.h>
+#include <linux/fs.h> /* iput() */
#include <linux/filter.h>
+#include <linux/landlock.h>
+#include <linux/mount.h> /* MNT_INTERNAL */
#include <linux/rculist_nulls.h>
#include <linux/random.h>
+#include <linux/sched/signal.h> /* rlimit() */
#include <uapi/linux/btf.h>
#include "percpu_freelist.h"
#include "bpf_lru_list.h"
@@ -684,6 +692,8 @@ static void free_htab_elem(struct bpf_htab *htab, struct htab_elem *l)
map->ops->map_fd_put_ptr(ptr);
}
+ if (map->ops->map_put_key)
+ map->ops->map_put_key(l->key);
if (htab_is_prealloc(htab)) {
__pcpu_freelist_push(&htab->freelist, &l->fnode);
@@ -1514,3 +1524,246 @@ const struct bpf_map_ops htab_of_maps_map_ops = {
.map_gen_lookup = htab_of_map_gen_lookup,
.map_check_btf = map_check_no_btf,
};
+
+/* inode_htab */
+
+static int inode_htab_map_alloc_check(union bpf_attr *attr)
+{
+ /* only allow root to create this type of map (for now), should be
+ * removed when Landlock will be usable by unprivileged users */
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ /* the key is a file descriptor */
+ if (attr->max_entries == 0 || attr->key_size != sizeof(int) ||
+ (attr->map_flags & ~(BPF_F_RDONLY | BPF_F_WRONLY |
+ BPF_F_RDONLY_PROG)) ||
+ /* for now, force read-only map for eBPF programs because only
+ * bpf_inode_map_lookup_elem() enable to access them */
+ !(attr->map_flags & BPF_F_RDONLY_PROG) ||
+ bpf_map_attr_numa_node(attr) != NUMA_NO_NODE)
+ return -EINVAL;
+
+ /*
+ * Limit number of entries in an inode map to the maximum number of
+ * open files for the current process. The maximum number of file
+ * references (including all inode maps) for a process is then
+ * (RLIMIT_NOFILE - 1) * RLIMIT_NOFILE. If the process' RLIMIT_NOFILE
+ * is 0, then any entry update is forbidden.
+ *
+ * An eBPF program can inherit all the inode map FD. The worse case is
+ * to fill a bunch of arraymaps, create an eBPF program, close the
+ * inode map FDs, and start again. The maximum number of inode map
+ * entries can then be close to RLIMIT_NOFILE^3.
+ */
+ if (attr->max_entries > rlimit(RLIMIT_NOFILE))
+ return -EMFILE;
+
+ /* decorelate UAPI from kernel API */
+ attr->key_size = sizeof(struct inode *);
+
+ return htab_map_alloc_check(attr);
+}
+
+static void inode_htab_put_key(void *key)
+{
+ struct inode **inode = key;
+
+ if ((*inode)->i_state & I_FREEING)
+ return;
+ iput(*inode);
+}
+
+/* called from syscall or (never) from eBPF program */
+static int map_get_next_no_key(struct bpf_map *map, void *key, void *next_key)
+{
+ /* do not leak a file descriptor */
+ return -ENOTSUPP;
+}
+
+/* must call iput(inode) after this call */
+static struct inode *inode_from_fd(int ufd, bool check_access)
+{
+ struct inode *ret;
+ struct fd f;
+ int deny;
+
+ f = fdget(ufd);
+ if (unlikely(!f.file))
+ return ERR_PTR(-EBADF);
+ /* TODO?: add this check when called from an eBPF program too (already
+ * checked by the LSM parent hooks anyway) */
+ if (unlikely(IS_PRIVATE(file_inode(f.file)))) {
+ ret = ERR_PTR(-EINVAL);
+ goto put_fd;
+ }
+ /* check if the FD is tied to a mount point */
+ /* TODO?: add this check when called from an eBPF program too */
+ if (unlikely(f.file->f_path.mnt->mnt_flags & MNT_INTERNAL)) {
+ ret = ERR_PTR(-EINVAL);
+ goto put_fd;
+ }
+ if (check_access) {
+ /*
+ * must be allowed to access attributes from this file to then
+ * be able to compare an inode to its map entry
+ */
+ deny = security_inode_getattr(&f.file->f_path);
+ if (deny) {
+ ret = ERR_PTR(deny);
+ goto put_fd;
+ }
+ }
+ ret = file_inode(f.file);
+ ihold(ret);
+
+put_fd:
+ fdput(f);
+ return ret;
+}
+
+/*
+ * The key is a FD when called from a syscall, but an inode address when called
+ * from an eBPF program.
+ */
+
+/* called from syscall */
+int bpf_inode_fd_htab_map_lookup_elem(struct bpf_map *map, int *key, void *value)
+{
+ void *ptr;
+ struct inode *inode;
+ int ret;
+
+ /* check inode access */
+ inode = inode_from_fd(*key, true);
+ if (IS_ERR(inode))
+ return PTR_ERR(inode);
+
+ rcu_read_lock();
+ ptr = htab_map_lookup_elem(map, &inode);
+ iput(inode);
+ if (IS_ERR(ptr)) {
+ ret = PTR_ERR(ptr);
+ } else if (!ptr) {
+ ret = -ENOENT;
+ } else {
+ ret = 0;
+ copy_map_value(map, value, ptr);
+ }
+ rcu_read_unlock();
+ return ret;
+}
+
+/* called from kernel */
+int bpf_inode_ptr_locked_htab_map_delete_elem(struct bpf_map *map,
+ struct inode **key, bool remove_in_inode)
+{
+ if (remove_in_inode)
+ landlock_inode_remove_map(*key, map);
+ return htab_map_delete_elem(map, key);
+}
+
+/* called from syscall */
+int bpf_inode_fd_htab_map_delete_elem(struct bpf_map *map, int *key)
+{
+ struct inode *inode;
+ int ret;
+
+ /* do not check inode access (similar to directory check) */
+ inode = inode_from_fd(*key, false);
+ if (IS_ERR(inode))
+ return PTR_ERR(inode);
+ ret = bpf_inode_ptr_locked_htab_map_delete_elem(map, &inode, true);
+ iput(inode);
+ return ret;
+}
+
+/* called from syscall */
+int bpf_inode_fd_htab_map_update_elem(struct bpf_map *map, int *key, void *value,
+ u64 map_flags)
+{
+ struct inode *inode;
+ int ret;
+
+ WARN_ON_ONCE(!rcu_read_lock_held());
+
+ /* check inode access */
+ inode = inode_from_fd(*key, true);
+ if (IS_ERR(inode))
+ return PTR_ERR(inode);
+ ret = htab_map_update_elem(map, &inode, value, map_flags);
+ if (!ret)
+ ret = landlock_inode_add_map(inode, map);
+ iput(inode);
+ return ret;
+}
+
+static void inode_htab_map_free(struct bpf_map *map)
+{
+ struct bpf_htab *htab = container_of(map, struct bpf_htab, map);
+ struct hlist_nulls_node *n;
+ struct hlist_nulls_head *head;
+ struct htab_elem *l;
+ int i;
+
+ for (i = 0; i < htab->n_buckets; i++) {
+ head = select_bucket(htab, i);
+ hlist_nulls_for_each_entry_safe(l, n, head, hash_node) {
+ landlock_inode_remove_map(*((struct inode **)l->key), map);
+ }
+ }
+ htab_map_free(map);
+}
+
+/* use the map_inode_lookup_elem() helper instead */
+static void *map_lookup_no_elem(struct bpf_map *map, void *key)
+{
+ WARN_ON_ONCE(1);
+ return NULL;
+}
+
+static int map_delete_no_elem(struct bpf_map *map, void *key)
+{
+ WARN_ON_ONCE(1);
+ return -ENOTSUPP;
+}
+
+static int map_update_no_elem(struct bpf_map *map, void *key, void *value,
+ u64 flags)
+{
+ WARN_ON_ONCE(1);
+ return -ENOTSUPP;
+}
+
+const struct bpf_map_ops htab_inode_ops = {
+ .map_alloc_check = inode_htab_map_alloc_check,
+ .map_alloc = htab_map_alloc,
+ .map_free = inode_htab_map_free,
+ .map_put_key = inode_htab_put_key,
+ .map_get_next_key = map_get_next_no_key,
+ .map_lookup_elem = map_lookup_no_elem,
+ .map_delete_elem = map_delete_no_elem,
+ .map_update_elem = map_update_no_elem,
+ .map_check_btf = map_check_no_btf,
+};
+
+/*
+ * We need a dedicated helper to deal with inode maps because the key is a
+ * pointer to an opaque data, only provided by the kernel. This really act
+ * like a (physical or cryptographic) key, which is why it is also not allowed
+ * to get the next key with map_get_next_key().
+ */
+BPF_CALL_2(bpf_inode_map_lookup_elem, struct bpf_map *, map, void *, key)
+{
+ WARN_ON_ONCE(!rcu_read_lock_held());
+ return (unsigned long)htab_map_lookup_elem(map, &key);
+}
+
+const struct bpf_func_proto bpf_inode_map_lookup_elem_proto = {
+ .func = bpf_inode_map_lookup_elem,
+ .gpl_only = false,
+ .pkt_access = true,
+ .ret_type = RET_PTR_TO_MAP_VALUE_OR_NULL,
+ .arg1_type = ARG_CONST_MAP_PTR,
+ .arg2_type = ARG_PTR_TO_INODE,
+};
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b2a8cb14f28e..e46441c42b68 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -801,6 +801,8 @@ static int map_lookup_elem(union bpf_attr *attr)
} else if (map->map_type == BPF_MAP_TYPE_QUEUE ||
map->map_type == BPF_MAP_TYPE_STACK) {
err = map->ops->map_peek_elem(map, value);
+ } else if (map->map_type == BPF_MAP_TYPE_INODE) {
+ err = bpf_inode_fd_htab_map_lookup_elem(map, key, value);
} else {
rcu_read_lock();
if (map->ops->map_lookup_elem_sys_only)
@@ -951,6 +953,10 @@ static int map_update_elem(union bpf_attr *attr)
} else if (map->map_type == BPF_MAP_TYPE_QUEUE ||
map->map_type == BPF_MAP_TYPE_STACK) {
err = map->ops->map_push_elem(map, value, attr->flags);
+ } else if (map->map_type == BPF_MAP_TYPE_INODE) {
+ rcu_read_lock();
+ err = bpf_inode_fd_htab_map_update_elem(map, key, value, attr->flags);
+ rcu_read_unlock();
} else {
rcu_read_lock();
err = map->ops->map_update_elem(map, key, value, attr->flags);
@@ -1006,7 +1012,10 @@ static int map_delete_elem(union bpf_attr *attr)
preempt_disable();
__this_cpu_inc(bpf_prog_active);
rcu_read_lock();
- err = map->ops->map_delete_elem(map, key);
+ if (map->map_type == BPF_MAP_TYPE_INODE)
+ err = bpf_inode_fd_htab_map_delete_elem(map, key);
+ else
+ err = map->ops->map_delete_elem(map, key);
rcu_read_unlock();
__this_cpu_dec(bpf_prog_active);
preempt_enable();
@@ -1018,6 +1027,22 @@ static int map_delete_elem(union bpf_attr *attr)
return err;
}
+int bpf_inode_ptr_unlocked_htab_map_delete_elem(struct bpf_map *map,
+ struct inode **key, bool remove_in_inode)
+{
+ int err;
+
+ preempt_disable();
+ __this_cpu_inc(bpf_prog_active);
+ rcu_read_lock();
+ err = bpf_inode_ptr_locked_htab_map_delete_elem(map, key, remove_in_inode);
+ rcu_read_unlock();
+ __this_cpu_dec(bpf_prog_active);
+ preempt_enable();
+ maybe_wait_bpf_programs(map);
+ return err;
+}
+
/* last field in 'union bpf_attr' used by this command */
#define BPF_MAP_GET_NEXT_KEY_LAST_FIELD next_key
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 026c68cb9116..3972b9f02dac 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -400,6 +400,7 @@ static const char * const reg_type_str[] = {
[PTR_TO_TCP_SOCK_OR_NULL] = "tcp_sock_or_null",
[PTR_TO_TP_BUFFER] = "tp_buffer",
[PTR_TO_XDP_SOCK] = "xdp_sock",
+ [PTR_TO_INODE] = "inode",
};
static char slot_type_char[] = {
@@ -1846,6 +1847,7 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
case PTR_TO_TCP_SOCK:
case PTR_TO_TCP_SOCK_OR_NULL:
case PTR_TO_XDP_SOCK:
+ case PTR_TO_INODE:
return true;
default:
return false;
@@ -3306,6 +3308,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
verbose(env, "verifier internal error\n");
return -EFAULT;
}
+ } else if (arg_type == ARG_PTR_TO_INODE) {
+ expected_type = PTR_TO_INODE;
+ if (type != expected_type)
+ goto err_type;
} else if (arg_type_is_mem_ptr(arg_type)) {
expected_type = PTR_TO_STACK;
/* One exception here. In case function allows for NULL to be
@@ -3511,6 +3517,10 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
func_id != BPF_FUNC_sk_storage_delete)
goto error;
break;
+ case BPF_MAP_TYPE_INODE:
+ if (func_id != BPF_FUNC_inode_map_lookup_elem)
+ goto error;
+ break;
default:
break;
}
@@ -3579,6 +3589,10 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
if (map->map_type != BPF_MAP_TYPE_SK_STORAGE)
goto error;
break;
+ case BPF_FUNC_inode_map_lookup_elem:
+ if (map->map_type != BPF_MAP_TYPE_INODE)
+ goto error;
+ break;
default:
break;
}
diff --git a/security/landlock/common.h b/security/landlock/common.h
index b2ee018eb6fc..b0ba3f31ac7d 100644
--- a/security/landlock/common.h
+++ b/security/landlock/common.h
@@ -11,6 +11,7 @@
#include <linux/bpf.h> /* enum bpf_attach_type */
#include <linux/filter.h> /* bpf_prog */
+#include <linux/lsm_hooks.h> /* lsm_blob_sizes */
#include <linux/refcount.h> /* refcount_t */
#include <uapi/linux/landlock.h> /* LANDLOCK_TRIGGER_* */
@@ -23,6 +24,8 @@
#define _LANDLOCK_TRIGGER_FS_PICK_LAST LANDLOCK_TRIGGER_FS_PICK_WRITE
#define _LANDLOCK_TRIGGER_FS_PICK_MASK ((_LANDLOCK_TRIGGER_FS_PICK_LAST << 1ULL) - 1)
+extern struct lsm_blob_sizes landlock_blob_sizes;
+
enum landlock_hook_type {
LANDLOCK_HOOK_FS_PICK = 1,
LANDLOCK_HOOK_FS_WALK,
@@ -55,6 +58,17 @@ struct landlock_prog_set {
refcount_t usage;
};
+struct landlock_inode_map {
+ struct list_head list;
+ struct rcu_head rcu_put;
+ struct bpf_map *map;
+ /*
+ * It would be nice to remove the inode field, but it is necessary for
+ * call_rcu() .
+ */
+ struct inode *inode;
+};
+
/**
* get_hook_index - get an index for the programs of struct landlock_prog_set
*
diff --git a/security/landlock/hooks_fs.c b/security/landlock/hooks_fs.c
index 3f81b7fc2938..8c9d6a333111 100644
--- a/security/landlock/hooks_fs.c
+++ b/security/landlock/hooks_fs.c
@@ -46,6 +46,12 @@ bool landlock_is_valid_access_fs_pick(int off, enum bpf_access_type type,
enum bpf_reg_type *reg_type, int *max_size)
{
switch (off) {
+ case offsetof(struct landlock_ctx_fs_pick, inode):
+ if (type != BPF_READ)
+ return false;
+ *reg_type = PTR_TO_INODE;
+ *max_size = sizeof(u64);
+ return true;
default:
return false;
}
@@ -55,6 +61,12 @@ bool landlock_is_valid_access_fs_walk(int off, enum bpf_access_type type,
enum bpf_reg_type *reg_type, int *max_size)
{
switch (off) {
+ case offsetof(struct landlock_ctx_fs_walk, inode):
+ if (type != BPF_READ)
+ return false;
+ *reg_type = PTR_TO_INODE;
+ *max_size = sizeof(u64);
+ return true;
default:
return false;
}
@@ -237,8 +249,79 @@ static int hook_sb_pivotroot(const struct path *old_path,
new_path->dentry->d_inode);
}
+/* inode helpers */
+
+static inline struct list_head *inode_landlock(const struct inode *inode)
+{
+ return inode->i_security + landlock_blob_sizes.lbs_inode;
+}
+
+int landlock_inode_add_map(struct inode *inode, struct bpf_map *map)
+{
+ struct landlock_inode_map *inode_map;
+
+ inode_map = kzalloc(sizeof(*inode_map), GFP_ATOMIC);
+ if (!inode_map)
+ return -ENOMEM;
+ INIT_LIST_HEAD(&inode_map->list);
+ inode_map->map = map;
+ inode_map->inode = inode;
+ list_add_tail(&inode_map->list, inode_landlock(inode));
+ return 0;
+}
+
+static void put_landlock_inode_map(struct rcu_head *head)
+{
+ struct landlock_inode_map *inode_map;
+ int err;
+
+ inode_map = container_of(head, struct landlock_inode_map, rcu_put);
+ err = bpf_inode_ptr_unlocked_htab_map_delete_elem(inode_map->map,
+ &inode_map->inode, false);
+ bpf_map_put(inode_map->map);
+ kfree(inode_map);
+}
+
+void landlock_inode_remove_map(struct inode *inode, const struct bpf_map *map)
+{
+ struct landlock_inode_map *inode_map;
+ bool found = false;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(inode_map, inode_landlock(inode), list) {
+ if (inode_map->map == map) {
+ found = true;
+ list_del_rcu(&inode_map->list);
+ kfree_rcu(inode_map, rcu_put);
+ break;
+ }
+ }
+ rcu_read_unlock();
+ WARN_ON(!found);
+}
+
/* inode hooks */
+static int hook_inode_alloc_security(struct inode *inode)
+{
+ struct list_head *ll_inode = inode_landlock(inode);
+
+ INIT_LIST_HEAD(ll_inode);
+ return 0;
+}
+
+static void hook_inode_free_security(struct inode *inode)
+{
+ struct landlock_inode_map *inode_map;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(inode_map, inode_landlock(inode), list) {
+ list_del_rcu(&inode_map->list);
+ call_rcu(&inode_map->rcu_put, put_landlock_inode_map);
+ }
+ rcu_read_unlock();
+}
+
/* a directory inode contains only one dentry */
static int hook_inode_create(struct inode *dir, struct dentry *dentry,
umode_t mode)
@@ -517,6 +600,8 @@ static struct security_hook_list landlock_hooks[] = {
LSM_HOOK_INIT(sb_mount, hook_sb_mount),
LSM_HOOK_INIT(sb_pivotroot, hook_sb_pivotroot),
+ LSM_HOOK_INIT(inode_alloc_security, hook_inode_alloc_security),
+ LSM_HOOK_INIT(inode_free_security, hook_inode_free_security),
LSM_HOOK_INIT(inode_create, hook_inode_create),
LSM_HOOK_INIT(inode_link, hook_inode_link),
LSM_HOOK_INIT(inode_unlink, hook_inode_unlink),
diff --git a/security/landlock/init.c b/security/landlock/init.c
index 391e88bd4d3a..eec4467cb5ee 100644
--- a/security/landlock/init.c
+++ b/security/landlock/init.c
@@ -104,6 +104,18 @@ static const struct bpf_func_proto *bpf_landlock_func_proto(
default:
break;
}
+
+ switch (get_hook_type(prog)) {
+ case LANDLOCK_HOOK_FS_WALK:
+ case LANDLOCK_HOOK_FS_PICK:
+ switch (func_id) {
+ case BPF_FUNC_inode_map_lookup_elem:
+ return &bpf_inode_map_lookup_elem_proto;
+ default:
+ break;
+ }
+ break;
+ }
return NULL;
}
@@ -123,6 +135,7 @@ static int __init landlock_init(void)
}
struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = {
+ .lbs_inode = sizeof(struct list_head),
};
DEFINE_LSM(LANDLOCK_NAME) = {
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 7b7a4f6c3104..7a55535f5dc1 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -134,6 +134,7 @@ enum bpf_map_type {
BPF_MAP_TYPE_QUEUE,
BPF_MAP_TYPE_STACK,
BPF_MAP_TYPE_SK_STORAGE,
+ BPF_MAP_TYPE_INODE,
};
/* Note that tracing related programs such as
@@ -2714,6 +2715,14 @@ union bpf_attr {
* **-EPERM** if no permission to send the *sig*.
*
* **-EAGAIN** if bpf program can try again.
+ *
+ * void *bpf_inode_map_lookup_elem(struct bpf_map *map, const void *key)
+ * Description
+ * Perform a lookup in *map* for an entry associated to an inode
+ * *key*.
+ * Return
+ * Map value associated to *key*, or **NULL** if no entry was
+ * found.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -2825,7 +2834,8 @@ union bpf_attr {
FN(strtoul), \
FN(sk_storage_get), \
FN(sk_storage_delete), \
- FN(send_signal),
+ FN(send_signal), \
+ FN(inode_map_lookup_elem),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
index 03c910d1f84c..98875221310d 100644
--- a/tools/lib/bpf/libbpf_probes.c
+++ b/tools/lib/bpf/libbpf_probes.c
@@ -250,6 +250,7 @@ bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)
case BPF_MAP_TYPE_XSKMAP:
case BPF_MAP_TYPE_SOCKHASH:
case BPF_MAP_TYPE_REUSEPORT_SOCKARRAY:
+ case BPF_MAP_TYPE_INODE:
default:
break;
}
--
2.22.0
^ permalink raw reply related
* [PATCH bpf-next v10 05/10] landlock: Handle filesystem access control
From: Mickaël Salaün @ 2019-07-21 21:31 UTC (permalink / raw)
To: linux-kernel
Cc: Mickaël Salaün, Alexander Viro, Alexei Starovoitov,
Andrew Morton, Andy Lutomirski, Arnaldo Carvalho de Melo,
Casey Schaufler, Daniel Borkmann, David Drysdale,
David S . Miller, Eric W . Biederman, James Morris, Jann Horn,
John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
Mickaël Salaün, Paul Moore, Sargun Dhillon,
Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
Tetsuo Handa, Thomas Graf, Tycho Andersen, Will Drewry,
kernel-hardening, linux-api, linux-fsdevel, linux-security-module,
netdev
In-Reply-To: <20190721213116.23476-1-mic@digikod.net>
This add two Landlock hooks: FS_WALK and FS_PICK.
The FS_WALK hook is used to walk through a file path. A program tied to
this hook will be evaluated for each directory traversal except the last
one if it is the leaf of the path. It is important to differentiate
this hook from FS_PICK to enable more powerful path evaluation in the
future (cf. Landlock patch v8).
The FS_PICK hook is used to validate a set of actions requested on a
file. This actions are defined with triggers (e.g. read, write, open,
append...).
The Landlock LSM hook registration is done after other LSM to only run
actions from user-space, via eBPF programs, if the access was granted by
major (privileged) LSMs.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
---
Changes since v9:
* replace subtype with expected_attach_type and expected_attach_triggers
Changes since v8:
* add a new LSM_ORDER_LAST, cf. commit e2bc445b66ca ("LSM: Introduce
enum lsm_order")
* add WARN_ON() for pointer dereferencement
* remove the FS_GET subtype which rely on program chaining
* remove the subtype option which was only used for chaining (with the
"previous" field)
* remove inode_lookup which depends on the (removed) nameidata security
blob
* remove eBPF helpers to get and set Landlock inode tags
* do not use task LSM credentials (for now)
Changes since v7:
* major rewrite with clean Landlock hooks able to deal with file paths
Changes since v6:
* add 3 more sub-events: IOCTL, LOCK, FCNTL
https://lkml.kernel.org/r/2fbc99a6-f190-f335-bd14-04bdeed35571@digikod.net
* use the new security_add_hooks()
* explain the -Werror=unused-function
* constify pointers
* cleanup headers
Changes since v5:
* split hooks.[ch] into hooks.[ch] and hooks_fs.[ch]
* add more documentation
* cosmetic fixes
* rebase (SCALAR_VALUE)
Changes since v4:
* add LSM hook abstraction called Landlock event
* use the compiler type checking to verify hooks use by an event
* handle all filesystem related LSM hooks (e.g. file_permission,
mmap_file, sb_mount...)
* register BPF programs for Landlock just after LSM hooks registration
* move hooks registration after other LSMs
* add failsafes to check if a hook is not used by the kernel
* allow partial raw value access form the context (needed for programs
generated by LLVM)
Changes since v3:
* split commit
* add hooks dealing with struct inode and struct path pointers:
inode_permission and inode_getattr
* add abstraction over eBPF helper arguments thanks to wrapping structs
---
include/linux/lsm_hooks.h | 1 +
security/landlock/Makefile | 3 +-
security/landlock/common.h | 9 +
security/landlock/hooks.c | 94 ++++++
security/landlock/hooks.h | 31 ++
security/landlock/hooks_fs.c | 554 +++++++++++++++++++++++++++++++++++
security/landlock/hooks_fs.h | 31 ++
security/landlock/init.c | 33 +++
security/security.c | 15 +
9 files changed, 770 insertions(+), 1 deletion(-)
create mode 100644 security/landlock/hooks.c
create mode 100644 security/landlock/hooks.h
create mode 100644 security/landlock/hooks_fs.c
create mode 100644 security/landlock/hooks_fs.h
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index df1318d85f7d..c06ad8a1d424 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2092,6 +2092,7 @@ extern void security_add_hooks(struct security_hook_list *hooks, int count,
enum lsm_order {
LSM_ORDER_FIRST = -1, /* This is only for capabilities. */
LSM_ORDER_MUTABLE = 0,
+ LSM_ORDER_LAST = 1, /* potentially-unprivileged LSM */
};
struct lsm_info {
diff --git a/security/landlock/Makefile b/security/landlock/Makefile
index 2a1a7082a365..270ece5d93de 100644
--- a/security/landlock/Makefile
+++ b/security/landlock/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
landlock-y := init.o \
- enforce.o enforce_seccomp.o
+ enforce.o enforce_seccomp.o \
+ hooks.o hooks_fs.o
diff --git a/security/landlock/common.h b/security/landlock/common.h
index 2cf36dbf4560..b2ee018eb6fc 100644
--- a/security/landlock/common.h
+++ b/security/landlock/common.h
@@ -79,4 +79,13 @@ static inline enum landlock_hook_type get_hook_type(const struct bpf_prog *prog)
}
}
+__maybe_unused
+static bool current_has_prog_type(enum landlock_hook_type hook_type)
+{
+ struct landlock_prog_set *prog_set;
+
+ prog_set = current->seccomp.landlock_prog_set;
+ return (prog_set && prog_set->programs[get_hook_index(hook_type)]);
+}
+
#endif /* _SECURITY_LANDLOCK_COMMON_H */
diff --git a/security/landlock/hooks.c b/security/landlock/hooks.c
new file mode 100644
index 000000000000..97c54957f17b
--- /dev/null
+++ b/security/landlock/hooks.c
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - hook helpers
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <asm/current.h>
+#include <linux/bpf.h> /* enum bpf_prog_aux */
+#include <linux/errno.h>
+#include <linux/filter.h> /* BPF_PROG_RUN() */
+#include <linux/rculist.h> /* list_add_tail_rcu */
+#include <uapi/linux/landlock.h> /* struct landlock_context */
+
+#include "common.h" /* struct landlock_rule, get_hook_index() */
+#include "hooks.h" /* landlock_hook_ctx */
+
+#include "hooks_fs.h"
+
+/* return a Landlock program context (e.g. hook_ctx->fs_walk.prog_ctx) */
+static const void *get_ctx(enum landlock_hook_type hook_type,
+ struct landlock_hook_ctx *hook_ctx)
+{
+ switch (hook_type) {
+ case LANDLOCK_HOOK_FS_WALK:
+ return landlock_get_ctx_fs_walk(hook_ctx->fs_walk);
+ case LANDLOCK_HOOK_FS_PICK:
+ return landlock_get_ctx_fs_pick(hook_ctx->fs_pick);
+ }
+ WARN_ON(1);
+ return NULL;
+}
+
+/**
+ * landlock_access_deny - run Landlock programs tied to a hook
+ *
+ * @hook_idx: hook index in the programs array
+ * @ctx: non-NULL valid eBPF context
+ * @prog_set: Landlock program set pointer
+ * @triggers: a bitmask to check if a program should be run
+ *
+ * Return true if at least one program return deny.
+ */
+static bool landlock_access_deny(enum landlock_hook_type hook_type,
+ struct landlock_hook_ctx *hook_ctx,
+ struct landlock_prog_set *prog_set, u64 triggers)
+{
+ struct landlock_prog_list *prog_list, *prev_list = NULL;
+ u32 hook_idx = get_hook_index(hook_type);
+
+ if (!prog_set)
+ return false;
+
+ for (prog_list = prog_set->programs[hook_idx];
+ prog_list; prog_list = prog_list->prev) {
+ u32 ret;
+ const void *prog_ctx;
+
+ /* check if @prog expect at least one of this triggers */
+ if (triggers && !(triggers & prog_list->prog->aux->
+ expected_attach_triggers))
+ continue;
+ prog_ctx = get_ctx(hook_type, hook_ctx);
+ if (!prog_ctx || WARN_ON(IS_ERR(prog_ctx)))
+ return true;
+ rcu_read_lock();
+ ret = BPF_PROG_RUN(prog_list->prog, prog_ctx);
+ rcu_read_unlock();
+ /* deny access if a program returns a value different than 0 */
+ if (ret)
+ return true;
+ if (prev_list && prog_list->prev && prog_list->prev->prog->
+ expected_attach_type ==
+ prev_list->prog->expected_attach_type)
+ WARN_ON(prog_list->prev != prev_list);
+ prev_list = prog_list;
+ }
+ return false;
+}
+
+int landlock_decide(enum landlock_hook_type hook_type,
+ struct landlock_hook_ctx *hook_ctx, u64 triggers)
+{
+ bool deny = false;
+
+#ifdef CONFIG_SECCOMP_FILTER
+ deny = landlock_access_deny(hook_type, hook_ctx,
+ current->seccomp.landlock_prog_set, triggers);
+#endif /* CONFIG_SECCOMP_FILTER */
+
+ /* should we use -EPERM or -EACCES? */
+ return deny ? -EACCES : 0;
+}
diff --git a/security/landlock/hooks.h b/security/landlock/hooks.h
new file mode 100644
index 000000000000..31446e6629fb
--- /dev/null
+++ b/security/landlock/hooks.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - hooks helpers
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <asm/current.h>
+#include <linux/sched.h> /* struct task_struct */
+#include <linux/seccomp.h>
+
+#include "hooks_fs.h"
+
+struct landlock_hook_ctx {
+ union {
+ struct landlock_hook_ctx_fs_walk *fs_walk;
+ struct landlock_hook_ctx_fs_pick *fs_pick;
+ };
+};
+
+static inline bool landlocked(const struct task_struct *task)
+{
+#ifdef CONFIG_SECCOMP_FILTER
+ return !!(task->seccomp.landlock_prog_set);
+#else
+ return false;
+#endif /* CONFIG_SECCOMP_FILTER */
+}
+
+int landlock_decide(enum landlock_hook_type, struct landlock_hook_ctx *, u64);
diff --git a/security/landlock/hooks_fs.c b/security/landlock/hooks_fs.c
new file mode 100644
index 000000000000..3f81b7fc2938
--- /dev/null
+++ b/security/landlock/hooks_fs.c
@@ -0,0 +1,554 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - filesystem hooks
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <linux/bpf.h> /* enum bpf_access_type */
+#include <linux/kernel.h> /* ARRAY_SIZE */
+#include <linux/lsm_hooks.h>
+#include <linux/rcupdate.h> /* synchronize_rcu() */
+#include <linux/stat.h> /* S_ISDIR */
+#include <linux/stddef.h> /* offsetof */
+#include <linux/types.h> /* uintptr_t */
+#include <linux/workqueue.h> /* INIT_WORK() */
+
+/* permissions translation */
+#include <linux/fs.h> /* MAY_* */
+#include <linux/mman.h> /* PROT_* */
+#include <linux/namei.h>
+
+/* hook arguments */
+#include <linux/dcache.h> /* struct dentry */
+#include <linux/fs.h> /* struct inode, struct iattr */
+#include <linux/mm_types.h> /* struct vm_area_struct */
+#include <linux/mount.h> /* struct vfsmount */
+#include <linux/path.h> /* struct path */
+#include <linux/sched.h> /* struct task_struct */
+#include <linux/time.h> /* struct timespec */
+
+#include "common.h"
+#include "hooks_fs.h"
+#include "hooks.h"
+
+/* fs_pick */
+
+#include <asm/page.h> /* PAGE_SIZE */
+#include <asm/syscall.h>
+#include <linux/dcache.h> /* d_path, dentry_path_raw */
+#include <linux/err.h> /* *_ERR */
+#include <linux/gfp.h> /* __get_free_page, GFP_KERNEL */
+#include <linux/path.h> /* struct path */
+
+bool landlock_is_valid_access_fs_pick(int off, enum bpf_access_type type,
+ enum bpf_reg_type *reg_type, int *max_size)
+{
+ switch (off) {
+ default:
+ return false;
+ }
+}
+
+bool landlock_is_valid_access_fs_walk(int off, enum bpf_access_type type,
+ enum bpf_reg_type *reg_type, int *max_size)
+{
+ switch (off) {
+ default:
+ return false;
+ }
+}
+
+/* fs_walk */
+
+struct landlock_hook_ctx_fs_walk {
+ struct landlock_ctx_fs_walk prog_ctx;
+};
+
+const struct landlock_ctx_fs_walk *landlock_get_ctx_fs_walk(
+ const struct landlock_hook_ctx_fs_walk *hook_ctx)
+{
+ if (WARN_ON(!hook_ctx))
+ return NULL;
+
+ return &hook_ctx->prog_ctx;
+}
+
+static int decide_fs_walk(int may_mask, struct inode *inode)
+{
+ struct landlock_hook_ctx_fs_walk fs_walk = {};
+ struct landlock_hook_ctx hook_ctx = {
+ .fs_walk = &fs_walk,
+ };
+ const enum landlock_hook_type hook_type = LANDLOCK_HOOK_FS_WALK;
+
+ if (!current_has_prog_type(hook_type))
+ /* no fs_walk */
+ return 0;
+ if (WARN_ON(!inode))
+ return -EFAULT;
+
+ /* init common data: inode */
+ fs_walk.prog_ctx.inode = (uintptr_t)inode;
+ return landlock_decide(hook_type, &hook_ctx, 0);
+}
+
+/* fs_pick */
+
+struct landlock_hook_ctx_fs_pick {
+ __u64 triggers;
+ struct landlock_ctx_fs_pick prog_ctx;
+};
+
+const struct landlock_ctx_fs_pick *landlock_get_ctx_fs_pick(
+ const struct landlock_hook_ctx_fs_pick *hook_ctx)
+{
+ if (WARN_ON(!hook_ctx))
+ return NULL;
+
+ return &hook_ctx->prog_ctx;
+}
+
+static int decide_fs_pick(__u64 triggers, struct inode *inode)
+{
+ struct landlock_hook_ctx_fs_pick fs_pick = {};
+ struct landlock_hook_ctx hook_ctx = {
+ .fs_pick = &fs_pick,
+ };
+ const enum landlock_hook_type hook_type = LANDLOCK_HOOK_FS_PICK;
+
+ if (WARN_ON(!triggers))
+ return 0;
+ if (!current_has_prog_type(hook_type))
+ /* no fs_pick */
+ return 0;
+ if (WARN_ON(!inode))
+ return -EFAULT;
+
+ fs_pick.triggers = triggers,
+ /* init common data: inode */
+ fs_pick.prog_ctx.inode = (uintptr_t)inode;
+ return landlock_decide(hook_type, &hook_ctx, fs_pick.triggers);
+}
+
+/* helpers */
+
+static u64 fs_may_to_triggers(int may_mask, umode_t mode)
+{
+ u64 ret = 0;
+
+ if (may_mask & MAY_EXEC)
+ ret |= LANDLOCK_TRIGGER_FS_PICK_EXECUTE;
+ if (may_mask & MAY_READ) {
+ if (S_ISDIR(mode))
+ ret |= LANDLOCK_TRIGGER_FS_PICK_READDIR;
+ else
+ ret |= LANDLOCK_TRIGGER_FS_PICK_READ;
+ }
+ if (may_mask & MAY_WRITE)
+ ret |= LANDLOCK_TRIGGER_FS_PICK_WRITE;
+ if (may_mask & MAY_APPEND)
+ ret |= LANDLOCK_TRIGGER_FS_PICK_APPEND;
+ if (may_mask & MAY_OPEN)
+ ret |= LANDLOCK_TRIGGER_FS_PICK_OPEN;
+ if (may_mask & MAY_CHROOT)
+ ret |= LANDLOCK_TRIGGER_FS_PICK_CHROOT;
+ else if (may_mask & MAY_CHDIR)
+ ret |= LANDLOCK_TRIGGER_FS_PICK_CHDIR;
+ /* XXX: ignore MAY_ACCESS */
+ WARN_ON(!ret);
+ return ret;
+}
+
+static inline u64 mem_prot_to_triggers(unsigned long prot, bool private)
+{
+ u64 ret = LANDLOCK_TRIGGER_FS_PICK_MAP;
+
+ /* private mapping do not write to files */
+ if (!private && (prot & PROT_WRITE))
+ ret |= LANDLOCK_TRIGGER_FS_PICK_WRITE;
+ if (prot & PROT_READ)
+ ret |= LANDLOCK_TRIGGER_FS_PICK_READ;
+ if (prot & PROT_EXEC)
+ ret |= LANDLOCK_TRIGGER_FS_PICK_EXECUTE;
+ WARN_ON(!ret);
+ return ret;
+}
+
+/* binder hooks */
+
+static int hook_binder_transfer_file(struct task_struct *from,
+ struct task_struct *to, struct file *file)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!file))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_TRANSFER,
+ file_inode(file));
+}
+
+/* sb hooks */
+
+static int hook_sb_statfs(struct dentry *dentry)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!dentry))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_GETATTR,
+ dentry->d_inode);
+}
+
+/* TODO: handle mount source and remount */
+static int hook_sb_mount(const char *dev_name, const struct path *path,
+ const char *type, unsigned long flags, void *data)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!path))
+ return 0;
+ if (WARN_ON(!path->dentry))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_MOUNTON,
+ path->dentry->d_inode);
+}
+
+/*
+ * The @old_path is similar to a destination mount point.
+ */
+static int hook_sb_pivotroot(const struct path *old_path,
+ const struct path *new_path)
+{
+ int err;
+
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!old_path))
+ return 0;
+ if (WARN_ON(!old_path->dentry))
+ return 0;
+ err = decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_MOUNTON,
+ old_path->dentry->d_inode);
+ if (err)
+ return err;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_CHROOT,
+ new_path->dentry->d_inode);
+}
+
+/* inode hooks */
+
+/* a directory inode contains only one dentry */
+static int hook_inode_create(struct inode *dir, struct dentry *dentry,
+ umode_t mode)
+{
+ if (!landlocked(current))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_CREATE, dir);
+}
+
+static int hook_inode_link(struct dentry *old_dentry, struct inode *dir,
+ struct dentry *new_dentry)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!old_dentry)) {
+ int ret = decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_LINK,
+ old_dentry->d_inode);
+ if (ret)
+ return ret;
+ }
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_LINKTO, dir);
+}
+
+static int hook_inode_unlink(struct inode *dir, struct dentry *dentry)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!dentry))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_UNLINK,
+ dentry->d_inode);
+}
+
+static int hook_inode_symlink(struct inode *dir, struct dentry *dentry,
+ const char *old_name)
+{
+ if (!landlocked(current))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_CREATE, dir);
+}
+
+static int hook_inode_mkdir(struct inode *dir, struct dentry *dentry,
+ umode_t mode)
+{
+ if (!landlocked(current))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_CREATE, dir);
+}
+
+static int hook_inode_rmdir(struct inode *dir, struct dentry *dentry)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!dentry))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_RMDIR, dentry->d_inode);
+}
+
+static int hook_inode_mknod(struct inode *dir, struct dentry *dentry,
+ umode_t mode, dev_t dev)
+{
+ if (!landlocked(current))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_CREATE, dir);
+}
+
+static int hook_inode_rename(struct inode *old_dir, struct dentry *old_dentry,
+ struct inode *new_dir, struct dentry *new_dentry)
+{
+ if (!landlocked(current))
+ return 0;
+ if (!WARN_ON(!old_dentry)) {
+ int ret = decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_RENAME,
+ old_dentry->d_inode);
+ if (ret)
+ return ret;
+ }
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_RENAMETO, new_dir);
+}
+
+static int hook_inode_readlink(struct dentry *dentry)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!dentry))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_READ, dentry->d_inode);
+}
+
+/*
+ * ignore the inode_follow_link hook (could set is_symlink in the fs_walk
+ * context)
+ */
+
+static int hook_inode_permission(struct inode *inode, int mask)
+{
+ u64 triggers;
+
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!inode))
+ return 0;
+
+ triggers = fs_may_to_triggers(mask, inode->i_mode);
+ /*
+ * decide_fs_walk() is exclusive with decide_fs_pick(): in a path walk,
+ * ignore execute-only access on directory for any fs_pick program
+ */
+ if (triggers == LANDLOCK_TRIGGER_FS_PICK_EXECUTE &&
+ S_ISDIR(inode->i_mode))
+ return decide_fs_walk(mask, inode);
+
+ return decide_fs_pick(triggers, inode);
+}
+
+static int hook_inode_setattr(struct dentry *dentry, struct iattr *attr)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!dentry))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_SETATTR,
+ dentry->d_inode);
+}
+
+static int hook_inode_getattr(const struct path *path)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!path))
+ return 0;
+ if (WARN_ON(!path->dentry))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_GETATTR,
+ path->dentry->d_inode);
+}
+
+static int hook_inode_setxattr(struct dentry *dentry, const char *name,
+ const void *value, size_t size, int flags)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!dentry))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_SETATTR,
+ dentry->d_inode);
+}
+
+static int hook_inode_getxattr(struct dentry *dentry, const char *name)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!dentry))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_GETATTR,
+ dentry->d_inode);
+}
+
+static int hook_inode_listxattr(struct dentry *dentry)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!dentry))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_GETATTR,
+ dentry->d_inode);
+}
+
+static int hook_inode_removexattr(struct dentry *dentry, const char *name)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!dentry))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_SETATTR,
+ dentry->d_inode);
+}
+
+static int hook_inode_getsecurity(struct inode *inode, const char *name,
+ void **buffer, bool alloc)
+{
+ if (!landlocked(current))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_GETATTR, inode);
+}
+
+static int hook_inode_setsecurity(struct inode *inode, const char *name,
+ const void *value, size_t size, int flag)
+{
+ if (!landlocked(current))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_SETATTR, inode);
+}
+
+static int hook_inode_listsecurity(struct inode *inode, char *buffer,
+ size_t buffer_size)
+{
+ if (!landlocked(current))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_GETATTR, inode);
+}
+
+/* file hooks */
+
+static int hook_file_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!file))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_IOCTL,
+ file_inode(file));
+}
+
+static int hook_file_lock(struct file *file, unsigned int cmd)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!file))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_LOCK, file_inode(file));
+}
+
+static int hook_file_fcntl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!file))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_FCNTL,
+ file_inode(file));
+}
+
+static int hook_mmap_file(struct file *file, unsigned long reqprot,
+ unsigned long prot, unsigned long flags)
+{
+ if (!landlocked(current))
+ return 0;
+ /* file can be null for anonymous mmap */
+ if (!file)
+ return 0;
+ return decide_fs_pick(mem_prot_to_triggers(prot, flags & MAP_PRIVATE),
+ file_inode(file));
+}
+
+static int hook_file_mprotect(struct vm_area_struct *vma,
+ unsigned long reqprot, unsigned long prot)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!vma))
+ return 0;
+ if (!vma->vm_file)
+ return 0;
+ return decide_fs_pick(mem_prot_to_triggers(prot,
+ !(vma->vm_flags & VM_SHARED)),
+ file_inode(vma->vm_file));
+}
+
+static int hook_file_receive(struct file *file)
+{
+ if (!landlocked(current))
+ return 0;
+ if (WARN_ON(!file))
+ return 0;
+ return decide_fs_pick(LANDLOCK_TRIGGER_FS_PICK_RECEIVE,
+ file_inode(file));
+}
+
+static struct security_hook_list landlock_hooks[] = {
+ LSM_HOOK_INIT(binder_transfer_file, hook_binder_transfer_file),
+
+ LSM_HOOK_INIT(sb_statfs, hook_sb_statfs),
+ LSM_HOOK_INIT(sb_mount, hook_sb_mount),
+ LSM_HOOK_INIT(sb_pivotroot, hook_sb_pivotroot),
+
+ LSM_HOOK_INIT(inode_create, hook_inode_create),
+ LSM_HOOK_INIT(inode_link, hook_inode_link),
+ LSM_HOOK_INIT(inode_unlink, hook_inode_unlink),
+ LSM_HOOK_INIT(inode_symlink, hook_inode_symlink),
+ LSM_HOOK_INIT(inode_mkdir, hook_inode_mkdir),
+ LSM_HOOK_INIT(inode_rmdir, hook_inode_rmdir),
+ LSM_HOOK_INIT(inode_mknod, hook_inode_mknod),
+ LSM_HOOK_INIT(inode_rename, hook_inode_rename),
+ LSM_HOOK_INIT(inode_readlink, hook_inode_readlink),
+ LSM_HOOK_INIT(inode_permission, hook_inode_permission),
+ LSM_HOOK_INIT(inode_setattr, hook_inode_setattr),
+ LSM_HOOK_INIT(inode_getattr, hook_inode_getattr),
+ LSM_HOOK_INIT(inode_setxattr, hook_inode_setxattr),
+ LSM_HOOK_INIT(inode_getxattr, hook_inode_getxattr),
+ LSM_HOOK_INIT(inode_listxattr, hook_inode_listxattr),
+ LSM_HOOK_INIT(inode_removexattr, hook_inode_removexattr),
+ LSM_HOOK_INIT(inode_getsecurity, hook_inode_getsecurity),
+ LSM_HOOK_INIT(inode_setsecurity, hook_inode_setsecurity),
+ LSM_HOOK_INIT(inode_listsecurity, hook_inode_listsecurity),
+
+ /* do not handle file_permission for now */
+ LSM_HOOK_INIT(file_ioctl, hook_file_ioctl),
+ LSM_HOOK_INIT(file_lock, hook_file_lock),
+ LSM_HOOK_INIT(file_fcntl, hook_file_fcntl),
+ LSM_HOOK_INIT(mmap_file, hook_mmap_file),
+ LSM_HOOK_INIT(file_mprotect, hook_file_mprotect),
+ LSM_HOOK_INIT(file_receive, hook_file_receive),
+ /* file_open is not handled, use inode_permission instead */
+};
+
+__init void landlock_add_hooks_fs(void)
+{
+ security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
+ LANDLOCK_NAME);
+}
diff --git a/security/landlock/hooks_fs.h b/security/landlock/hooks_fs.h
new file mode 100644
index 000000000000..eeae4dcd842f
--- /dev/null
+++ b/security/landlock/hooks_fs.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - filesystem hooks
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <linux/bpf.h> /* enum bpf_access_type */
+
+__init void landlock_add_hooks_fs(void);
+
+/* fs_pick */
+
+struct landlock_hook_ctx_fs_pick;
+
+bool landlock_is_valid_access_fs_pick(int off, enum bpf_access_type type,
+ enum bpf_reg_type *reg_type, int *max_size);
+
+const struct landlock_ctx_fs_pick *landlock_get_ctx_fs_pick(
+ const struct landlock_hook_ctx_fs_pick *hook_ctx);
+
+/* fs_walk */
+
+struct landlock_hook_ctx_fs_walk;
+
+bool landlock_is_valid_access_fs_walk(int off, enum bpf_access_type type,
+ enum bpf_reg_type *reg_type, int *max_size);
+
+const struct landlock_ctx_fs_walk *landlock_get_ctx_fs_walk(
+ const struct landlock_hook_ctx_fs_walk *hook_ctx);
diff --git a/security/landlock/init.c b/security/landlock/init.c
index 8dfd5fea3c1f..391e88bd4d3a 100644
--- a/security/landlock/init.c
+++ b/security/landlock/init.c
@@ -9,8 +9,10 @@
#include <linux/bpf.h> /* enum bpf_access_type */
#include <linux/capability.h> /* capable */
#include <linux/filter.h> /* struct bpf_prog */
+#include <linux/lsm_hooks.h>
#include "common.h" /* LANDLOCK_* */
+#include "hooks_fs.h"
static bool bpf_landlock_is_valid_access(int off, int size,
enum bpf_access_type type, const struct bpf_prog *prog,
@@ -27,6 +29,20 @@ static bool bpf_landlock_is_valid_access(int off, int size,
if (size <= 0 || size > sizeof(__u64))
return false;
+ /* set register type and max size */
+ switch (get_hook_type(prog)) {
+ case LANDLOCK_HOOK_FS_PICK:
+ if (!landlock_is_valid_access_fs_pick(off, type, ®_type,
+ &max_size))
+ return false;
+ break;
+ case LANDLOCK_HOOK_FS_WALK:
+ if (!landlock_is_valid_access_fs_walk(off, type, ®_type,
+ &max_size))
+ return false;
+ break;
+ }
+
/* check memory range access */
switch (reg_type) {
case NOT_INIT:
@@ -98,3 +114,20 @@ const struct bpf_verifier_ops landlock_verifier_ops = {
};
const struct bpf_prog_ops landlock_prog_ops = {};
+
+static int __init landlock_init(void)
+{
+ pr_info(LANDLOCK_NAME ": Initializing (sandbox with seccomp)\n");
+ landlock_add_hooks_fs();
+ return 0;
+}
+
+struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = {
+};
+
+DEFINE_LSM(LANDLOCK_NAME) = {
+ .name = LANDLOCK_NAME,
+ .order = LSM_ORDER_LAST,
+ .blobs = &landlock_blob_sizes,
+ .init = landlock_init,
+};
diff --git a/security/security.c b/security/security.c
index 250ee2d76406..e694e5fe7021 100644
--- a/security/security.c
+++ b/security/security.c
@@ -263,6 +263,21 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
}
}
+ /*
+ * In case of an unprivileged access-control, we don't want to give the
+ * ability to any process to do some checks (e.g. through an eBPF
+ * program) on kernel objects (e.g. files) if a privileged security
+ * policy forbid their access. We must then load
+ * potentially-unprivileged security modules after all other LSMs.
+ *
+ * LSM_ORDER_LAST is always last and does not appear in the modifiable
+ * ordered list of enabled LSMs.
+ */
+ for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
+ if (lsm->order == LSM_ORDER_LAST)
+ append_ordered_lsm(lsm, "last");
+ }
+
/* Disable all LSMs not in the ordered list. */
for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
if (exists_ordered_lsm(lsm))
--
2.22.0
^ permalink raw reply related
* [PATCH bpf-next v10 09/10] bpf,landlock: Add tests for Landlock
From: Mickaël Salaün @ 2019-07-21 21:31 UTC (permalink / raw)
To: linux-kernel
Cc: Mickaël Salaün, Alexander Viro, Alexei Starovoitov,
Andrew Morton, Andy Lutomirski, Arnaldo Carvalho de Melo,
Casey Schaufler, Daniel Borkmann, David Drysdale,
David S . Miller, Eric W . Biederman, James Morris, Jann Horn,
John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
Mickaël Salaün, Paul Moore, Sargun Dhillon,
Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
Tetsuo Handa, Thomas Graf, Tycho Andersen, Will Drewry,
kernel-hardening, linux-api, linux-fsdevel, linux-security-module,
netdev
In-Reply-To: <20190721213116.23476-1-mic@digikod.net>
Test basic context access, ptrace protection and filesystem hooks.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Will Drewry <wad@chromium.org>
---
Changes since v9:
* replace subtype with expected_attach_type and expected_attach_triggers
* rename inode_map_lookup() into inode_map_lookup_elem()
* check for inode map entry without value (which is now possible thanks
to the pointer null check)
* use read-only inode map for Landlock programs
Changes since v8:
* update eBPF include path for macros
* use TEST_GEN_PROGS and use the generic "clean" target
* add more verbose errors
* update the bpf/verifier files
* remove chain tests (from landlock and bpf/verifier)
* replace the whitelist tests with blacklist tests (because of stateless
Landlock programs): remove "dotdot" tests and other depth tests
* sync the landlock Makefile with its bpf sibling directory and use
bpf_load_program_xattr()
Changes since v7:
* update tests and add new ones for filesystem hierarchy and Landlock
chains.
Changes since v6:
* use the new kselftest_harness.h
* use const variables
* replace ASSERT_STEP with ASSERT_*
* rename BPF_PROG_TYPE_LANDLOCK to BPF_PROG_TYPE_LANDLOCK_RULE
* force sample library rebuild
* fix install target
Changes since v5:
* add subtype test
* add ptrace tests
* split and rename files
* cleanup and rebase
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/bpf/test_verifier.c | 1 +
.../testing/selftests/bpf/verifier/landlock.c | 24 ++
tools/testing/selftests/landlock/.gitignore | 4 +
tools/testing/selftests/landlock/Makefile | 39 +++
tools/testing/selftests/landlock/test.h | 50 ++++
tools/testing/selftests/landlock/test_base.c | 24 ++
tools/testing/selftests/landlock/test_fs.c | 256 ++++++++++++++++++
.../testing/selftests/landlock/test_ptrace.c | 148 ++++++++++
9 files changed, 547 insertions(+)
create mode 100644 tools/testing/selftests/bpf/verifier/landlock.c
create mode 100644 tools/testing/selftests/landlock/.gitignore
create mode 100644 tools/testing/selftests/landlock/Makefile
create mode 100644 tools/testing/selftests/landlock/test.h
create mode 100644 tools/testing/selftests/landlock/test_base.c
create mode 100644 tools/testing/selftests/landlock/test_fs.c
create mode 100644 tools/testing/selftests/landlock/test_ptrace.c
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 25b43a8c2b15..1949fbb3098e 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -21,6 +21,7 @@ TARGETS += ir
TARGETS += kcmp
TARGETS += kexec
TARGETS += kvm
+TARGETS += landlock
TARGETS += lib
TARGETS += livepatch
TARGETS += membarrier
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index b0773291012a..b8542431c78b 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -30,6 +30,7 @@
#include <linux/bpf.h>
#include <linux/if_ether.h>
#include <linux/btf.h>
+#include <linux/landlock.h>
#include <bpf/bpf.h>
#include <bpf/libbpf.h>
diff --git a/tools/testing/selftests/bpf/verifier/landlock.c b/tools/testing/selftests/bpf/verifier/landlock.c
new file mode 100644
index 000000000000..eaf6dddbf208
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/landlock.c
@@ -0,0 +1,24 @@
+{
+ "landlock/fs_walk: always accept",
+ .insns = {
+ BPF_MOV32_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_LANDLOCK_HOOK,
+ .expected_attach_type = BPF_LANDLOCK_FS_WALK,
+},
+{
+ "landlock/fs_pick: read context",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_7, BPF_REG_6,
+ offsetof(struct landlock_ctx_fs_pick, inode)),
+ BPF_MOV32_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_LANDLOCK_HOOK,
+ .expected_attach_type = BPF_LANDLOCK_FS_PICK,
+ .expected_attach_triggers = LANDLOCK_TRIGGER_FS_PICK_READ,
+},
diff --git a/tools/testing/selftests/landlock/.gitignore b/tools/testing/selftests/landlock/.gitignore
new file mode 100644
index 000000000000..25b9cd834c3c
--- /dev/null
+++ b/tools/testing/selftests/landlock/.gitignore
@@ -0,0 +1,4 @@
+/test_base
+/test_fs
+/test_ptrace
+/tmp_*
diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
new file mode 100644
index 000000000000..7a253bf6d580
--- /dev/null
+++ b/tools/testing/selftests/landlock/Makefile
@@ -0,0 +1,39 @@
+LIBDIR := ../../../lib
+BPFDIR := $(LIBDIR)/bpf
+APIDIR := ../../../include/uapi
+GENDIR := ../../../../include/generated
+GENHDR := $(GENDIR)/autoconf.h
+
+ifneq ($(wildcard $(GENHDR)),)
+ GENFLAGS := -DHAVE_GENHDR
+endif
+
+BPFOBJS := $(BPFDIR)/bpf.o $(BPFDIR)/nlattr.o
+LOADOBJ := ../../../../samples/bpf/bpf_load.o
+
+CFLAGS += -Wl,-no-as-needed -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
+LDFLAGS += -lelf
+
+test_src = $(wildcard test_*.c)
+
+test_objs := $(test_src:.c=)
+
+TEST_GEN_PROGS := $(test_objs)
+
+.PHONY: all clean force
+
+all: $(test_objs)
+
+# force a rebuild of BPFOBJS when its dependencies are updated
+force:
+
+# rebuild bpf.o as a workaround for the samples/bpf bug
+$(BPFOBJS): $(LOADOBJ) force
+ $(MAKE) -C $(BPFDIR)
+
+$(LOADOBJ): force
+ $(MAKE) -C $(dir $(LOADOBJ))
+
+$(test_objs): $(BPFOBJS) $(LOADOBJ) ../kselftest_harness.h
+
+include ../lib.mk
diff --git a/tools/testing/selftests/landlock/test.h b/tools/testing/selftests/landlock/test.h
new file mode 100644
index 000000000000..e1e86a804180
--- /dev/null
+++ b/tools/testing/selftests/landlock/test.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Landlock helpers
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2019 ANSSI
+ */
+
+#include <bpf/bpf.h>
+#include <errno.h>
+#include <linux/filter.h>
+#include <linux/landlock.h>
+#include <linux/seccomp.h>
+#include <sys/prctl.h>
+#include <sys/syscall.h>
+
+#include "../kselftest_harness.h"
+#include "../../../../samples/bpf/bpf_load.h"
+
+#ifndef SECCOMP_PREPEND_LANDLOCK_PROG
+#define SECCOMP_PREPEND_LANDLOCK_PROG 4
+#endif
+
+#ifndef seccomp
+static int __attribute__((unused)) seccomp(unsigned int op, unsigned int flags,
+ void *args)
+{
+ errno = 0;
+ return syscall(__NR_seccomp, op, flags, args);
+}
+#endif
+
+/* bpf_load_program() with subtype */
+static int __attribute__((unused)) ll_bpf_load_program(
+ const struct bpf_insn *insns, size_t insns_cnt, char *log_buf,
+ size_t log_buf_sz, const enum bpf_attach_type attach_type,
+ __u64 attach_triggers)
+{
+ struct bpf_load_program_attr load_attr;
+
+ memset(&load_attr, 0, sizeof(struct bpf_load_program_attr));
+ load_attr.prog_type = BPF_PROG_TYPE_LANDLOCK_HOOK;
+ load_attr.expected_attach_type = attach_type;
+ load_attr.expected_attach_triggers = attach_triggers;
+ load_attr.insns = insns;
+ load_attr.insns_cnt = insns_cnt;
+ load_attr.license = "GPL";
+
+ return bpf_load_program_xattr(&load_attr, log_buf, log_buf_sz);
+}
diff --git a/tools/testing/selftests/landlock/test_base.c b/tools/testing/selftests/landlock/test_base.c
new file mode 100644
index 000000000000..db46f39048cb
--- /dev/null
+++ b/tools/testing/selftests/landlock/test_base.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Landlock tests - base
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+
+#include "test.h"
+
+TEST(seccomp_landlock)
+{
+ int ret;
+
+ ret = seccomp(SECCOMP_PREPEND_LANDLOCK_PROG, 0, NULL);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EFAULT, errno) {
+ TH_LOG("Kernel does not support CONFIG_SECURITY_LANDLOCK");
+ }
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/landlock/test_fs.c b/tools/testing/selftests/landlock/test_fs.c
new file mode 100644
index 000000000000..f35b99fcb70f
--- /dev/null
+++ b/tools/testing/selftests/landlock/test_fs.c
@@ -0,0 +1,256 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Landlock tests - file system
+ *
+ * Copyright © 2018-2019 Mickaël Salaün <mic@digikod.net>
+ */
+
+#include <bpf/bpf.h> /* bpf_create_map() */
+#include <fcntl.h> /* O_DIRECTORY */
+#include <sys/stat.h> /* statbuf */
+#include <unistd.h> /* faccessat() */
+
+#include "test.h"
+
+#define TEST_PATH_TRIGGERS ( \
+ LANDLOCK_TRIGGER_FS_PICK_OPEN | \
+ LANDLOCK_TRIGGER_FS_PICK_READDIR | \
+ LANDLOCK_TRIGGER_FS_PICK_EXECUTE | \
+ LANDLOCK_TRIGGER_FS_PICK_GETATTR)
+
+static void test_path_rel(struct __test_metadata *_metadata, int dirfd,
+ const char *path, int ret)
+{
+ int fd;
+ struct stat statbuf;
+
+ ASSERT_EQ(ret, faccessat(dirfd, path, R_OK | X_OK, 0));
+ ASSERT_EQ(ret, fstatat(dirfd, path, &statbuf, 0));
+ fd = openat(dirfd, path, O_DIRECTORY);
+ if (ret) {
+ ASSERT_EQ(-1, fd);
+ } else {
+ ASSERT_NE(-1, fd);
+ EXPECT_EQ(0, close(fd));
+ }
+}
+
+static void test_path(struct __test_metadata *_metadata, const char *path,
+ int ret)
+{
+ return test_path_rel(_metadata, AT_FDCWD, path, ret);
+}
+
+static const char d1[] = "/usr";
+static const char d2[] = "/usr/share";
+static const char d3[] = "/usr/share/doc";
+
+TEST(fs_base)
+{
+ test_path(_metadata, d1, 0);
+ test_path(_metadata, d2, 0);
+ test_path(_metadata, d3, 0);
+}
+
+#define MAP_VALUE_DENY 1
+
+static int create_denied_inode_map(struct __test_metadata *_metadata,
+ const char *const dirs[])
+{
+ int map, key, dirs_len, i;
+ __u64 value = MAP_VALUE_DENY;
+
+ ASSERT_NE(NULL, dirs) {
+ TH_LOG("No directory list\n");
+ }
+ ASSERT_NE(NULL, dirs[0]) {
+ TH_LOG("Empty directory list\n");
+ }
+
+ /* get the number of dir entries */
+ for (dirs_len = 0; dirs[dirs_len]; dirs_len++);
+ map = bpf_create_map(BPF_MAP_TYPE_INODE, sizeof(key), sizeof(value),
+ dirs_len, BPF_F_RDONLY_PROG);
+ ASSERT_NE(-1, map) {
+ TH_LOG("Failed to create a map of %d elements: %s\n", dirs_len,
+ strerror(errno));
+ }
+
+ for (i = 0; dirs[i]; i++) {
+ key = open(dirs[i], O_RDONLY | O_CLOEXEC | O_DIRECTORY);
+ ASSERT_NE(-1, key) {
+ TH_LOG("Failed to open directory \"%s\": %s\n", dirs[i],
+ strerror(errno));
+ }
+ ASSERT_EQ(0, bpf_map_update_elem(map, &key, &value, BPF_ANY)) {
+ TH_LOG("Failed to update the map with \"%s\": %s\n",
+ dirs[i], strerror(errno));
+ }
+ close(key);
+ }
+ return map;
+}
+
+static void enforce_map(struct __test_metadata *_metadata, int map,
+ bool subpath)
+{
+ const struct bpf_insn prog_deny[] = {
+ BPF_ALU64_REG(BPF_MOV, BPF_REG_6, BPF_REG_1),
+ /* look for the requested inode in the map */
+ BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_6,
+ offsetof(struct landlock_ctx_fs_walk, inode)),
+ BPF_LD_MAP_FD(BPF_REG_1, map), /* 2 instructions */
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
+ BPF_FUNC_inode_map_lookup_elem),
+ /* if there is no mark, then allow access to this inode */
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0, 0),
+ /* otherwise, deny access to this inode */
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, MAP_VALUE_DENY, 2),
+ BPF_MOV32_IMM(BPF_REG_0, LANDLOCK_RET_ALLOW),
+ BPF_EXIT_INSN(),
+ BPF_MOV32_IMM(BPF_REG_0, LANDLOCK_RET_DENY),
+ BPF_EXIT_INSN(),
+ };
+ int fd_walk = -1, fd_pick;
+ char log[1024] = "";
+
+ if (subpath) {
+ fd_walk = ll_bpf_load_program((const struct bpf_insn *)&prog_deny,
+ sizeof(prog_deny) / sizeof(struct bpf_insn),
+ log, sizeof(log), BPF_LANDLOCK_FS_WALK, 0);
+ ASSERT_NE(-1, fd_walk) {
+ TH_LOG("Failed to load fs_walk program: %s\n%s",
+ strerror(errno), log);
+ }
+ ASSERT_EQ(0, seccomp(SECCOMP_PREPEND_LANDLOCK_PROG, 0, &fd_walk)) {
+ TH_LOG("Failed to apply Landlock program: %s", strerror(errno));
+ }
+ EXPECT_EQ(0, close(fd_walk));
+ }
+
+ fd_pick = ll_bpf_load_program((const struct bpf_insn *)&prog_deny,
+ sizeof(prog_deny) / sizeof(struct bpf_insn), log,
+ sizeof(log), BPF_LANDLOCK_FS_PICK, TEST_PATH_TRIGGERS);
+ ASSERT_NE(-1, fd_pick) {
+ TH_LOG("Failed to load fs_pick program: %s\n%s",
+ strerror(errno), log);
+ }
+ ASSERT_EQ(0, seccomp(SECCOMP_PREPEND_LANDLOCK_PROG, 0, &fd_pick)) {
+ TH_LOG("Failed to apply Landlock program: %s", strerror(errno));
+ }
+ EXPECT_EQ(0, close(fd_pick));
+}
+
+static void check_map_blacklist(struct __test_metadata *_metadata,
+ bool subpath)
+{
+ int map = create_denied_inode_map(_metadata, (const char *const [])
+ { d2, NULL });
+ ASSERT_NE(-1, map);
+ enforce_map(_metadata, map, subpath);
+ test_path(_metadata, d1, 0);
+ test_path(_metadata, d2, -1);
+ test_path(_metadata, d3, subpath ? -1 : 0);
+ EXPECT_EQ(0, close(map));
+}
+
+TEST(fs_map_blacklist_literal)
+{
+ check_map_blacklist(_metadata, false);
+}
+
+TEST(fs_map_blacklist_subpath)
+{
+ check_map_blacklist(_metadata, true);
+}
+
+static const char r2[] = ".";
+static const char r3[] = "./doc";
+
+enum relative_access {
+ REL_OPEN,
+ REL_CHDIR,
+ REL_CHROOT,
+};
+
+static void check_access(struct __test_metadata *_metadata,
+ bool enforce, enum relative_access rel)
+{
+ int dirfd;
+ int map = -1;
+
+ if (rel == REL_CHROOT)
+ ASSERT_NE(-1, chdir(d2));
+ if (enforce) {
+ map = create_denied_inode_map(_metadata, (const char *const [])
+ { d3, NULL });
+ ASSERT_NE(-1, map);
+ enforce_map(_metadata, map, true);
+ }
+ switch (rel) {
+ case REL_OPEN:
+ dirfd = open(d2, O_DIRECTORY);
+ ASSERT_NE(-1, dirfd);
+ break;
+ case REL_CHDIR:
+ ASSERT_NE(-1, chdir(d2));
+ dirfd = AT_FDCWD;
+ break;
+ case REL_CHROOT:
+ ASSERT_NE(-1, chroot(d2)) {
+ TH_LOG("Failed to chroot: %s\n", strerror(errno));
+ }
+ dirfd = AT_FDCWD;
+ break;
+ default:
+ ASSERT_TRUE(false);
+ return;
+ }
+
+ test_path_rel(_metadata, dirfd, r2, 0);
+ test_path_rel(_metadata, dirfd, r3, enforce ? -1 : 0);
+
+ if (rel == REL_OPEN)
+ EXPECT_EQ(0, close(dirfd));
+ if (enforce)
+ EXPECT_EQ(0, close(map));
+}
+
+TEST(fs_allow_open)
+{
+ /* no enforcement, via open */
+ check_access(_metadata, false, REL_OPEN);
+}
+
+TEST(fs_allow_chdir)
+{
+ /* no enforcement, via chdir */
+ check_access(_metadata, false, REL_CHDIR);
+}
+
+TEST(fs_allow_chroot)
+{
+ /* no enforcement, via chroot */
+ check_access(_metadata, false, REL_CHROOT);
+}
+
+TEST(fs_deny_open)
+{
+ /* enforcement without tag, via open */
+ check_access(_metadata, true, REL_OPEN);
+}
+
+TEST(fs_deny_chdir)
+{
+ /* enforcement without tag, via chdir */
+ check_access(_metadata, true, REL_CHDIR);
+}
+
+TEST(fs_deny_chroot)
+{
+ /* enforcement without tag, via chroot */
+ check_access(_metadata, true, REL_CHROOT);
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/landlock/test_ptrace.c b/tools/testing/selftests/landlock/test_ptrace.c
new file mode 100644
index 000000000000..b190a809ceec
--- /dev/null
+++ b/tools/testing/selftests/landlock/test_ptrace.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Landlock tests - ptrace
+ *
+ * Copyright © 2017-2019 Mickaël Salaün <mic@digikod.net>
+ */
+
+#define _GNU_SOURCE
+#include <signal.h> /* raise */
+#include <sys/ptrace.h>
+#include <sys/types.h> /* waitpid */
+#include <sys/wait.h> /* waitpid */
+#include <unistd.h> /* fork, pipe */
+
+#include "test.h"
+
+static void apply_null_sandbox(struct __test_metadata *_metadata)
+{
+ const struct bpf_insn prog_accept[] = {
+ BPF_MOV32_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ };
+ int prog;
+ char log[256] = "";
+
+ prog = ll_bpf_load_program((const struct bpf_insn *)&prog_accept,
+ sizeof(prog_accept) / sizeof(struct bpf_insn), log,
+ sizeof(log), BPF_LANDLOCK_FS_PICK, LANDLOCK_TRIGGER_FS_PICK_OPEN);
+ ASSERT_NE(-1, prog) {
+ TH_LOG("Failed to load minimal rule: %s\n%s",
+ strerror(errno), log);
+ }
+ ASSERT_EQ(0, seccomp(SECCOMP_PREPEND_LANDLOCK_PROG, 0, &prog)) {
+ TH_LOG("Failed to apply minimal rule: %s", strerror(errno));
+ }
+ EXPECT_EQ(0, close(prog));
+}
+
+/* PTRACE_TRACEME and PTRACE_ATTACH without Landlock rules effect */
+static void check_ptrace(struct __test_metadata *_metadata,
+ int sandbox_both, int sandbox_parent, int sandbox_child,
+ int expect_ptrace)
+{
+ pid_t child;
+ int status;
+ int pipefd[2];
+
+ ASSERT_EQ(0, pipe(pipefd));
+ if (sandbox_both)
+ apply_null_sandbox(_metadata);
+
+ child = fork();
+ ASSERT_LE(0, child);
+ if (child == 0) {
+ char buf;
+
+ EXPECT_EQ(0, close(pipefd[1]));
+ if (sandbox_child)
+ apply_null_sandbox(_metadata);
+
+ /* test traceme */
+ ASSERT_EQ(expect_ptrace, ptrace(PTRACE_TRACEME));
+ if (expect_ptrace) {
+ ASSERT_EQ(EPERM, errno);
+ } else {
+ ASSERT_EQ(0, raise(SIGSTOP));
+ }
+
+ /* sync */
+ ASSERT_EQ(1, read(pipefd[0], &buf, 1)) {
+ TH_LOG("Failed to read() sync from parent");
+ }
+ ASSERT_EQ('.', buf);
+ _exit(_metadata->passed ? EXIT_SUCCESS : EXIT_FAILURE);
+ }
+
+ EXPECT_EQ(0, close(pipefd[0]));
+ if (sandbox_parent)
+ apply_null_sandbox(_metadata);
+
+ /* test traceme */
+ if (!expect_ptrace) {
+ ASSERT_EQ(child, waitpid(child, &status, 0));
+ ASSERT_EQ(1, WIFSTOPPED(status));
+ ASSERT_EQ(0, ptrace(PTRACE_DETACH, child, NULL, 0));
+ }
+ /* test attach */
+ ASSERT_EQ(expect_ptrace, ptrace(PTRACE_ATTACH, child, NULL, 0));
+ if (expect_ptrace) {
+ ASSERT_EQ(EPERM, errno);
+ } else {
+ ASSERT_EQ(child, waitpid(child, &status, 0));
+ ASSERT_EQ(1, WIFSTOPPED(status));
+ ASSERT_EQ(0, ptrace(PTRACE_CONT, child, NULL, 0));
+ }
+
+ /* sync */
+ ASSERT_EQ(1, write(pipefd[1], ".", 1)) {
+ TH_LOG("Failed to write() sync to child");
+ }
+ ASSERT_EQ(child, waitpid(child, &status, 0));
+ if (WIFSIGNALED(status) || WEXITSTATUS(status))
+ _metadata->passed = 0;
+}
+
+TEST(ptrace_allow_without_sandbox)
+{
+ /* no sandbox */
+ check_ptrace(_metadata, 0, 0, 0, 0);
+}
+
+TEST(ptrace_allow_with_one_sandbox)
+{
+ /* child sandbox */
+ check_ptrace(_metadata, 0, 0, 1, 0);
+}
+
+TEST(ptrace_allow_with_nested_sandbox)
+{
+ /* inherited and child sandbox */
+ check_ptrace(_metadata, 1, 0, 1, 0);
+}
+
+TEST(ptrace_deny_with_parent_sandbox)
+{
+ /* parent sandbox */
+ check_ptrace(_metadata, 0, 1, 0, -1);
+}
+
+TEST(ptrace_deny_with_nested_and_parent_sandbox)
+{
+ /* inherited and parent sandbox */
+ check_ptrace(_metadata, 1, 1, 0, -1);
+}
+
+TEST(ptrace_deny_with_forked_sandbox)
+{
+ /* inherited, parent and child sandbox */
+ check_ptrace(_metadata, 1, 1, 1, -1);
+}
+
+TEST(ptrace_deny_with_sibling_sandbox)
+{
+ /* parent and child sandbox */
+ check_ptrace(_metadata, 0, 1, 1, -1);
+}
+
+TEST_HARNESS_MAIN
--
2.22.0
^ permalink raw reply related
* [PATCH bpf-next v10 02/10] bpf: Add expected_attach_triggers and a is_valid_triggers() verifier
From: Mickaël Salaün @ 2019-07-21 21:31 UTC (permalink / raw)
To: linux-kernel
Cc: Mickaël Salaün, Alexander Viro, Alexei Starovoitov,
Andrew Morton, Andy Lutomirski, Arnaldo Carvalho de Melo,
Casey Schaufler, Daniel Borkmann, David Drysdale,
David S . Miller, Eric W . Biederman, James Morris, Jann Horn,
John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
Mickaël Salaün, Paul Moore, Sargun Dhillon,
Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
Tetsuo Handa, Thomas Graf, Tycho Andersen, Will Drewry,
kernel-hardening, linux-api, linux-fsdevel, linux-security-module,
netdev
In-Reply-To: <20190721213116.23476-1-mic@digikod.net>
The goal of the program triggers is to be able to have static triggers
(bitflags) conditionning an eBPF program interpretation. This help to
avoid unnecessary runs.
The struct bpf_verifier_ops gets a new optional function:
is_valid_verifier(). This new verifier is called at the beginning of the
eBPF program verification to check if the (optional) program triggers
are valid.
For now, only Landlock eBPF programs are using program triggers (see
next commits) but this could be used by other program types in the
future.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Link: https://lkml.kernel.org/r/20160827205559.GA43880@ast-mbp.thefacebook.com
---
Changes since v9:
* replace subtype with expected_attach_type (suggested by Alexei
Starovoitov) and a new expected_attach_triggers
* add new bpf_attach_type: BPF_LANDLOCK_FS_PICK and BPF_LANDLOCK_FS_WALK
* remove bpf_prog_extra from bpf_base_func_proto()
* update libbpf and test_verifier to handle triggers
Changes since v8:
* use bpf_load_program_xattr() instead of bpf_load_program() and add
bpf_verify_program_xattr() to deal with subtypes
* remove put_extra() since there is no more "previous" field (for now)
Changes since v7:
* rename LANDLOCK_SUBTYPE_* to LANDLOCK_*
* move subtype in bpf_prog_aux and use only one bit for has_subtype
(suggested by Alexei Starovoitov)
* wrap the prog_subtype with a prog_extra to be able to reference kernel
pointers:
* add an optional put_extra() function to struct bpf_prog_ops to be
able to free the pointed data
* replace all the prog_subtype with prog_extra in the struct
bpf_verifier_ops functions
* remove the ABI field (requested by Alexei Starovoitov)
* rename subtype fields
Changes since v6:
* rename Landlock version to ABI to better reflect its purpose
* fix unsigned integer checks
* fix pointer cast
* constify pointers
* rebase
Changes since v5:
* use a prog_subtype pointer and make it future-proof
* add subtype test
* constify bpf_load_program()'s subtype argument
* cleanup subtype initialization
* rebase
Changes since v4:
* replace the "status" field with "version" (more generic)
* replace the "access" field with "ability" (less confusing)
Changes since v3:
* remove the "origin" field
* add an "option" field
* cleanup comments
---
include/linux/bpf.h | 2 ++
include/linux/bpf_types.h | 3 +++
include/uapi/linux/bpf.h | 3 +++
kernel/bpf/syscall.c | 14 +++++++++++++-
kernel/bpf/verifier.c | 11 +++++++++++
tools/include/uapi/linux/bpf.h | 3 +++
tools/lib/bpf/bpf.h | 1 +
tools/lib/bpf/libbpf.map | 1 +
8 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 18f4cc2c6acd..6d9c7a08713e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -319,6 +319,7 @@ struct bpf_verifier_ops {
const struct bpf_insn *src,
struct bpf_insn *dst,
struct bpf_prog *prog, u32 *target_size);
+ bool (*is_valid_triggers)(const struct bpf_prog *prog);
};
struct bpf_prog_offload_ops {
@@ -418,6 +419,7 @@ struct bpf_prog_aux {
struct work_struct work;
struct rcu_head rcu;
};
+ u64 expected_attach_triggers;
};
struct bpf_array {
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index eec5aeeeaf92..2ab647323f3a 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -38,6 +38,9 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_LIRC_MODE2, lirc_mode2)
#ifdef CONFIG_INET
BPF_PROG_TYPE(BPF_PROG_TYPE_SK_REUSEPORT, sk_reuseport)
#endif
+#ifdef CONFIG_SECURITY_LANDLOCK
+BPF_PROG_TYPE(BPF_PROG_TYPE_LANDLOCK_HOOK, landlock)
+#endif
BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 6f68438aa4ed..1664d260861b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -197,6 +197,8 @@ enum bpf_attach_type {
BPF_CGROUP_UDP6_RECVMSG,
BPF_CGROUP_GETSOCKOPT,
BPF_CGROUP_SETSOCKOPT,
+ BPF_LANDLOCK_FS_PICK,
+ BPF_LANDLOCK_FS_WALK,
__MAX_BPF_ATTACH_TYPE
};
@@ -412,6 +414,7 @@ union bpf_attr {
__u32 line_info_rec_size; /* userspace bpf_line_info size */
__aligned_u64 line_info; /* line info */
__u32 line_info_cnt; /* number of bpf_line_info records */
+ __aligned_u64 expected_attach_triggers; /* bitfield of triggers, e.g. LANDLOCK_TRIGGER_* */
};
struct { /* anonymous struct used by BPF_OBJ_* commands */
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 5d141f16f6fa..b2a8cb14f28e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1598,13 +1598,23 @@ bpf_prog_load_check_attach_type(enum bpf_prog_type prog_type,
default:
return -EINVAL;
}
+#ifdef CONFIG_SECURITY_LANDLOCK
+ case BPF_PROG_TYPE_LANDLOCK_HOOK:
+ switch (expected_attach_type) {
+ case BPF_LANDLOCK_FS_PICK:
+ case BPF_LANDLOCK_FS_WALK:
+ return 0;
+ default:
+ return -EINVAL;
+ }
+#endif
default:
return 0;
}
}
/* last field in 'union bpf_attr' used by this command */
-#define BPF_PROG_LOAD_LAST_FIELD line_info_cnt
+#define BPF_PROG_LOAD_LAST_FIELD expected_attach_triggers
static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
{
@@ -1694,6 +1704,8 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
if (err)
goto free_prog;
+ prog->aux->expected_attach_triggers = attr->expected_attach_triggers;
+
/* run eBPF verifier */
err = bpf_check(&prog, attr, uattr);
if (err < 0)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a2e763703c30..94a43d7c8175 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9265,6 +9265,17 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
if (ret < 0)
goto skip_full_check;
+ if (env->ops->is_valid_triggers) {
+ if (!env->ops->is_valid_triggers(env->prog)) {
+ ret = -EINVAL;
+ goto err_unlock;
+ }
+ } else if (env->prog->aux->expected_attach_triggers) {
+ /* do not accept triggers if they are not handled */
+ ret = -EINVAL;
+ goto err_unlock;
+ }
+
if (bpf_prog_is_dev_bound(env->prog->aux)) {
ret = bpf_prog_offload_verifier_prep(env->prog);
if (ret)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index f506c68b2612..232747393405 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -197,6 +197,8 @@ enum bpf_attach_type {
BPF_CGROUP_UDP6_RECVMSG,
BPF_CGROUP_GETSOCKOPT,
BPF_CGROUP_SETSOCKOPT,
+ BPF_LANDLOCK_FS_PICK,
+ BPF_LANDLOCK_FS_WALK,
__MAX_BPF_ATTACH_TYPE
};
@@ -412,6 +414,7 @@ union bpf_attr {
__u32 line_info_rec_size; /* userspace bpf_line_info size */
__aligned_u64 line_info; /* line info */
__u32 line_info_cnt; /* number of bpf_line_info records */
+ __aligned_u64 expected_attach_triggers; /* bitfield of triggers, e.g. LANDLOCK_TRIGGER_* */
};
struct { /* anonymous struct used by BPF_OBJ_* commands */
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index ff42ca043dc8..468bb3ac0be0 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -102,6 +102,7 @@ LIBBPF_API int bpf_load_program(enum bpf_prog_type type,
const struct bpf_insn *insns, size_t insns_cnt,
const char *license, __u32 kern_version,
char *log_buf, size_t log_buf_sz);
+LIBBPF_API int bpf_verify_program_xattr(union bpf_attr *attr, size_t attr_sz);
LIBBPF_API int bpf_verify_program(enum bpf_prog_type type,
const struct bpf_insn *insns,
size_t insns_cnt, __u32 prog_flags,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index f9d316e873d8..36ac26bdfda0 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -107,6 +107,7 @@ LIBBPF_0.0.1 {
bpf_set_link_xdp_fd;
bpf_task_fd_query;
bpf_verify_program;
+ bpf_verify_program_xattr;
btf__fd;
btf__find_by_name;
btf__free;
--
2.22.0
^ permalink raw reply related
* [PATCH bpf-next v10 04/10] seccomp,landlock: Enforce Landlock programs per process hierarchy
From: Mickaël Salaün @ 2019-07-21 21:31 UTC (permalink / raw)
To: linux-kernel
Cc: Mickaël Salaün, Alexander Viro, Alexei Starovoitov,
Andrew Morton, Andy Lutomirski, Arnaldo Carvalho de Melo,
Casey Schaufler, Daniel Borkmann, David Drysdale,
David S . Miller, Eric W . Biederman, James Morris, Jann Horn,
John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
Mickaël Salaün, Paul Moore, Sargun Dhillon,
Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
Tetsuo Handa, Thomas Graf, Tycho Andersen, Will Drewry,
kernel-hardening, linux-api, linux-fsdevel, linux-security-module,
netdev
In-Reply-To: <20190721213116.23476-1-mic@digikod.net>
The seccomp(2) syscall can be used by a task to apply a Landlock program
to itself. As a seccomp filter, a Landlock program is enforced for the
current task and all its future children. A program is immutable and a
task can only add new restricting programs to itself, forming a list of
programss.
A Landlock program is tied to a Landlock hook. If the action on a kernel
object is allowed by the other Linux security mechanisms (e.g. DAC,
capabilities, other LSM), then a Landlock hook related to this kind of
object is triggered. The list of programs for this hook is then
evaluated. Each program return a binary value which can deny the action
on a kernel object with a non-zero value. If every programs of the list
return zero, then the action on the object is allowed.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Cc: Will Drewry <wad@chromium.org>
Link: https://lkml.kernel.org/r/c10a503d-5e35-7785-2f3d-25ed8dd63fab@digikod.net
---
Changes since v9:
* replace subtype with expected_attach_type and expected_attach_triggers
Changes since v8:
* Remove the chaining concept from the eBPF program contexts (chain and
cookie). We need to keep these subtypes this way to be able to make
them evolve, though.
Changes since v7:
* handle and verify program chains
* split and rename providers.c to enforce.c and enforce_seccomp.c
* rename LANDLOCK_SUBTYPE_* to LANDLOCK_*
Changes since v6:
* rename some functions with more accurate names to reflect that an eBPF
program for Landlock could be used for something else than a rule
* reword rule "appending" to "prepending" and explain it
* remove the superfluous no_new_privs check, only check global
CAP_SYS_ADMIN when prepending a Landlock rule (needed for containers)
* create and use {get,put}_seccomp_landlock() (suggested by Kees Cook)
* replace ifdef with static inlined function (suggested by Kees Cook)
* use get_user() (suggested by Kees Cook)
* replace atomic_t with refcount_t (requested by Kees Cook)
* move struct landlock_{rule,events} from landlock.h to common.h
* cleanup headers
Changes since v5:
* remove struct landlock_node and use a similar inheritance mechanisme
as seccomp-bpf (requested by Andy Lutomirski)
* rename SECCOMP_ADD_LANDLOCK_RULE to SECCOMP_APPEND_LANDLOCK_RULE
* rename file manager.c to providers.c
* add comments
* typo and cosmetic fixes
Changes since v4:
* merge manager and seccomp patches
* return -EFAULT in seccomp(2) when user_bpf_fd is null to easely check
if Landlock is supported
* only allow a process with the global CAP_SYS_ADMIN to use Landlock
(will be lifted in the future)
* add an early check to exit as soon as possible if the current process
does not have Landlock rules
Changes since v3:
* remove the hard link with seccomp (suggested by Andy Lutomirski and
Kees Cook):
* remove the cookie which could imply multiple evaluation of Landlock
rules
* remove the origin field in struct landlock_data
* remove documentation fix (merged upstream)
* rename the new seccomp command to SECCOMP_ADD_LANDLOCK_RULE
* internal renaming
* split commit
* new design to be able to inherit on the fly the parent rules
Changes since v2:
* Landlock programs can now be run without seccomp filter but for any
syscall (from the process) or interruption
* move Landlock related functions and structs into security/landlock/*
(to manage cgroups as well)
* fix seccomp filter handling: run Landlock programs for each of their
legitimate seccomp filter
* properly clean up all seccomp results
* cosmetic changes to ease the understanding
* fix some ifdef
---
include/linux/landlock.h | 34 ++++
include/linux/seccomp.h | 5 +
include/uapi/linux/seccomp.h | 1 +
kernel/fork.c | 8 +-
kernel/seccomp.c | 4 +
security/landlock/Makefile | 3 +-
security/landlock/common.h | 38 ++++
security/landlock/enforce.c | 272 ++++++++++++++++++++++++++++
security/landlock/enforce.h | 18 ++
security/landlock/enforce_seccomp.c | 92 ++++++++++
10 files changed, 473 insertions(+), 2 deletions(-)
create mode 100644 include/linux/landlock.h
create mode 100644 security/landlock/enforce.c
create mode 100644 security/landlock/enforce.h
create mode 100644 security/landlock/enforce_seccomp.c
diff --git a/include/linux/landlock.h b/include/linux/landlock.h
new file mode 100644
index 000000000000..8ac7942f50fc
--- /dev/null
+++ b/include/linux/landlock.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Landlock LSM - public kernel headers
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#ifndef _LINUX_LANDLOCK_H
+#define _LINUX_LANDLOCK_H
+
+#include <linux/errno.h>
+#include <linux/sched.h> /* task_struct */
+
+#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_SECURITY_LANDLOCK)
+extern int landlock_seccomp_prepend_prog(unsigned int flags,
+ const int __user *user_bpf_fd);
+extern void put_seccomp_landlock(struct task_struct *tsk);
+extern void get_seccomp_landlock(struct task_struct *tsk);
+#else /* CONFIG_SECCOMP_FILTER && CONFIG_SECURITY_LANDLOCK */
+static inline int landlock_seccomp_prepend_prog(unsigned int flags,
+ const int __user *user_bpf_fd)
+{
+ return -EINVAL;
+}
+static inline void put_seccomp_landlock(struct task_struct *tsk)
+{
+}
+static inline void get_seccomp_landlock(struct task_struct *tsk)
+{
+}
+#endif /* CONFIG_SECCOMP_FILTER && CONFIG_SECURITY_LANDLOCK */
+
+#endif /* _LINUX_LANDLOCK_H */
diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
index 84868d37b35d..106a0ceff3d7 100644
--- a/include/linux/seccomp.h
+++ b/include/linux/seccomp.h
@@ -11,6 +11,7 @@
#ifdef CONFIG_SECCOMP
+#include <linux/landlock.h>
#include <linux/thread_info.h>
#include <asm/seccomp.h>
@@ -22,6 +23,7 @@ struct seccomp_filter;
* system calls available to a process.
* @filter: must always point to a valid seccomp-filter or NULL as it is
* accessed without locking during system call entry.
+ * @landlock_prog_set: contains a set of Landlock programs.
*
* @filter must only be accessed from the context of current as there
* is no read locking.
@@ -29,6 +31,9 @@ struct seccomp_filter;
struct seccomp {
int mode;
struct seccomp_filter *filter;
+#if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_SECURITY_LANDLOCK)
+ struct landlock_prog_set *landlock_prog_set;
+#endif /* CONFIG_SECCOMP_FILTER && CONFIG_SECURITY_LANDLOCK */
};
#ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER
diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index 90734aa5aa36..bce6534e7feb 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -16,6 +16,7 @@
#define SECCOMP_SET_MODE_FILTER 1
#define SECCOMP_GET_ACTION_AVAIL 2
#define SECCOMP_GET_NOTIF_SIZES 3
+#define SECCOMP_PREPEND_LANDLOCK_PROG 4
/* Valid flags for SECCOMP_SET_MODE_FILTER */
#define SECCOMP_FILTER_FLAG_TSYNC (1UL << 0)
diff --git a/kernel/fork.c b/kernel/fork.c
index 8f3e2d97d771..6c43517abdb9 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -51,6 +51,7 @@
#include <linux/security.h>
#include <linux/hugetlb.h>
#include <linux/seccomp.h>
+#include <linux/landlock.h>
#include <linux/swap.h>
#include <linux/syscalls.h>
#include <linux/jiffies.h>
@@ -458,6 +459,7 @@ void free_task(struct task_struct *tsk)
rt_mutex_debug_task_free(tsk);
ftrace_graph_exit_task(tsk);
put_seccomp_filter(tsk);
+ put_seccomp_landlock(tsk);
arch_release_task_struct(tsk);
if (tsk->flags & PF_KTHREAD)
free_kthread_struct(tsk);
@@ -888,7 +890,10 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
* the usage counts on the error path calling free_task.
*/
tsk->seccomp.filter = NULL;
-#endif
+#ifdef CONFIG_SECURITY_LANDLOCK
+ tsk->seccomp.landlock_prog_set = NULL;
+#endif /* CONFIG_SECURITY_LANDLOCK */
+#endif /* CONFIG_SECCOMP */
setup_thread_stack(tsk, orig);
clear_user_return_notifier(tsk);
@@ -1604,6 +1609,7 @@ static void copy_seccomp(struct task_struct *p)
/* Ref-count the new filter user, and assign it. */
get_seccomp_filter(current);
+ get_seccomp_landlock(current);
p->seccomp = current->seccomp;
/*
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index dba52a7db5e8..af542a2d21e7 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -41,6 +41,7 @@
#include <linux/tracehook.h>
#include <linux/uaccess.h>
#include <linux/anon_inodes.h>
+#include <linux/landlock.h>
enum notify_state {
SECCOMP_NOTIFY_INIT,
@@ -1397,6 +1398,9 @@ static long do_seccomp(unsigned int op, unsigned int flags,
return -EINVAL;
return seccomp_get_notif_sizes(uargs);
+ case SECCOMP_PREPEND_LANDLOCK_PROG:
+ return landlock_seccomp_prepend_prog(flags,
+ (const int __user *)uargs);
default:
return -EINVAL;
}
diff --git a/security/landlock/Makefile b/security/landlock/Makefile
index 7205f9a7a2ee..2a1a7082a365 100644
--- a/security/landlock/Makefile
+++ b/security/landlock/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
-landlock-y := init.o
+landlock-y := init.o \
+ enforce.o enforce_seccomp.o
diff --git a/security/landlock/common.h b/security/landlock/common.h
index 80dc36f4d0ac..2cf36dbf4560 100644
--- a/security/landlock/common.h
+++ b/security/landlock/common.h
@@ -28,6 +28,44 @@ enum landlock_hook_type {
LANDLOCK_HOOK_FS_WALK,
};
+struct landlock_prog_list {
+ struct landlock_prog_list *prev;
+ struct bpf_prog *prog;
+ refcount_t usage;
+};
+
+/**
+ * struct landlock_prog_set - Landlock programs enforced on a thread
+ *
+ * This is used for low performance impact when forking a process. Instead of
+ * copying the full array and incrementing the usage of each entries, only
+ * create a pointer to &struct landlock_prog_set and increments its usage. When
+ * prepending a new program, if &struct landlock_prog_set is shared with other
+ * tasks, then duplicate it and prepend the program to this new &struct
+ * landlock_prog_set.
+ *
+ * @usage: reference count to manage the object lifetime. When a thread need to
+ * add Landlock programs and if @usage is greater than 1, then the
+ * thread must duplicate &struct landlock_prog_set to not change the
+ * children's programs as well.
+ * @programs: array of non-NULL &struct landlock_prog_list pointers
+ */
+struct landlock_prog_set {
+ struct landlock_prog_list *programs[_LANDLOCK_HOOK_LAST];
+ refcount_t usage;
+};
+
+/**
+ * get_hook_index - get an index for the programs of struct landlock_prog_set
+ *
+ * @type: a Landlock hook type
+ */
+static inline int get_hook_index(enum landlock_hook_type type)
+{
+ /* type ID > 0 for loaded programs */
+ return type - 1;
+}
+
static inline enum landlock_hook_type get_hook_type(const struct bpf_prog *prog)
{
switch (prog->expected_attach_type) {
diff --git a/security/landlock/enforce.c b/security/landlock/enforce.c
new file mode 100644
index 000000000000..b6979de69d3e
--- /dev/null
+++ b/security/landlock/enforce.c
@@ -0,0 +1,272 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - enforcing helpers
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#include <asm/barrier.h> /* smp_store_release() */
+#include <asm/page.h> /* PAGE_SIZE */
+#include <linux/bpf.h> /* bpf_prog_put() */
+#include <linux/compiler.h> /* READ_ONCE() */
+#include <linux/err.h> /* PTR_ERR() */
+#include <linux/errno.h>
+#include <linux/filter.h> /* struct bpf_prog */
+#include <linux/refcount.h>
+#include <linux/slab.h> /* alloc(), kfree() */
+
+#include "common.h" /* struct landlock_prog_list */
+
+/* TODO: use a dedicated kmem_cache_alloc() instead of k*alloc() */
+
+static void put_landlock_prog_list(struct landlock_prog_list *prog_list)
+{
+ struct landlock_prog_list *orig = prog_list;
+
+ /* clean up single-reference branches iteratively */
+ while (orig && refcount_dec_and_test(&orig->usage)) {
+ struct landlock_prog_list *freeme = orig;
+
+ if (orig->prog)
+ bpf_prog_put(orig->prog);
+ orig = orig->prev;
+ kfree(freeme);
+ }
+}
+
+void landlock_put_prog_set(struct landlock_prog_set *prog_set)
+{
+ if (prog_set && refcount_dec_and_test(&prog_set->usage)) {
+ size_t i;
+
+ for (i = 0; i < ARRAY_SIZE(prog_set->programs); i++)
+ put_landlock_prog_list(prog_set->programs[i]);
+ kfree(prog_set);
+ }
+}
+
+void landlock_get_prog_set(struct landlock_prog_set *prog_set)
+{
+ if (!prog_set)
+ return;
+ refcount_inc(&prog_set->usage);
+}
+
+static struct landlock_prog_set *new_landlock_prog_set(void)
+{
+ struct landlock_prog_set *ret;
+
+ /* array filled with NULL values */
+ ret = kzalloc(sizeof(*ret), GFP_KERNEL);
+ if (!ret)
+ return ERR_PTR(-ENOMEM);
+ refcount_set(&ret->usage, 1);
+ return ret;
+}
+
+/**
+ * store_landlock_prog - prepend and deduplicate a Landlock prog_list
+ *
+ * Prepend @prog to @init_prog_set while ignoring @prog
+ * if they are already in @ref_prog_set. Whatever is the result of this
+ * function call, you can call bpf_prog_put(@prog) after.
+ *
+ * @init_prog_set: empty prog_set to prepend to
+ * @ref_prog_set: prog_set to check for duplicate programs
+ * @prog: program to prepend
+ *
+ * Return -errno on error or 0 if @prog was successfully stored.
+ */
+static int store_landlock_prog(struct landlock_prog_set *init_prog_set,
+ const struct landlock_prog_set *ref_prog_set,
+ struct bpf_prog *prog)
+{
+ struct landlock_prog_list *tmp_list = NULL;
+ int err;
+ u32 hook_idx;
+ enum landlock_hook_type last_type;
+ struct bpf_prog *new = prog;
+
+ /* allocate all the memory we need */
+ struct landlock_prog_list *new_list;
+
+ last_type = get_hook_type(new);
+
+ /* ignore duplicate programs */
+ if (ref_prog_set) {
+ struct landlock_prog_list *ref;
+
+ hook_idx = get_hook_index(get_hook_type(new));
+ for (ref = ref_prog_set->programs[hook_idx];
+ ref; ref = ref->prev) {
+ if (ref->prog == new)
+ return -EINVAL;
+ }
+ }
+
+ new = bpf_prog_inc(new);
+ if (IS_ERR(new)) {
+ err = PTR_ERR(new);
+ goto put_tmp_list;
+ }
+ new_list = kzalloc(sizeof(*new_list), GFP_KERNEL);
+ if (!new_list) {
+ bpf_prog_put(new);
+ err = -ENOMEM;
+ goto put_tmp_list;
+ }
+ /* ignore Landlock types in this tmp_list */
+ new_list->prog = new;
+ new_list->prev = tmp_list;
+ refcount_set(&new_list->usage, 1);
+ tmp_list = new_list;
+
+ if (!tmp_list)
+ /* inform user space that this program was already added */
+ return -EEXIST;
+
+ /* properly store the list (without error cases) */
+ while (tmp_list) {
+ struct landlock_prog_list *new_list;
+
+ new_list = tmp_list;
+ tmp_list = tmp_list->prev;
+ /* do not increment the previous prog list usage */
+ hook_idx = get_hook_index(get_hook_type(new_list->prog));
+ new_list->prev = init_prog_set->programs[hook_idx];
+ /* no need to add from the last program to the first because
+ * each of them are a different Landlock type */
+ smp_store_release(&init_prog_set->programs[hook_idx], new_list);
+ }
+ return 0;
+
+put_tmp_list:
+ put_landlock_prog_list(tmp_list);
+ return err;
+}
+
+/* limit Landlock programs set to 256KB */
+#define LANDLOCK_PROGRAMS_MAX_PAGES (1 << 6)
+
+/**
+ * landlock_prepend_prog - attach a Landlock prog_list to @current_prog_set
+ *
+ * Whatever is the result of this function call, you can call
+ * bpf_prog_put(@prog) after.
+ *
+ * @current_prog_set: landlock_prog_set pointer, must be locked (if needed) to
+ * prevent a concurrent put/free. This pointer must not be
+ * freed after the call.
+ * @prog: non-NULL Landlock prog_list to prepend to @current_prog_set. @prog
+ * will be owned by landlock_prepend_prog() and freed if an error
+ * happened.
+ *
+ * Return @current_prog_set or a new pointer when OK. Return a pointer error
+ * otherwise.
+ */
+struct landlock_prog_set *landlock_prepend_prog(
+ struct landlock_prog_set *current_prog_set,
+ struct bpf_prog *prog)
+{
+ struct landlock_prog_set *new_prog_set = current_prog_set;
+ unsigned long pages;
+ int err;
+ size_t i;
+ struct landlock_prog_set tmp_prog_set = {};
+
+ if (prog->type != BPF_PROG_TYPE_LANDLOCK_HOOK)
+ return ERR_PTR(-EINVAL);
+
+ /* validate memory size allocation */
+ pages = prog->pages;
+ if (current_prog_set) {
+ size_t i;
+
+ for (i = 0; i < ARRAY_SIZE(current_prog_set->programs); i++) {
+ struct landlock_prog_list *walker_p;
+
+ for (walker_p = current_prog_set->programs[i];
+ walker_p; walker_p = walker_p->prev)
+ pages += walker_p->prog->pages;
+ }
+ /* count a struct landlock_prog_set if we need to allocate one */
+ if (refcount_read(¤t_prog_set->usage) != 1)
+ pages += round_up(sizeof(*current_prog_set), PAGE_SIZE)
+ / PAGE_SIZE;
+ }
+ if (pages > LANDLOCK_PROGRAMS_MAX_PAGES)
+ return ERR_PTR(-E2BIG);
+
+ /* ensure early that we can allocate enough memory for the new
+ * prog_lists */
+ err = store_landlock_prog(&tmp_prog_set, current_prog_set, prog);
+ if (err)
+ return ERR_PTR(err);
+
+ /*
+ * Each task_struct points to an array of prog list pointers. These
+ * tables are duplicated when additions are made (which means each
+ * table needs to be refcounted for the processes using it). When a new
+ * table is created, all the refcounters on the prog_list are bumped (to
+ * track each table that references the prog). When a new prog is
+ * added, it's just prepended to the list for the new table to point
+ * at.
+ *
+ * Manage all the possible errors before this step to not uselessly
+ * duplicate current_prog_set and avoid a rollback.
+ */
+ if (!new_prog_set) {
+ /*
+ * If there is no Landlock program set used by the current task,
+ * then create a new one.
+ */
+ new_prog_set = new_landlock_prog_set();
+ if (IS_ERR(new_prog_set))
+ goto put_tmp_lists;
+ } else if (refcount_read(¤t_prog_set->usage) > 1) {
+ /*
+ * If the current task is not the sole user of its Landlock
+ * program set, then duplicate them.
+ */
+ new_prog_set = new_landlock_prog_set();
+ if (IS_ERR(new_prog_set))
+ goto put_tmp_lists;
+ for (i = 0; i < ARRAY_SIZE(new_prog_set->programs); i++) {
+ new_prog_set->programs[i] =
+ READ_ONCE(current_prog_set->programs[i]);
+ if (new_prog_set->programs[i])
+ refcount_inc(&new_prog_set->programs[i]->usage);
+ }
+
+ /*
+ * Landlock program set from the current task will not be freed
+ * here because the usage is strictly greater than 1. It is
+ * only prevented to be freed by another task thanks to the
+ * caller of landlock_prepend_prog() which should be locked if
+ * needed.
+ */
+ landlock_put_prog_set(current_prog_set);
+ }
+
+ /* prepend tmp_prog_set to new_prog_set */
+ for (i = 0; i < ARRAY_SIZE(tmp_prog_set.programs); i++) {
+ /* get the last new list */
+ struct landlock_prog_list *last_list =
+ tmp_prog_set.programs[i];
+
+ if (last_list) {
+ while (last_list->prev)
+ last_list = last_list->prev;
+ /* no need to increment usage (pointer replacement) */
+ last_list->prev = new_prog_set->programs[i];
+ new_prog_set->programs[i] = tmp_prog_set.programs[i];
+ }
+ }
+ return new_prog_set;
+
+put_tmp_lists:
+ for (i = 0; i < ARRAY_SIZE(tmp_prog_set.programs); i++)
+ put_landlock_prog_list(tmp_prog_set.programs[i]);
+ return new_prog_set;
+}
diff --git a/security/landlock/enforce.h b/security/landlock/enforce.h
new file mode 100644
index 000000000000..39b800d9999f
--- /dev/null
+++ b/security/landlock/enforce.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - enforcing helpers headers
+ *
+ * Copyright © 2016-2019 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#ifndef _SECURITY_LANDLOCK_ENFORCE_H
+#define _SECURITY_LANDLOCK_ENFORCE_H
+
+struct landlock_prog_set *landlock_prepend_prog(
+ struct landlock_prog_set *current_prog_set,
+ struct bpf_prog *prog);
+void landlock_put_prog_set(struct landlock_prog_set *prog_set);
+void landlock_get_prog_set(struct landlock_prog_set *prog_set);
+
+#endif /* _SECURITY_LANDLOCK_ENFORCE_H */
diff --git a/security/landlock/enforce_seccomp.c b/security/landlock/enforce_seccomp.c
new file mode 100644
index 000000000000..c38c81e6b01a
--- /dev/null
+++ b/security/landlock/enforce_seccomp.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - enforcing with seccomp
+ *
+ * Copyright © 2016-2018 Mickaël Salaün <mic@digikod.net>
+ * Copyright © 2018-2019 ANSSI
+ */
+
+#ifdef CONFIG_SECCOMP_FILTER
+
+#include <linux/bpf.h> /* bpf_prog_put() */
+#include <linux/capability.h>
+#include <linux/err.h> /* PTR_ERR() */
+#include <linux/errno.h>
+#include <linux/filter.h> /* struct bpf_prog */
+#include <linux/landlock.h>
+#include <linux/refcount.h>
+#include <linux/sched.h> /* current */
+#include <linux/uaccess.h> /* get_user() */
+
+#include "enforce.h"
+
+/* headers in include/linux/landlock.h */
+
+/**
+ * landlock_seccomp_prepend_prog - attach a Landlock program to the current
+ * process
+ *
+ * current->seccomp.landlock_state->prog_set is lazily allocated. When a
+ * process fork, only a pointer is copied. When a new program is added by a
+ * process, if there is other references to this process' prog_set, then a new
+ * allocation is made to contain an array pointing to Landlock program lists.
+ * This design enable low-performance impact and is memory efficient while
+ * keeping the property of prepend-only programs.
+ *
+ * For now, installing a Landlock prog requires that the requesting task has
+ * the global CAP_SYS_ADMIN. We cannot force the use of no_new_privs to not
+ * exclude containers where a process may legitimately acquire more privileges
+ * thanks to an SUID binary.
+ *
+ * @flags: not used for now, but could be used for TSYNC
+ * @user_bpf_fd: file descriptor pointing to a loaded Landlock prog
+ */
+int landlock_seccomp_prepend_prog(unsigned int flags,
+ const int __user *user_bpf_fd)
+{
+ struct landlock_prog_set *new_prog_set;
+ struct bpf_prog *prog;
+ int bpf_fd, err;
+
+ /* planned to be replaced with a no_new_privs check to allow
+ * unprivileged tasks */
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+ /* enable to check if Landlock is supported with early EFAULT */
+ if (!user_bpf_fd)
+ return -EFAULT;
+ if (flags)
+ return -EINVAL;
+ err = get_user(bpf_fd, user_bpf_fd);
+ if (err)
+ return err;
+
+ prog = bpf_prog_get(bpf_fd);
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
+
+ /*
+ * We don't need to lock anything for the current process hierarchy,
+ * everything is guarded by the atomic counters.
+ */
+ new_prog_set = landlock_prepend_prog(
+ current->seccomp.landlock_prog_set, prog);
+ bpf_prog_put(prog);
+ /* @prog is managed/freed by landlock_prepend_prog() */
+ if (IS_ERR(new_prog_set))
+ return PTR_ERR(new_prog_set);
+ current->seccomp.landlock_prog_set = new_prog_set;
+ return 0;
+}
+
+void put_seccomp_landlock(struct task_struct *tsk)
+{
+ landlock_put_prog_set(tsk->seccomp.landlock_prog_set);
+}
+
+void get_seccomp_landlock(struct task_struct *tsk)
+{
+ landlock_get_prog_set(tsk->seccomp.landlock_prog_set);
+}
+
+#endif /* CONFIG_SECCOMP_FILTER */
--
2.22.0
^ permalink raw reply related
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