* Re: [PATCH] Smack: Restore the smackfsdef mount option and add missing prefixes
From: David Howells @ 2019-05-31 10:56 UTC (permalink / raw)
To: viro
Cc: dhowells, stable, Jose Bollo, Casey Schaufler, jmorris, torvalds,
linux-security-module, linux-kernel
In-Reply-To: <155930001303.17253.2447519598157285098.stgit@warthog.procyon.org.uk>
Should this go via Al's tree, James's tree, Casey's tree or directly to Linus?
David
^ permalink raw reply
* Re: [PATCH 1/7] General notification queue with user mmap()'able ring buffer
From: Peter Zijlstra @ 2019-05-31 11:14 UTC (permalink / raw)
To: David Howells
Cc: Jann Horn, Greg KH, Al Viro, raven, linux-fsdevel, Linux API,
linux-block, keyrings, linux-security-module, kernel list,
Kees Cook, Kernel Hardening
In-Reply-To: <16193.1559163763@warthog.procyon.org.uk>
On Wed, May 29, 2019 at 10:02:43PM +0100, David Howells wrote:
> Jann Horn <jannh@google.com> wrote:
>
> > Does this mean that refcount_read() isn't sufficient for what you want
> > to do with tracing (because for some reason you actually need to know
> > the values atomically at the time of increment/decrement)?
>
> Correct. There's a gap and if an interrupt or something occurs, it's
> sufficiently big for the refcount trace to go weird.
>
> I've seen it in afs/rxrpc where the incoming network packets that are part of
> the rxrpc call flow disrupt the refcounts noted in trace lines.
Can you re-iterate the exact problem? I konw we talked about this in the
past, but I seem to have misplaced those memories :/
FWIW I agree that kref is useless fluff, but I've long ago given up on
that fight.
^ permalink raw reply
* Re: sleep in selinux_audit_rule_init
From: Janne Karhunen @ 2019-05-31 11:22 UTC (permalink / raw)
To: Stephen Smalley
Cc: Mimi Zohar, Paul Moore, linux-integrity, linux-security-module,
Dan Jurgens
In-Reply-To: <54d804d0-25a6-2f5c-8fc9-0c671e34b8eb@tycho.nsa.gov>
On Thu, May 30, 2019 at 5:17 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> >> Both of those issues also exist prior to your
> >> patch but you aren't fixing them here. And lastly, it looks like lsm
> >> notifiers are atomic notifiers (not clear to me why) so you can't block
> >> in the callback, thereby requiring scheduling the work as is done in
> >> infiniband.
> >
> > Great catch, thank you. That's an easy fix if no-one objects pushing
> > these through the system-wq for example.
>
> I think you can switch the lsm notifier over to using blocking notifiers
> instead; there seems to be no valid reason for making it atomic.
Further drafting. I certainly feel uneasy with the rcu update side of
it, but the string is not used in the matching so.. ?
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d213e835c498..2203451862d4 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -154,6 +154,8 @@ unsigned long ima_get_binary_runtime_size(void);
int ima_init_template(void);
void ima_init_template_list(void);
int __init ima_init_digests(void);
+int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
+ void *lsm_data);
/*
* used to protect h_table and sha_table
diff --git a/security/integrity/ima/ima_main.c
b/security/integrity/ima/ima_main.c
index 5749ec92516f..449502f5c3dc 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -52,6 +52,10 @@ int ima_hash_algo = HASH_ALGO_SHA1;
static int hash_setup_done;
static struct workqueue_struct *ima_update_wq;
+static struct notifier_block ima_lsm_policy_notifier = {
+ .notifier_call = ima_lsm_policy_change,
+};
+
static int __init hash_setup(char *str)
{
struct ima_template_desc *template_desc = ima_template_desc_current();
@@ -691,6 +695,10 @@ static int __init init_ima(void)
error = ima_init();
}
+ error = register_lsm_notifier(&ima_lsm_policy_notifier);
+ if (error)
+ pr_warn("Couldn't register LSM notifier, error %d\n", error);
+
if (!error)
ima_update_policy_flag();
else
diff --git a/security/integrity/ima/ima_policy.c
b/security/integrity/ima/ima_policy.c
index e0cc323f948f..6776dc2b9664 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -252,12 +252,14 @@ __setup("ima_appraise_tcb",
default_appraise_policy_setup);
/*
* The LSM policy can be reloaded, leaving the IMA LSM based rules referring
* to the old, stale LSM policy. Update the IMA LSM based rules to reflect
- * the reloaded LSM policy. We assume the rules still exist; and BUG_ON() if
- * they don't.
+ * the reloaded LSM policy.
*/
static void ima_lsm_update_rules(void)
{
struct ima_rule_entry *entry;
+ void *rule_new;
+ char *lsm_new;
+ char *lsm_old;
int result;
int i;
@@ -265,13 +267,37 @@ static void ima_lsm_update_rules(void)
for (i = 0; i < MAX_LSM_RULES; i++) {
if (!entry->lsm[i].rule)
continue;
+
+ lsm_old = entry->lsm[i].args_p;
+ lsm_new = kstrdup(lsm_old, GFP_KERNEL);
+ if (unlikely(!lsm_new))
+ return;
+
result = security_filter_rule_init(entry->lsm[i].type,
Audit_equal,
- entry->lsm[i].args_p,
- &entry->lsm[i].rule);
- BUG_ON(!entry->lsm[i].rule);
- }
- }
+ lsm_new,
+ &rule_new);
+ if (result == -EINVAL)
+ pr_warn("ima: rule for LSM \'%d\' is invalid\n",
+ entry->lsm[i].type);
+
+ entry->lsm[i].rule = rule_new;
+ entry->lsm[i].args_p = lsm_new;
+ synchronize_rcu();
+
+ kfree(lsm_old);
+ }
+ }
+}
+
+int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
+ void *lsm_data)
+{
+ if (event != LSM_POLICY_CHANGE)
+ return NOTIFY_DONE;
+
+ ima_lsm_update_rules();
+ return NOTIFY_OK;
}
/**
@@ -327,11 +353,10 @@ static bool ima_match_rules(struct
ima_rule_entry *rule, struct inode *inode,
for (i = 0; i < MAX_LSM_RULES; i++) {
int rc = 0;
u32 osid;
- int retried = 0;
if (!rule->lsm[i].rule)
continue;
-retry:
+
switch (i) {
case LSM_OBJ_USER:
case LSM_OBJ_ROLE:
@@ -352,11 +377,6 @@ static bool ima_match_rules(struct ima_rule_entry
*rule, struct inode *inode,
default:
break;
}
- if ((rc < 0) && (!retried)) {
- retried = 1;
- ima_lsm_update_rules();
- goto retry;
- }
if (!rc)
return false;
}
diff --git a/security/security.c b/security/security.c
index 23cbb1a295a3..c5e69ce81521 100644
--- a/security/security.c
+++ b/security/security.c
@@ -39,7 +39,7 @@
#define LSM_COUNT (__end_lsm_info - __start_lsm_info)
struct security_hook_heads security_hook_heads __lsm_ro_after_init;
-static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
+static BLOCKING_NOTIFIER_HEAD(lsm_notifier_chain);
static struct kmem_cache *lsm_file_cache;
static struct kmem_cache *lsm_inode_cache;
@@ -432,19 +432,19 @@ void __init security_add_hooks(struct
security_hook_list *hooks, int count,
int call_lsm_notifier(enum lsm_event event, void *data)
{
- return atomic_notifier_call_chain(&lsm_notifier_chain, event, data);
+ return blocking_notifier_call_chain(&lsm_notifier_chain, event, data);
}
EXPORT_SYMBOL(call_lsm_notifier);
int register_lsm_notifier(struct notifier_block *nb)
{
- return atomic_notifier_chain_register(&lsm_notifier_chain, nb);
+ return blocking_notifier_chain_register(&lsm_notifier_chain, nb);
}
EXPORT_SYMBOL(register_lsm_notifier);
int unregister_lsm_notifier(struct notifier_block *nb)
{
- return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb);
+ return blocking_notifier_chain_unregister(&lsm_notifier_chain, nb);
}
EXPORT_SYMBOL(unregister_lsm_notifier);
--
Janne
^ permalink raw reply related
* Re: [PATCH 1/7] General notification queue with user mmap()'able ring buffer
From: David Howells @ 2019-05-31 12:02 UTC (permalink / raw)
To: Peter Zijlstra
Cc: dhowells, Jann Horn, Greg KH, Al Viro, raven, linux-fsdevel,
Linux API, linux-block, keyrings, linux-security-module,
kernel list, Kees Cook, Kernel Hardening
In-Reply-To: <20190531111445.GO2677@hirez.programming.kicks-ass.net>
Peter Zijlstra <peterz@infradead.org> wrote:
> Can you re-iterate the exact problem? I konw we talked about this in the
> past, but I seem to have misplaced those memories :/
Take this for example:
void afs_put_call(struct afs_call *call)
{
struct afs_net *net = call->net;
int n = atomic_dec_return(&call->usage);
int o = atomic_read(&net->nr_outstanding_calls);
trace_afs_call(call, afs_call_trace_put, n + 1, o,
__builtin_return_address(0));
ASSERTCMP(n, >=, 0);
if (n == 0) {
...
}
}
I am printing the usage count in the afs_call tracepoint so that I can use it
to debug refcount bugs. If I do it like this:
void afs_put_call(struct afs_call *call)
{
int n = refcount_read(&call->usage);
int o = atomic_read(&net->nr_outstanding_calls);
trace_afs_call(call, afs_call_trace_put, n, o,
__builtin_return_address(0));
if (refcount_dec_and_test(&call->usage)) {
...
}
}
then there's a temporal gap between the usage count being read and the actual
atomic decrement in which another CPU can alter the count. This can be
exacerbated by an interrupt occurring, a softirq occurring or someone enabling
the tracepoint.
I can't do the tracepoint after the decrement if refcount_dec_and_test()
returns false unless I save all the values from the object that I might need
as the object could be destroyed any time from that point on. In this
particular case, that's just call->debug_id, but it could be other things in
other cases.
Note that I also can't touch the afs_net object in that situation either, and
the outstanding calls count that I record will potentially be out of date -
but there's not a lot I can do about that.
David
^ permalink raw reply
* Re: [PATCH 1/7] General notification queue with user mmap()'able ring buffer
From: David Howells @ 2019-05-31 12:42 UTC (permalink / raw)
To: Greg KH
Cc: dhowells, viro, raven, linux-fsdevel, linux-api, linux-block,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20190529230954.GA3164@kroah.com>
Greg KH <gregkh@linuxfoundation.org> wrote:
> > kref_put() enforces a very specific destructor signature. I know of places
> > where that doesn't work because the destructor takes more than one argument
> > (granted that this is not the case here). So why does kref_put() exist at
> > all? Why not kref_dec_and_test()?
>
> The destructor only takes one object pointer as you are finally freeing
> that object. What more do you need/want to "know" at that point in
> time?
Imagine that I have an object that's on a list rooted in a namespace and that
I have a lot of these objects. Imagine further that any time I want to put a
ref on one of these objects, it's in a context that has the namespace pinned.
I therefore don't need to store a pointer to the namespace in every object
because I can pass that in to the put function
Indeed, I can still access the namespace even after the decrement didn't
reduce the usage count to 0 - say for doing statistics.
> What would kref_dec_and_test() be needed for?
Why do you need kref_put() to take a destructor function pointer? Why cannot
that be replaced with, say:
static inline bool __kref_put(struct kref *k)
{
return refcount_dec_and_test(&k->refcount);
}
and then one could do:
void put_foo(struct foo_net *ns, struct foo *f)
{
if (__kref_put(&f->refcount)) {
// destroy foo
}
}
that way the destruction code does not have to be offloaded into its own
function and you still have your pattern to look for.
For tracing purposes, I could live with something like:
static inline
bool __kref_put_return(struct kref *k, unsigned int *_usage)
{
return refcount_dec_and_test_return(&k->refcount, _usage);
}
and then I could do:
void put_foo(struct foo_net *ns, struct foo *f)
{
unsigned int u;
bool is_zero = __kref_put_return(&f->refcount, &u);
trace_foo_refcount(f, u);
if (is_zero) {
// destroy foo
}
}
then it could be made such that you can disable the ability of
refcount_dec_and_test_return() to pass back a useful refcount value if you
want a bit of extra speed.
Or even if refcount_dec_return() is guaranteed to return 0 if the count hits
the floor and non-zero otherwise and there's a config switch to impose a
stronger guarantee that it will return a value that's appropriately
transformed to look as if I was using atomic_dec_return().
Similarly for refcount_inc_return() - it could just return gibberish unless
the same config switch is enabled.
Question for AMD/Intel guys: I'm curious if LOCK DECL faster than LOCK XADD -1
on x86_64?
> > Why doesn't refcount_t get merged into kref, or vice versa? Having both
> > would seem redundant.
>
> kref uses refcount_t and provides a different functionality on top of
> it. Not all uses of a refcount in the kernel is for object lifecycle
> reference counting, as you know :)
I do? I can't think of one offhand. Not that I'm saying you're wrong on
that - there's an awful lot of kernel.
David
^ permalink raw reply
* Re: [PATCH 1/7] General notification queue with user mmap()'able ring buffer
From: Peter Zijlstra @ 2019-05-31 13:26 UTC (permalink / raw)
To: David Howells
Cc: Jann Horn, Greg KH, Al Viro, raven, linux-fsdevel, Linux API,
linux-block, keyrings, linux-security-module, kernel list,
Kees Cook, Kernel Hardening
In-Reply-To: <21942.1559304135@warthog.procyon.org.uk>
On Fri, May 31, 2019 at 01:02:15PM +0100, David Howells wrote:
> Peter Zijlstra <peterz@infradead.org> wrote:
>
> > Can you re-iterate the exact problem? I konw we talked about this in the
> > past, but I seem to have misplaced those memories :/
>
> Take this for example:
>
> void afs_put_call(struct afs_call *call)
> {
> struct afs_net *net = call->net;
> int n = atomic_dec_return(&call->usage);
> int o = atomic_read(&net->nr_outstanding_calls);
>
> trace_afs_call(call, afs_call_trace_put, n + 1, o,
> __builtin_return_address(0));
>
> ASSERTCMP(n, >=, 0);
> if (n == 0) {
> ...
> }
> }
>
> I am printing the usage count in the afs_call tracepoint so that I can use it
> to debug refcount bugs. If I do it like this:
>
> void afs_put_call(struct afs_call *call)
> {
> int n = refcount_read(&call->usage);
> int o = atomic_read(&net->nr_outstanding_calls);
>
> trace_afs_call(call, afs_call_trace_put, n, o,
> __builtin_return_address(0));
>
> if (refcount_dec_and_test(&call->usage)) {
> ...
> }
> }
>
> then there's a temporal gap between the usage count being read and the actual
> atomic decrement in which another CPU can alter the count. This can be
> exacerbated by an interrupt occurring, a softirq occurring or someone enabling
> the tracepoint.
>
> I can't do the tracepoint after the decrement if refcount_dec_and_test()
> returns false unless I save all the values from the object that I might need
> as the object could be destroyed any time from that point on.
Is it not the responsibility of the task that affects the 1->0
transition to actually free the memory?
That is, I'm expecting the '...' in both cases above the include the
actual freeing of the object. If this is not the case, then @usage is
not a reference count.
(and it has already been established that refcount_t doesn't work for
usage count scenarios)
Aside from that, is the problem that refcount_dec_and_test() returns a
boolean (true - last put, false - not last) instead of the refcount
value? This does indeed make it hard to print the exact count value for
the event.
^ permalink raw reply
* [PATCH 1/2] LSM: switch to blocking policy update notifiers
From: Janne Karhunen @ 2019-05-31 14:02 UTC (permalink / raw)
To: sds, zohar, paul, linux-integrity, linux-security-module; +Cc: Janne Karhunen
Atomic policy updaters are not very useful as they cannot
usually perform the policy updates on their own. Since it
seems that there is no strict need for the atomicity,
switch to the blocking variant.
Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
---
security/security.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/security/security.c b/security/security.c
index 23cbb1a295a3..c5e69ce81521 100644
--- a/security/security.c
+++ b/security/security.c
@@ -39,7 +39,7 @@
#define LSM_COUNT (__end_lsm_info - __start_lsm_info)
struct security_hook_heads security_hook_heads __lsm_ro_after_init;
-static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
+static BLOCKING_NOTIFIER_HEAD(lsm_notifier_chain);
static struct kmem_cache *lsm_file_cache;
static struct kmem_cache *lsm_inode_cache;
@@ -432,19 +432,19 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
int call_lsm_notifier(enum lsm_event event, void *data)
{
- return atomic_notifier_call_chain(&lsm_notifier_chain, event, data);
+ return blocking_notifier_call_chain(&lsm_notifier_chain, event, data);
}
EXPORT_SYMBOL(call_lsm_notifier);
int register_lsm_notifier(struct notifier_block *nb)
{
- return atomic_notifier_chain_register(&lsm_notifier_chain, nb);
+ return blocking_notifier_chain_register(&lsm_notifier_chain, nb);
}
EXPORT_SYMBOL(register_lsm_notifier);
int unregister_lsm_notifier(struct notifier_block *nb)
{
- return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb);
+ return blocking_notifier_chain_unregister(&lsm_notifier_chain, nb);
}
EXPORT_SYMBOL(unregister_lsm_notifier);
--
2.17.1
^ permalink raw reply related
* [PATCH 2/2] ima: use the lsm policy update notifier
From: Janne Karhunen @ 2019-05-31 14:02 UTC (permalink / raw)
To: sds, zohar, paul, linux-integrity, linux-security-module; +Cc: Janne Karhunen
In-Reply-To: <20190531140237.9199-1-janne.karhunen@gmail.com>
Don't do lazy policy updates while running the rule matching,
run the updates as they happen.
Depends on commit cda44589be1c ("LSM: switch to blocking policy update notifiers")
Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
---
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_main.c | 8 ++++++
security/integrity/ima/ima_policy.c | 44 +++++++++++++++++++++--------
3 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d213e835c498..2203451862d4 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -154,6 +154,8 @@ unsigned long ima_get_binary_runtime_size(void);
int ima_init_template(void);
void ima_init_template_list(void);
int __init ima_init_digests(void);
+int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
+ void *lsm_data);
/*
* used to protect h_table and sha_table
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 357edd140c09..f9629c5e1aee 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -43,6 +43,10 @@ int ima_appraise;
int ima_hash_algo = HASH_ALGO_SHA1;
static int hash_setup_done;
+static struct notifier_block ima_lsm_policy_notifier = {
+ .notifier_call = ima_lsm_policy_change,
+};
+
static int __init hash_setup(char *str)
{
struct ima_template_desc *template_desc = ima_template_desc_current();
@@ -593,6 +597,10 @@ static int __init init_ima(void)
error = ima_init();
}
+ error = register_lsm_notifier(&ima_lsm_policy_notifier);
+ if (error)
+ pr_warn("Couldn't register LSM notifier, error %d\n", error);
+
if (!error)
ima_update_policy_flag();
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index e0cc323f948f..4201a21ff42f 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -252,12 +252,14 @@ __setup("ima_appraise_tcb", default_appraise_policy_setup);
/*
* The LSM policy can be reloaded, leaving the IMA LSM based rules referring
* to the old, stale LSM policy. Update the IMA LSM based rules to reflect
- * the reloaded LSM policy. We assume the rules still exist; and BUG_ON() if
- * they don't.
+ * the reloaded LSM policy.
*/
static void ima_lsm_update_rules(void)
{
struct ima_rule_entry *entry;
+ void *rule_new;
+ char *lsm_new;
+ char *lsm_old;
int result;
int i;
@@ -265,15 +267,39 @@ static void ima_lsm_update_rules(void)
for (i = 0; i < MAX_LSM_RULES; i++) {
if (!entry->lsm[i].rule)
continue;
+
+ lsm_old = entry->lsm[i].args_p;
+ lsm_new = kstrdup(lsm_old, GFP_KERNEL);
+ if (unlikely(!lsm_new))
+ return;
+
result = security_filter_rule_init(entry->lsm[i].type,
Audit_equal,
- entry->lsm[i].args_p,
- &entry->lsm[i].rule);
- BUG_ON(!entry->lsm[i].rule);
+ lsm_new,
+ &rule_new);
+ if (result == -EINVAL)
+ pr_warn("ima: rule for LSM \'%d\' is invalid\n",
+ entry->lsm[i].type);
+
+ entry->lsm[i].rule = rule_new;
+ entry->lsm[i].args_p = lsm_new;
+ synchronize_rcu();
+
+ kfree(lsm_old);
}
}
}
+int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
+ void *lsm_data)
+{
+ if (event != LSM_POLICY_CHANGE)
+ return NOTIFY_DONE;
+
+ ima_lsm_update_rules();
+ return NOTIFY_OK;
+}
+
/**
* ima_match_rules - determine whether an inode matches the measure rule.
* @rule: a pointer to a rule
@@ -327,11 +353,10 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
for (i = 0; i < MAX_LSM_RULES; i++) {
int rc = 0;
u32 osid;
- int retried = 0;
if (!rule->lsm[i].rule)
continue;
-retry:
+
switch (i) {
case LSM_OBJ_USER:
case LSM_OBJ_ROLE:
@@ -352,11 +377,6 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
default:
break;
}
- if ((rc < 0) && (!retried)) {
- retried = 1;
- ima_lsm_update_rules();
- goto retry;
- }
if (!rc)
return false;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 1/7] General notification queue with user mmap()'able ring buffer
From: David Howells @ 2019-05-31 14:20 UTC (permalink / raw)
To: Peter Zijlstra
Cc: dhowells, Jann Horn, Greg KH, Al Viro, raven, linux-fsdevel,
Linux API, linux-block, keyrings, linux-security-module,
kernel list, Kees Cook, Kernel Hardening
In-Reply-To: <20190531132620.GC2606@hirez.programming.kicks-ass.net>
Peter Zijlstra <peterz@infradead.org> wrote:
> Is it not the responsibility of the task that affects the 1->0
> transition to actually free the memory?
>
> That is, I'm expecting the '...' in both cases above the include the
> actual freeing of the object. If this is not the case, then @usage is
> not a reference count.
Yes. The '...' does the freeing. It seemed unnecessary to include the code
ellipsised there since it's not the point of the discussion, but if you want
the full function:
void afs_put_call(struct afs_call *call)
{
struct afs_net *net = call->net;
int n = atomic_dec_return(&call->usage);
int o = atomic_read(&net->nr_outstanding_calls);
trace_afs_call(call, afs_call_trace_put, n + 1, o,
__builtin_return_address(0));
ASSERTCMP(n, >=, 0);
if (n == 0) {
ASSERT(!work_pending(&call->async_work));
ASSERT(call->type->name != NULL);
if (call->rxcall) {
rxrpc_kernel_end_call(net->socket, call->rxcall);
call->rxcall = NULL;
}
if (call->type->destructor)
call->type->destructor(call);
afs_put_server(call->net, call->server);
afs_put_cb_interest(call->net, call->cbi);
afs_put_addrlist(call->alist);
kfree(call->request);
trace_afs_call(call, afs_call_trace_free, 0, o,
__builtin_return_address(0));
kfree(call);
o = atomic_dec_return(&net->nr_outstanding_calls);
if (o == 0)
wake_up_var(&net->nr_outstanding_calls);
}
}
You can see the kfree(call) in there.
Peter Zijlstra <peterz@infradead.org> wrote:
> (and it has already been established that refcount_t doesn't work for
> usage count scenarios)
?
Does that mean struct kref doesn't either?
> Aside from that, is the problem that refcount_dec_and_test() returns a
> boolean (true - last put, false - not last) instead of the refcount
> value? This does indeed make it hard to print the exact count value for
> the event.
That is the problem, yes - well, one of them: refcount_inc() doesn't either.
David
^ permalink raw reply
* Re: security/loadpin: Allow to exclude specific file types
From: Kees Cook @ 2019-05-31 14:44 UTC (permalink / raw)
To: Colin Ian King
Cc: Ke Wu, James Morris, Serge E. Hallyn, linux-security-module,
linux-kernel@vger.kernel.org
In-Reply-To: <73fac64c-fe49-4738-49a4-0afe668eed94@canonical.com>
On Fri, May 31, 2019 at 11:46:29AM +0100, Colin Ian King wrote:
> Hi,
>
> Static analysis with Coverity on linux-next has found a potential issue
> with the following commit:
>
> commit 1633a4f04cc171fc638deb5c95af96032d3c591b
> Author: Ke Wu <mikewu@google.com>
> Date: Thu May 30 12:22:08 2019 -0700
>
> security/loadpin: Allow to exclude specific file types
>
>
> 209 for (j = 0; j < ARRAY_SIZE(kernel_read_file_str); j++) {
> 210 if (strcmp(cur, kernel_read_file_str[j]) == 0) {
> 211 pr_info("excluding: %s\n",
> 212 kernel_read_file_str[j]);
>
> CID 81977 (#1 of 1): Out-of-bounds write
> overrun-local: Overrunning array ignore_read_file_id of 8 4-byte
> elements at element index 8 (byte offset 35) using index j (which
> evaluates to 8).
>
> 213 ignore_read_file_id[j] = 1;
>
> According to Coverity ignore_read_file_id is an array of 8 integers.
> However, ARRAY_SIZE(kernel_read_file_str) is 9, so we have an out of
> bounds write on ignore_read_file[j] when j is 8.
What am I missing? This doesn't fail the build:
+ BUILD_BUG_ON(ARRAY_SIZE(exclude_read_files) !=
+ ARRAY_SIZE(ignore_read_file_id));
They have the same number of elements.
--
Kees Cook
^ permalink raw reply
* Re: security/loadpin: Allow to exclude specific file types
From: Colin Ian King @ 2019-05-31 14:49 UTC (permalink / raw)
To: Kees Cook
Cc: Ke Wu, James Morris, Serge E. Hallyn, linux-security-module,
linux-kernel@vger.kernel.org
In-Reply-To: <201905310740.522B3A7C1@keescook>
On 31/05/2019 15:44, Kees Cook wrote:
> On Fri, May 31, 2019 at 11:46:29AM +0100, Colin Ian King wrote:
>> Hi,
>>
>> Static analysis with Coverity on linux-next has found a potential issue
>> with the following commit:
>>
>> commit 1633a4f04cc171fc638deb5c95af96032d3c591b
>> Author: Ke Wu <mikewu@google.com>
>> Date: Thu May 30 12:22:08 2019 -0700
>>
>> security/loadpin: Allow to exclude specific file types
>>
>>
>> 209 for (j = 0; j < ARRAY_SIZE(kernel_read_file_str); j++) {
>> 210 if (strcmp(cur, kernel_read_file_str[j]) == 0) {
>> 211 pr_info("excluding: %s\n",
>> 212 kernel_read_file_str[j]);
>>
>> CID 81977 (#1 of 1): Out-of-bounds write
>> overrun-local: Overrunning array ignore_read_file_id of 8 4-byte
>> elements at element index 8 (byte offset 35) using index j (which
>> evaluates to 8).
>>
>> 213 ignore_read_file_id[j] = 1;
>>
>> According to Coverity ignore_read_file_id is an array of 8 integers.
>> However, ARRAY_SIZE(kernel_read_file_str) is 9, so we have an out of
>> bounds write on ignore_read_file[j] when j is 8.
>
> What am I missing? This doesn't fail the build:
>
> + BUILD_BUG_ON(ARRAY_SIZE(exclude_read_files) !=
> + ARRAY_SIZE(ignore_read_file_id));
>
> They have the same number of elements.
>
Yep, that's very true. I'll discuss this with Coverity as this seems
like a weird false positive.
Apologies for the noise.
Colin
^ permalink raw reply
* Re: [PATCH 1/7] General notification queue with user mmap()'able ring buffer
From: David Howells @ 2019-05-31 14:55 UTC (permalink / raw)
To: Greg KH
Cc: dhowells, viro, raven, linux-fsdevel, linux-api, linux-block,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20190529231112.GB3164@kroah.com>
Greg KH <gregkh@linuxfoundation.org> wrote:
> So, if that's all that needs to be fixed, can you use the same
> buffer/code if that patch is merged?
I really don't know. The perf code is complex, partially in hardware drivers
and is tricky to understand - though a chunk of that is the "aux" buffer part;
PeterZ used words like "special" and "magic" and the comments in the code talk
about the hardware writing into the buffer.
__perf_output_begin() does not appear to be SMP safe. It uses local_cmpxchg()
and local_add() which on x86 lack the LOCK prefix.
stracing the perf command on my test machine, it calls perf_event_open(2) four
times and mmap's each fd it gets back. I'm guessing that each one maps a
separate buffer for each CPU.
So to use watch_queue based on perf's buffering, you would have to have a
(2^N)+1 pages-sized buffer for each CPU. So that would be a minimum of 64K of
unswappable memory for my desktop machine, say). Multiply that by each
process that wants to listen for events...
What I'm aiming for is something that has a single buffer used by all CPUs for
each instance of /dev/watch_queue opened and I'd also like to avoid having to
allocate the metadata page and the aux buffer to save space. This is locked
memory and cannot be swapped.
Also, perf has to leave a gap in the ring because it uses CIRC_SPACE(), though
that's a minor detail that I guess can't be fixed now.
I'm also slightly concerned that __perf_output_begin() doesn't check if
rb->user->tail has got ahead of rb->user->head or that it's lagging too far
behind. I doubt it's a serious problem for the kernel since it won't write
outside of the buffer, but userspace might screw up. I think the worst that
will happen is that userspace will get confused.
One thing I would like is to waive the 2^N size requirement. I understand
*why* we do that, but I wonder how expensive DIV instructions are for
relatively small divisors.
David
^ permalink raw reply
* [GIT PULL] integrity subsystem fixes for v5.2
From: Mimi Zohar @ 2019-05-31 15:58 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-security-module, linux-integrity
Hi Linus,
Included in this pull request are four bug fixes, which are not
specific to 5.2. The first two are related to the architecture
specific IMA policy support. The other two patches, one is related to
EVM signatures, based on additional hash algorithms, and the other is
related to displaying the IMA policy.
At least Scott's patch should have been included in the last open
window, but I sent the v5.2 pull request a bit earlier than normal.
thanks,
Mimi
The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9:
Linux 5.2-rc1 (2019-05-19 15:47:09 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-fixes-for-5.2-rc
for you to fetch changes up to 8cdc23a3d9ec0944000ad43bad588e36afdc38cd:
ima: show rules with IMA_INMASK correctly (2019-05-29 23:18:25 -0400)
----------------------------------------------------------------
Petr Vorel (1):
ima: fix wrong signed policy requirement when not appraising
Roberto Sassu (2):
evm: check hash algorithm passed to init_desc()
ima: show rules with IMA_INMASK correctly
Scott Wood (1):
x86/ima: Check EFI_RUNTIME_SERVICES before using
arch/x86/kernel/ima_arch.c | 5 +++++
security/integrity/evm/evm_crypto.c | 3 +++
security/integrity/ima/ima_policy.c | 28 ++++++++++++++++------------
3 files changed, 24 insertions(+), 12 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/7] General notification queue with user mmap()'able ring buffer
From: Peter Zijlstra @ 2019-05-31 16:44 UTC (permalink / raw)
To: David Howells
Cc: Jann Horn, Greg KH, Al Viro, raven, linux-fsdevel, Linux API,
linux-block, keyrings, linux-security-module, kernel list,
Kees Cook, Kernel Hardening
In-Reply-To: <606.1559312412@warthog.procyon.org.uk>
On Fri, May 31, 2019 at 03:20:12PM +0100, David Howells wrote:
> Peter Zijlstra <peterz@infradead.org> wrote:
> > (and it has already been established that refcount_t doesn't work for
> > usage count scenarios)
>
> ?
>
> Does that mean struct kref doesn't either?
Indeed, since kref is just a pointless wrapper around refcount_t it does
not either.
The main distinction between a reference count and a usage count is that
0 means different things. For a refcount 0 means dead. For a usage count
0 is merely unused but valid.
Incrementing a 0 refcount is a serious bug -- use-after-free (and hence
refcount_t will refuse this and splat), for a usage count this is no
problem.
Now, it is sort-of possible to merge the two, by basically stating
something like: usage = refcount - 1. But that can get tricky and people
have not really liked the result much for the few times I tried.
^ permalink raw reply
* Re: [PATCH 1/7] General notification queue with user mmap()'able ring buffer
From: David Howells @ 2019-05-31 17:12 UTC (permalink / raw)
To: Peter Zijlstra
Cc: dhowells, Jann Horn, Greg KH, Al Viro, raven, linux-fsdevel,
Linux API, linux-block, keyrings, linux-security-module,
kernel list, Kees Cook, Kernel Hardening
In-Reply-To: <20190531164444.GD2606@hirez.programming.kicks-ass.net>
Peter Zijlstra <peterz@infradead.org> wrote:
> > > (and it has already been established that refcount_t doesn't work for
> > > usage count scenarios)
> >
> > ?
> >
> > Does that mean struct kref doesn't either?
>
> Indeed, since kref is just a pointless wrapper around refcount_t it does
> not either.
>
> The main distinction between a reference count and a usage count is that
> 0 means different things. For a refcount 0 means dead. For a usage count
> 0 is merely unused but valid.
Ah - I consider the terms interchangeable.
Take Documentation/filesystems/vfs.txt for instance:
dget: open a new handle for an existing dentry (this just increments
the usage count)
dput: close a handle for a dentry (decrements the usage count). ...
...
d_lookup: look up a dentry given its parent and path name component
It looks up the child of that given name from the dcache
hash table. If it is found, the reference count is incremented
and the dentry is returned. The caller must use dput()
to free the dentry when it finishes using it.
Here we interchange the terms.
Or https://www.kernel.org/doc/gorman/html/understand/understand013.html
which seems to interchange the terms in reference to struct page.
David
^ permalink raw reply
* Re: security/loadpin: Allow to exclude specific file types
From: Ke Wu @ 2019-05-31 18:03 UTC (permalink / raw)
To: Colin Ian King
Cc: Kees Cook, James Morris, Serge E. Hallyn, linux-security-module,
linux-kernel@vger.kernel.org
In-Reply-To: <c80362bd-1dec-3e4f-c3e8-1c58d3a33070@canonical.com>
I think Coverity is correct. Note that it's the size of
kernel_read_file_str (rather than exclude_read_files) doesn't equal to
ignore_read_file_id.
This is because READING_MAX_ID is also an element in
kernel_read_file_str, which makes the size of kernel_read_file_str to
be READING_MAX_ID+1. I will send a new patch to fix the issue. Thanks
for the analysis!
On Fri, May 31, 2019 at 7:49 AM Colin Ian King <colin.king@canonical.com> wrote:
>
> On 31/05/2019 15:44, Kees Cook wrote:
> > On Fri, May 31, 2019 at 11:46:29AM +0100, Colin Ian King wrote:
> >> Hi,
> >>
> >> Static analysis with Coverity on linux-next has found a potential issue
> >> with the following commit:
> >>
> >> commit 1633a4f04cc171fc638deb5c95af96032d3c591b
> >> Author: Ke Wu <mikewu@google.com>
> >> Date: Thu May 30 12:22:08 2019 -0700
> >>
> >> security/loadpin: Allow to exclude specific file types
> >>
> >>
> >> 209 for (j = 0; j < ARRAY_SIZE(kernel_read_file_str); j++) {
> >> 210 if (strcmp(cur, kernel_read_file_str[j]) == 0) {
> >> 211 pr_info("excluding: %s\n",
> >> 212 kernel_read_file_str[j]);
> >>
> >> CID 81977 (#1 of 1): Out-of-bounds write
> >> overrun-local: Overrunning array ignore_read_file_id of 8 4-byte
> >> elements at element index 8 (byte offset 35) using index j (which
> >> evaluates to 8).
> >>
> >> 213 ignore_read_file_id[j] = 1;
> >>
> >> According to Coverity ignore_read_file_id is an array of 8 integers.
> >> However, ARRAY_SIZE(kernel_read_file_str) is 9, so we have an out of
> >> bounds write on ignore_read_file[j] when j is 8.
> >
> > What am I missing? This doesn't fail the build:
> >
> > + BUILD_BUG_ON(ARRAY_SIZE(exclude_read_files) !=
> > + ARRAY_SIZE(ignore_read_file_id));
> >
> > They have the same number of elements.
> >
>
> Yep, that's very true. I'll discuss this with Coverity as this seems
> like a weird false positive.
>
> Apologies for the noise.
>
> Colin
--
Ke Wu | Software Engineer | mikewu@google.com | Google Inc.
^ permalink raw reply
* Re: [PATCH V7 0/4] Add support for crypto agile logs
From: Joe Richey @ 2019-05-31 18:07 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-integrity, peterhuewe, jarkko.sakkinen, jgg, roberto.sassu,
linux-efi, linux-security-module, linux-kernel,
Thiébaud Weksteen, bsz
In-Reply-To: <20190520205501.177637-1-matthewgarrett@google.com>
[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]
On Mon, May 20, 2019 at 1:56 PM Matthew Garrett
<matthewgarrett@google.com> wrote:
>
> Identical to previous version except without the KSAN workaround - Ard
> has a better solution for that.
I just tested this on x86_64 with the systemd-boot (previously gummiboot)
bootloader. For context, this bootloader is essentially just an EFI
chainloader. This bootloader measures the kernel cmdline into PCR 8.
However, it calls GetEventLog before calling HashLogExtendEvent, intending
to have the log entry written to the "EFI TCG 2.0 final events table". See:
https://github.com/systemd/systemd/blob/75e40119a471454516ad0acc96f6f4094e7fb652/src/boot/efi/measure.c#L212-L227
With the current patchset, this log entry appears _twice_ in the sysfs file.
This is caused by the fact that the sysfs event log unconditionally appends
the entire final event log to the output of GetEventLog. However, the correct
behavior would be to append only the _new_ entries that appear in the final
event log to the output of GetEventLog.
This could be done by first calculating the length of the final events log
table, then recalculating the length of the final events log after the
kernel calls ExitBootServices. This would let us know for sure that we are
only appending new log entries.
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4849 bytes --]
^ permalink raw reply
* Re: [GIT PULL] integrity subsystem fixes for v5.2
From: pr-tracker-bot @ 2019-05-31 18:15 UTC (permalink / raw)
To: Mimi Zohar
Cc: Linus Torvalds, linux-kernel, linux-security-module,
linux-integrity
In-Reply-To: <1559318325.4280.13.camel@linux.ibm.com>
The pull request you sent on Fri, 31 May 2019 11:58:45 -0400:
> git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-fixes-for-5.2-rc
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/d266b3f5cac09434eb624af202f9a31307b34a88
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* [PATCH v3] Allow to exclude specific file types in LoadPin
From: Ke Wu @ 2019-05-31 18:25 UTC (permalink / raw)
To: Kees Cook, Jonathan Corbet, James Morris, Serge E. Hallyn
Cc: linux-doc, linux-kernel, linux-security-module, Ke Wu
In-Reply-To: <20190529224350.6460-1-mikewu@google.com>
Linux kernel already provide MODULE_SIG and KEXEC_VERIFY_SIG to
make sure loaded kernel module and kernel image are trusted. This
patch adds a kernel command line option "loadpin.exclude" which
allows to exclude specific file types from LoadPin. This is useful
when people want to use different mechanisms to verify module and
kernel image while still use LoadPin to protect the integrity of
other files kernel loads.
Signed-off-by: Ke Wu <mikewu@google.com>
---
Changelog since v2:
- Make size of exclude_read_files and ignore_read_file_id to be
equal to the size of kernel_read_file_str.
Changelog since v1:
- Mark ignore_read_file_id with __ro_after_init.
- Mark parse_exclude() with __init.
- Use ARRAY_SIZE(ignore_read_file_id) instead of READING_MAX_ID.
Documentation/admin-guide/LSM/LoadPin.rst | 10 ++++++
security/loadpin/loadpin.c | 42 +++++++++++++++++++++++
2 files changed, 52 insertions(+)
diff --git a/Documentation/admin-guide/LSM/LoadPin.rst b/Documentation/admin-guide/LSM/LoadPin.rst
index 32070762d24c..716ad9b23c9a 100644
--- a/Documentation/admin-guide/LSM/LoadPin.rst
+++ b/Documentation/admin-guide/LSM/LoadPin.rst
@@ -19,3 +19,13 @@ block device backing the filesystem is not read-only, a sysctl is
created to toggle pinning: ``/proc/sys/kernel/loadpin/enabled``. (Having
a mutable filesystem means pinning is mutable too, but having the
sysctl allows for easy testing on systems with a mutable filesystem.)
+
+It's also possible to exclude specific file types from LoadPin using kernel
+command line option "``loadpin.exclude``". By default, all files are
+included, but they can be excluded using kernel command line option such
+as "``loadpin.exclude=kernel-module,kexec-image``". This allows to use
+different mechanisms such as ``CONFIG_MODULE_SIG`` and
+``CONFIG_KEXEC_VERIFY_SIG`` to verify kernel module and kernel image while
+still use LoadPin to protect the integrity of other files kernel loads. The
+full list of valid file types can be found in ``kernel_read_file_str``
+defined in ``include/linux/fs.h``.
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 055fb0a64169..baa8a5b08c53 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -45,6 +45,12 @@ static void report_load(const char *origin, struct file *file, char *operation)
}
static int enforce = IS_ENABLED(CONFIG_SECURITY_LOADPIN_ENFORCE);
+/*
+ * The size should be READING_MAX_ID + 1 to be equal to the size of
+ * kernel_read_file_str.
+ */
+static char *exclude_read_files[READING_MAX_ID + 1];
+static int ignore_read_file_id[READING_MAX_ID + 1] __ro_after_init;
static struct super_block *pinned_root;
static DEFINE_SPINLOCK(pinned_root_spinlock);
@@ -129,6 +135,13 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
struct super_block *load_root;
const char *origin = kernel_read_file_id_str(id);
+ /* If the file id is excluded, ignore the pinning. */
+ if ((unsigned int)id < ARRAY_SIZE(ignore_read_file_id) &&
+ ignore_read_file_id[id]) {
+ report_load(origin, file, "pinning-excluded");
+ return 0;
+ }
+
/* This handles the older init_module API that has a NULL file. */
if (!file) {
if (!enforce) {
@@ -187,10 +200,37 @@ static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(kernel_load_data, loadpin_load_data),
};
+static void __init parse_exclude(void)
+{
+ int i, j;
+ char *cur;
+
+ for (i = 0; i < ARRAY_SIZE(exclude_read_files); i++) {
+ cur = exclude_read_files[i];
+ if (!cur)
+ break;
+ if (*cur == '\0')
+ continue;
+
+ for (j = 0; j < ARRAY_SIZE(kernel_read_file_str); j++) {
+ if (strcmp(cur, kernel_read_file_str[j]) == 0) {
+ pr_info("excluding: %s\n",
+ kernel_read_file_str[j]);
+ ignore_read_file_id[j] = 1;
+ /*
+ * Can not break, because one read_file_str
+ * may map to more than on read_file_id.
+ */
+ }
+ }
+ }
+}
+
static int __init loadpin_init(void)
{
pr_info("ready to pin (currently %senforcing)\n",
enforce ? "" : "not ");
+ parse_exclude();
security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
return 0;
}
@@ -203,3 +243,5 @@ DEFINE_LSM(loadpin) = {
/* Should not be mutable after boot, so not listed in sysfs (perm == 0). */
module_param(enforce, int, 0);
MODULE_PARM_DESC(enforce, "Enforce module/firmware pinning");
+module_param_array_named(exclude, exclude_read_files, charp, NULL, 0);
+MODULE_PARM_DESC(exclude, "Exclude pinning specific read file types");
--
2.22.0.rc1.257.g3120a18244-goog
^ permalink raw reply related
* Re: [PATCH 2/2] ima: use the lsm policy update notifier
From: Stephen Smalley @ 2019-05-31 18:35 UTC (permalink / raw)
To: Janne Karhunen, zohar, paul, linux-integrity,
linux-security-module
In-Reply-To: <20190531140237.9199-2-janne.karhunen@gmail.com>
On 5/31/19 10:02 AM, Janne Karhunen wrote:
> Don't do lazy policy updates while running the rule matching,
> run the updates as they happen.
>
> Depends on commit cda44589be1c ("LSM: switch to blocking policy update notifiers")
>
> Signed-off-by: Janne Karhunen <janne.karhunen@gmail.com>
> ---
> security/integrity/ima/ima.h | 2 ++
> security/integrity/ima/ima_main.c | 8 ++++++
> security/integrity/ima/ima_policy.c | 44 +++++++++++++++++++++--------
> 3 files changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index d213e835c498..2203451862d4 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -154,6 +154,8 @@ unsigned long ima_get_binary_runtime_size(void);
> int ima_init_template(void);
> void ima_init_template_list(void);
> int __init ima_init_digests(void);
> +int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
> + void *lsm_data);
>
> /*
> * used to protect h_table and sha_table
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 357edd140c09..f9629c5e1aee 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -43,6 +43,10 @@ int ima_appraise;
> int ima_hash_algo = HASH_ALGO_SHA1;
> static int hash_setup_done;
>
> +static struct notifier_block ima_lsm_policy_notifier = {
> + .notifier_call = ima_lsm_policy_change,
> +};
> +
> static int __init hash_setup(char *str)
> {
> struct ima_template_desc *template_desc = ima_template_desc_current();
> @@ -593,6 +597,10 @@ static int __init init_ima(void)
> error = ima_init();
> }
>
> + error = register_lsm_notifier(&ima_lsm_policy_notifier);
> + if (error)
> + pr_warn("Couldn't register LSM notifier, error %d\n", error);
> +
> if (!error)
> ima_update_policy_flag();
>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index e0cc323f948f..4201a21ff42f 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -252,12 +252,14 @@ __setup("ima_appraise_tcb", default_appraise_policy_setup);
> /*
> * The LSM policy can be reloaded, leaving the IMA LSM based rules referring
> * to the old, stale LSM policy. Update the IMA LSM based rules to reflect
> - * the reloaded LSM policy. We assume the rules still exist; and BUG_ON() if
> - * they don't.
> + * the reloaded LSM policy.
> */
> static void ima_lsm_update_rules(void)
> {
> struct ima_rule_entry *entry;
> + void *rule_new;
> + char *lsm_new;
> + char *lsm_old;
> int result;
> int i;
>
> @@ -265,15 +267,39 @@ static void ima_lsm_update_rules(void)
> for (i = 0; i < MAX_LSM_RULES; i++) {
> if (!entry->lsm[i].rule)
> continue;
> +
> + lsm_old = entry->lsm[i].args_p;
> + lsm_new = kstrdup(lsm_old, GFP_KERNEL);
> + if (unlikely(!lsm_new))
> + return;
> +
> result = security_filter_rule_init(entry->lsm[i].type,
> Audit_equal,
> - entry->lsm[i].args_p,
> - &entry->lsm[i].rule);
> - BUG_ON(!entry->lsm[i].rule);
> + lsm_new,
> + &rule_new);
> + if (result == -EINVAL)
> + pr_warn("ima: rule for LSM \'%d\' is invalid\n",
> + entry->lsm[i].type);
> +
> + entry->lsm[i].rule = rule_new;
Doesn't this still leak the old entry->lsm[i].rule?
Also, I don't think you can just mutate entry like this under RCU.
Don't you need to deep copy the entire entry, initialize the lsm rule in
the new entry, switch the new entry for the old in the list via the
appropriate rcu interface e.g. list_replace_rcu, and then free the old
entry via call_rcu() or after synchronize_rcu()? Consider how
audit_update_lsm_rules() works; it takes a mutex to synchronize with
other writers to the list and calls update_lsm_rule() on every rule.
update_lsm_rule() checks whether the rule contains a lsm filter via
security_audit_rule_known(), and if so, deep copies the rule via
audit_dupe_rule(), replaces the old with the new via list_replace_rcu(),
and then performs a delayed free of the old rule via call_rcu().
audit_dupe_rule() ultimately calls audit_dupe_lsm_field() to "duplicate"
the LSM field information, which consists of copying the string and then
calling security_audit_rule_init() aka security_filter_rule_init() to
create the internal structure representation of the lsm rule from the
string. I could be wrong but I don't think you can short-circuit the
copying or mutate entry in place like this.
> + entry->lsm[i].args_p = lsm_new;
> + synchronize_rcu();
> +
> + kfree(lsm_old);
> }
> }
> }
>
> +int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
> + void *lsm_data)
> +{
> + if (event != LSM_POLICY_CHANGE)
> + return NOTIFY_DONE;
> +
> + ima_lsm_update_rules();
> + return NOTIFY_OK;
> +}
> +
> /**
> * ima_match_rules - determine whether an inode matches the measure rule.
> * @rule: a pointer to a rule
> @@ -327,11 +353,10 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> for (i = 0; i < MAX_LSM_RULES; i++) {
> int rc = 0;
> u32 osid;
> - int retried = 0;
>
> if (!rule->lsm[i].rule)
> continue;
> -retry:
> +
> switch (i) {
> case LSM_OBJ_USER:
> case LSM_OBJ_ROLE:
> @@ -352,11 +377,6 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> default:
> break;
> }
> - if ((rc < 0) && (!retried)) {
> - retried = 1;
> - ima_lsm_update_rules();
> - goto retry;
> - }
> if (!rc)
> return false;
> }
>
^ permalink raw reply
* Re: security/loadpin: Allow to exclude specific file types
From: Kees Cook @ 2019-05-31 20:33 UTC (permalink / raw)
To: Ke Wu
Cc: Colin Ian King, James Morris, Serge E. Hallyn,
linux-security-module, linux-kernel@vger.kernel.org
In-Reply-To: <CANRnR9Q4AY1nyTebWgkVM-wUfZv7kKNLnsrchWPBH4HPbB-X6Q@mail.gmail.com>
On Fri, May 31, 2019 at 11:03:17AM -0700, Ke Wu wrote:
> I think Coverity is correct. Note that it's the size of
> kernel_read_file_str (rather than exclude_read_files) doesn't equal to
> ignore_read_file_id.
>
> This is because READING_MAX_ID is also an element in
> kernel_read_file_str, which makes the size of kernel_read_file_str to
> be READING_MAX_ID+1. I will send a new patch to fix the issue. Thanks
> for the analysis!
Ah! Yes, I see now. I was looking at the wrong things. It should be
possible to just do:
> > >> 209 for (j = 0; j < ARRAY_SIZE(kernel_read_file_str); j++) {
for (j = 0; j < ARRAY_SIZE(ignore_read_file_id); j++)
and add a
BUILD_BUG_ON(ARRAY_SIZE(kernel_read_file_str) < ARRAY_SIZE(ignore_read_file_id))
for future robustness checking.
Thanks for looking at this more closely!
-Kees
> > >> 210 if (strcmp(cur, kernel_read_file_str[j]) == 0) {
> > >> 211 pr_info("excluding: %s\n",
> > >> 212 kernel_read_file_str[j]);
> > >>
> > >> CID 81977 (#1 of 1): Out-of-bounds write
> > >> overrun-local: Overrunning array ignore_read_file_id of 8 4-byte
> > >> elements at element index 8 (byte offset 35) using index j (which
> > >> evaluates to 8).
> > >>
> > >> 213 ignore_read_file_id[j] = 1;
> > >>
> > >> According to Coverity ignore_read_file_id is an array of 8 integers.
> > >> However, ARRAY_SIZE(kernel_read_file_str) is 9, so we have an out of
> > >> bounds write on ignore_read_file[j] when j is 8.
> > >
> > > What am I missing? This doesn't fail the build:
> > >
> > > + BUILD_BUG_ON(ARRAY_SIZE(exclude_read_files) !=
> > > + ARRAY_SIZE(ignore_read_file_id));
> > >
> > > They have the same number of elements.
> > >
> >
> > Yep, that's very true. I'll discuss this with Coverity as this seems
> > like a weird false positive.
> >
> > Apologies for the noise.
> >
> > Colin
>
>
>
> --
> Ke Wu | Software Engineer | mikewu@google.com | Google Inc.
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v3] Allow to exclude specific file types in LoadPin
From: Kees Cook @ 2019-05-31 21:01 UTC (permalink / raw)
To: Ke Wu
Cc: Jonathan Corbet, James Morris, Serge E. Hallyn, linux-doc,
linux-kernel, linux-security-module
In-Reply-To: <20190531182553.51721-1-mikewu@google.com>
On Fri, May 31, 2019 at 11:25:53AM -0700, Ke Wu wrote:
> Linux kernel already provide MODULE_SIG and KEXEC_VERIFY_SIG to
> make sure loaded kernel module and kernel image are trusted. This
> patch adds a kernel command line option "loadpin.exclude" which
> allows to exclude specific file types from LoadPin. This is useful
> when people want to use different mechanisms to verify module and
> kernel image while still use LoadPin to protect the integrity of
> other files kernel loads.
>
> Signed-off-by: Ke Wu <mikewu@google.com>
> ---
> Changelog since v2:
> - Make size of exclude_read_files and ignore_read_file_id to be
> equal to the size of kernel_read_file_str.
Thanks! I've fixed this differently and it should be visible shortly.
-Kees
>
> Changelog since v1:
> - Mark ignore_read_file_id with __ro_after_init.
> - Mark parse_exclude() with __init.
> - Use ARRAY_SIZE(ignore_read_file_id) instead of READING_MAX_ID.
>
>
> Documentation/admin-guide/LSM/LoadPin.rst | 10 ++++++
> security/loadpin/loadpin.c | 42 +++++++++++++++++++++++
> 2 files changed, 52 insertions(+)
>
> diff --git a/Documentation/admin-guide/LSM/LoadPin.rst b/Documentation/admin-guide/LSM/LoadPin.rst
> index 32070762d24c..716ad9b23c9a 100644
> --- a/Documentation/admin-guide/LSM/LoadPin.rst
> +++ b/Documentation/admin-guide/LSM/LoadPin.rst
> @@ -19,3 +19,13 @@ block device backing the filesystem is not read-only, a sysctl is
> created to toggle pinning: ``/proc/sys/kernel/loadpin/enabled``. (Having
> a mutable filesystem means pinning is mutable too, but having the
> sysctl allows for easy testing on systems with a mutable filesystem.)
> +
> +It's also possible to exclude specific file types from LoadPin using kernel
> +command line option "``loadpin.exclude``". By default, all files are
> +included, but they can be excluded using kernel command line option such
> +as "``loadpin.exclude=kernel-module,kexec-image``". This allows to use
> +different mechanisms such as ``CONFIG_MODULE_SIG`` and
> +``CONFIG_KEXEC_VERIFY_SIG`` to verify kernel module and kernel image while
> +still use LoadPin to protect the integrity of other files kernel loads. The
> +full list of valid file types can be found in ``kernel_read_file_str``
> +defined in ``include/linux/fs.h``.
> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
> index 055fb0a64169..baa8a5b08c53 100644
> --- a/security/loadpin/loadpin.c
> +++ b/security/loadpin/loadpin.c
> @@ -45,6 +45,12 @@ static void report_load(const char *origin, struct file *file, char *operation)
> }
>
> static int enforce = IS_ENABLED(CONFIG_SECURITY_LOADPIN_ENFORCE);
> +/*
> + * The size should be READING_MAX_ID + 1 to be equal to the size of
> + * kernel_read_file_str.
> + */
> +static char *exclude_read_files[READING_MAX_ID + 1];
> +static int ignore_read_file_id[READING_MAX_ID + 1] __ro_after_init;
> static struct super_block *pinned_root;
> static DEFINE_SPINLOCK(pinned_root_spinlock);
>
> @@ -129,6 +135,13 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> struct super_block *load_root;
> const char *origin = kernel_read_file_id_str(id);
>
> + /* If the file id is excluded, ignore the pinning. */
> + if ((unsigned int)id < ARRAY_SIZE(ignore_read_file_id) &&
> + ignore_read_file_id[id]) {
> + report_load(origin, file, "pinning-excluded");
> + return 0;
> + }
> +
> /* This handles the older init_module API that has a NULL file. */
> if (!file) {
> if (!enforce) {
> @@ -187,10 +200,37 @@ static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(kernel_load_data, loadpin_load_data),
> };
>
> +static void __init parse_exclude(void)
> +{
> + int i, j;
> + char *cur;
> +
> + for (i = 0; i < ARRAY_SIZE(exclude_read_files); i++) {
> + cur = exclude_read_files[i];
> + if (!cur)
> + break;
> + if (*cur == '\0')
> + continue;
> +
> + for (j = 0; j < ARRAY_SIZE(kernel_read_file_str); j++) {
> + if (strcmp(cur, kernel_read_file_str[j]) == 0) {
> + pr_info("excluding: %s\n",
> + kernel_read_file_str[j]);
> + ignore_read_file_id[j] = 1;
> + /*
> + * Can not break, because one read_file_str
> + * may map to more than on read_file_id.
> + */
> + }
> + }
> + }
> +}
> +
> static int __init loadpin_init(void)
> {
> pr_info("ready to pin (currently %senforcing)\n",
> enforce ? "" : "not ");
> + parse_exclude();
> security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
> return 0;
> }
> @@ -203,3 +243,5 @@ DEFINE_LSM(loadpin) = {
> /* Should not be mutable after boot, so not listed in sysfs (perm == 0). */
> module_param(enforce, int, 0);
> MODULE_PARM_DESC(enforce, "Enforce module/firmware pinning");
> +module_param_array_named(exclude, exclude_read_files, charp, NULL, 0);
> +MODULE_PARM_DESC(exclude, "Exclude pinning specific read file types");
> --
> 2.22.0.rc1.257.g3120a18244-goog
>
--
Kees Cook
^ permalink raw reply
* [PATCH] Add support to carry ima buffer to in kexec_file_load for amr64
From: Prakhar Srivastava @ 2019-05-31 22:07 UTC (permalink / raw)
To: linux-integrity, linux-security-module
Cc: zohar, bauerman, Prakhar Srivastava
This patch re-uses the code written for powerpc to carry ima buffer
to the next kernel during kexec_file_load for arm64.
https://lore.kernel.org/patchwork/patch/740277/
changes added:
- add new entries in the arch_kimage to contain the memory
address of the kernel segment containing the ima buffer.
- set up the dtb accordingly to contain the address of the
ima buffer.
Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
---
arch/arm64/Kconfig | 7 +
arch/arm64/include/asm/ima.h | 31 ++++
arch/arm64/include/asm/kexec.h | 4 +
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/ima_kexec.c | 217 +++++++++++++++++++++++++
arch/arm64/kernel/machine_kexec_file.c | 39 +++++
6 files changed, 299 insertions(+)
create mode 100644 arch/arm64/include/asm/ima.h
create mode 100644 arch/arm64/kernel/ima_kexec.c
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 7e34b9eba5de..586152608d09 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -940,6 +940,13 @@ config KEXEC_FILE
for kernel and initramfs as opposed to list of segments as
accepted by previous system call.
+config HAVE_IMA_KEXEC
+ bool "enable arch specific ima buffer pass"
+ depends on KEXEC_FILE
+ help
+ This adds support to carry ima log to the next kernel in case
+ of kexec_file_load
+
config KEXEC_VERIFY_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
depends on KEXEC_FILE
diff --git a/arch/arm64/include/asm/ima.h b/arch/arm64/include/asm/ima.h
new file mode 100644
index 000000000000..2c504281028d
--- /dev/null
+++ b/arch/arm64/include/asm/ima.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ARM64_IMA_H
+#define _ASM_ARM64_IMA_H
+
+#define FDT_PROP_KEXEC_BUFFER "linux,ima-kexec-buffer"
+
+struct kimage;
+
+int ima_get_kexec_buffer(void **addr, size_t *size);
+int ima_free_kexec_buffer(void);
+
+#ifdef CONFIG_IMA
+void remove_ima_buffer(void *fdt, int chosen_node);
+#else
+static inline void remove_ima_buffer(void *fdt, int chosen_node) {}
+#endif
+
+#ifdef CONFIG_IMA_KEXEC
+int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
+ size_t size);
+
+int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node);
+#else
+static inline int setup_ima_buffer(const struct kimage *image, void *fdt,
+ int chosen_node)
+{
+ remove_ima_buffer(fdt, chosen_node);
+ return 0;
+}
+#endif /* CONFIG_IMA_KEXEC */
+#endif /* _ASM_ARM64_IMA_H */
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 67e4cb75d1fd..2498dc3617a4 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -99,6 +99,8 @@ static inline void crash_post_resume(void) {}
struct kimage_arch {
void *dtb;
unsigned long dtb_mem;
+ phys_addr_t ima_buffer_addr;
+ size_t ima_buffer_size;
};
extern const struct kexec_file_ops kexec_image_ops;
@@ -110,6 +112,8 @@ extern int load_other_segments(struct kimage *image,
unsigned long kernel_load_addr, unsigned long kernel_size,
char *initrd, unsigned long initrd_len,
char *cmdline);
+extern int delete_fdt_mem_rsv(void *fdt, unsigned long start,
+ unsigned long size);
#endif
#endif /* __ASSEMBLY__ */
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index cd434d0719c1..bc4d84266b29 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o
obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o
obj-$(CONFIG_ARM64_SSBD) += ssbd.o
obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o
+obj-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
obj-y += vdso/ probes/
head-y := head.o
diff --git a/arch/arm64/kernel/ima_kexec.c b/arch/arm64/kernel/ima_kexec.c
new file mode 100644
index 000000000000..41d6721861c9
--- /dev/null
+++ b/arch/arm64/kernel/ima_kexec.c
@@ -0,0 +1,217 @@
+/*
+ * Copyright (C) 2016 IBM Corporation
+ *
+ * Authors:
+ * Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/slab.h>
+#include <linux/kexec.h>
+#include <linux/of.h>
+#include <linux/memblock.h>
+#include <linux/libfdt.h>
+#include <asm/kexec.h>
+#include <asm/ima.h>
+
+static int get_addr_size_cells(int *addr_cells, int *size_cells)
+{
+ struct device_node *root;
+
+ root = of_find_node_by_path("/");
+ if (!root)
+ return -EINVAL;
+
+ *addr_cells = of_n_addr_cells(root);
+ *size_cells = of_n_size_cells(root);
+
+ of_node_put(root);
+
+ return 0;
+}
+
+static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
+ size_t *size)
+{
+
+ int ret, addr_cells, size_cells;
+
+ ret = get_addr_size_cells(&addr_cells, &size_cells);
+ if (ret)
+ return ret;
+
+ if (len < 4 * (addr_cells + size_cells))
+ return -ENOENT;
+
+ *addr = of_read_number(prop, addr_cells);
+ *size = of_read_number(prop + 4 * addr_cells, size_cells);
+
+ return 0;
+}
+
+/**
+ * ima_get_kexec_buffer - get IMA buffer from the previous kernel
+ * @addr: On successful return, set to point to the buffer contents.
+ * @size: On successful return, set to the buffer size.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ int ret, len;
+ unsigned long tmp_addr;
+ size_t tmp_size;
+ const void *prop;
+
+ prop = of_get_property(of_chosen, FDT_PROP_KEXEC_BUFFER, &len);
+ if (!prop)
+ return -ENOENT;
+
+ ret = do_get_kexec_buffer(prop, len, &tmp_addr, &tmp_size);
+ if (ret)
+ return ret;
+
+ *addr = __va(tmp_addr);
+ *size = tmp_size;
+ return 0;
+}
+
+/**
+ * ima_free_kexec_buffer - free memory used by the IMA buffer
+ */
+int ima_free_kexec_buffer(void)
+{
+ int ret;
+ unsigned long addr;
+ size_t size;
+ struct property *prop;
+
+ prop = of_find_property(of_chosen, FDT_PROP_KEXEC_BUFFER, NULL);
+ if (!prop)
+ return -ENOENT;
+
+ ret = do_get_kexec_buffer(prop->value, prop->length, &addr, &size);
+ if (ret)
+ return ret;
+
+ ret = of_remove_property(of_chosen, prop);
+ if (ret)
+ return ret;
+
+ return memblock_free(addr, size);
+}
+
+/**
+ * remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
+ *
+ * The IMA measurement buffer is of no use to a subsequent kernel, so we always
+ * remove it from the device tree.
+ */
+void remove_ima_buffer(void *fdt, int chosen_node)
+{
+ int ret, len;
+ unsigned long addr;
+ size_t size;
+ const void *prop;
+
+ prop = fdt_getprop(fdt, chosen_node, FDT_PROP_KEXEC_BUFFER, &len);
+ if (!prop)
+ return;
+
+ ret = do_get_kexec_buffer(prop, len, &addr, &size);
+ fdt_delprop(fdt, chosen_node, FDT_PROP_KEXEC_BUFFER);
+ if (ret)
+ return;
+
+ ret = delete_fdt_mem_rsv(fdt, addr, size);
+ if (!ret)
+ pr_debug("Removed old IMA buffer reservation.\n");
+}
+
+#ifdef CONFIG_IMA_KEXEC
+/**
+ * arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA buffer
+ *
+ * Architectures should use this function to pass on the IMA buffer
+ * information to the next kernel.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
+ size_t size)
+{
+ image->arch.ima_buffer_addr = load_addr;
+ image->arch.ima_buffer_size = size;
+ return 0;
+}
+
+static int write_number(void *p, u64 value, int cells)
+{
+ if (cells == 1) {
+ u32 tmp;
+
+ if (value > U32_MAX)
+ return -EINVAL;
+
+ tmp = cpu_to_be32(value);
+ memcpy(p, &tmp, sizeof(tmp));
+ } else if (cells == 2) {
+ u64 tmp;
+
+ tmp = cpu_to_be64(value);
+ memcpy(p, &tmp, sizeof(tmp));
+ } else
+ return -EINVAL;
+ return 0;
+}
+
+/**
+ * setup_ima_buffer - add IMA buffer information to the fdt
+ * @image: kexec image being loaded.
+ * @dtb: Flattened device tree for the next kernel.
+ * @chosen_node: Offset to the chosen node.
+ *
+ * Return: 0 on success, or negative errno on error.
+ */
+int setup_ima_buffer(const struct kimage *image, void *dtb, int chosen_node)
+{
+ int ret, addr_cells, size_cells, entry_size;
+ u8 value[16];
+
+ remove_ima_buffer(dtb, chosen_node);
+
+ ret = get_addr_size_cells(&addr_cells, &size_cells);
+ if (ret)
+ return ret;
+
+ entry_size = 4 * (addr_cells + size_cells);
+
+ if (entry_size > sizeof(value))
+ return -EINVAL;
+
+ ret = write_number(value, image->arch.ima_buffer_addr, addr_cells);
+ if (ret)
+ return ret;
+
+ ret = write_number(value + 4 * addr_cells, image->arch.ima_buffer_size,
+ size_cells);
+ if (ret)
+ return ret;
+
+ ret = fdt_setprop(dtb, chosen_node, FDT_PROP_KEXEC_BUFFER, value,
+ entry_size);
+ if (ret < 0)
+ return -EINVAL;
+
+ ret = fdt_add_mem_rsv(dtb, image->segment[0].mem,
+ image->segment[0].memsz);
+ if (ret)
+ return -EINVAL;
+
+ return 0;
+}
+#endif /* CONFIG_IMA_KEXEC */
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index 58871333737a..c05ad6b74b62 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -21,6 +21,7 @@
#include <linux/types.h>
#include <linux/vmalloc.h>
#include <asm/byteorder.h>
+#include <asm/ima.h>
/* relevant device tree properties */
#define FDT_PROP_INITRD_START "linux,initrd-start"
@@ -85,6 +86,11 @@ static int setup_dtb(struct kimage *image,
goto out;
}
+ /* add ima_buffer */
+ ret = setup_ima_buffer(image, dtb, off);
+ if (ret)
+ goto out;
+
/* add kaslr-seed */
ret = fdt_delprop(dtb, off, FDT_PROP_KASLR_SEED);
if (ret == -FDT_ERR_NOTFOUND)
@@ -114,6 +120,39 @@ static int setup_dtb(struct kimage *image,
*/
#define DTB_EXTRA_SPACE 0x1000
+
+/**
+ * delete_fdt_mem_rsv - delete memory reservation with given address and size
+ *
+ * Return: 0 on success, or negative errno on error.
+ */
+int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size)
+{
+ int i, ret, num_rsvs = fdt_num_mem_rsv(fdt);
+
+ for (i = 0; i < num_rsvs; i++) {
+ uint64_t rsv_start, rsv_size;
+
+ ret = fdt_get_mem_rsv(fdt, i, &rsv_start, &rsv_size);
+ if (ret) {
+ pr_err("Malformed device tree.\n");
+ return -EINVAL;
+ }
+
+ if (rsv_start == start && rsv_size == size) {
+ ret = fdt_del_mem_rsv(fdt, i);
+ if (ret) {
+ pr_err("Error deleting device tree reservation.\n");
+ return -EINVAL;
+ }
+
+ return 0;
+ }
+ }
+
+ return -ENOENT;
+}
+
static int create_dtb(struct kimage *image,
unsigned long initrd_load_addr, unsigned long initrd_len,
char *cmdline, void **dtb)
--
2.17.1
^ permalink raw reply related
* [PATCH 00/58] LSM: Module stacking for AppArmor
From: Casey Schaufler @ 2019-05-31 23:09 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds
This patchset provides the changes required for
the AppArmor security module to stack safely with any other.
A new process attribute identifies which security module
information should be reported by SO_PEERSEC and the
/proc/.../attr/current interface. This is provided by
/proc/.../attr/display. Writing the name of the security
module desired to this interface will set which LSM hooks
will be called for this information. The first security
module providing the hooks will be used by default.
The use of integer based security tokens (secids) is
generally (but not completely) replaced by a structure
lsm_export. The lsm_export structure can contain information
for each of the security modules that export information
outside the LSM layer.
The LSM interfaces that provide "secctx" text strings
have been changed to use a structure "lsm_context"
instead of a pointer/length pair. In some cases the
interfaces used a "char *" pointer and in others a
"void *". This was necessary to ensure that the correct
release mechanism for the text is used. It also makes
many of the interfaces cleaner.
https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v1-apparmor
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
drivers/android/binder.c | 25 ++-
fs/kernfs/dir.c | 6 +-
fs/kernfs/inode.c | 31 ++-
fs/kernfs/kernfs-internal.h | 3 +-
fs/nfs/inode.c | 13 +-
fs/nfs/internal.h | 8 +-
fs/nfs/nfs4proc.c | 17 +-
fs/nfs/nfs4xdr.c | 16 +-
fs/nfsd/nfs4proc.c | 8 +-
fs/nfsd/nfs4xdr.c | 14 +-
fs/nfsd/vfs.c | 7 +-
fs/proc/base.c | 1 +
include/linux/cred.h | 3 +-
include/linux/lsm_hooks.h | 91 +++++----
include/linux/nfs4.h | 8 +-
include/linux/security.h | 133 +++++++++----
include/net/af_unix.h | 2 +-
include/net/netlabel.h | 10 +-
include/net/scm.h | 14 +-
kernel/audit.c | 43 ++--
kernel/audit.h | 9 +-
kernel/auditfilter.c | 6 +-
kernel/auditsc.c | 77 ++++----
kernel/cred.c | 15 +-
net/ipv4/cipso_ipv4.c | 13 +-
net/ipv4/ip_sockglue.c | 12 +-
net/netfilter/nf_conntrack_netlink.c | 29 ++-
net/netfilter/nf_conntrack_standalone.c | 16 +-
net/netfilter/nfnetlink_queue.c | 38 ++--
net/netfilter/nft_meta.c | 13 +-
net/netfilter/xt_SECMARK.c | 14 +-
net/netlabel/netlabel_kapi.c | 5 +-
net/netlabel/netlabel_unlabeled.c | 101 +++++-----
net/netlabel/netlabel_unlabeled.h | 2 +-
net/netlabel/netlabel_user.c | 13 +-
net/netlabel/netlabel_user.h | 2 +-
net/unix/af_unix.c | 6 +-
security/apparmor/audit.c | 4 +-
security/apparmor/include/audit.h | 2 +-
security/apparmor/include/net.h | 6 +-
security/apparmor/include/secid.h | 9 +-
security/apparmor/lsm.c | 64 +++---
security/apparmor/secid.c | 42 ++--
security/integrity/ima/ima.h | 14 +-
security/integrity/ima/ima_api.c | 9 +-
security/integrity/ima/ima_appraise.c | 6 +-
security/integrity/ima/ima_main.c | 34 ++--
security/integrity/ima/ima_policy.c | 19 +-
security/security.c | 338 +++++++++++++++++++++++++++-----
security/selinux/hooks.c | 259 ++++++++++++------------
security/selinux/include/audit.h | 5 +-
security/selinux/include/objsec.h | 42 +++-
security/selinux/netlabel.c | 25 +--
security/selinux/ss/services.c | 18 +-
security/smack/smack.h | 18 ++
security/smack/smack_lsm.c | 238 +++++++++++-----------
security/smack/smack_netfilter.c | 8 +-
security/smack/smackfs.c | 12 +-
58 files changed, 1217 insertions(+), 779 deletions(-)
^ permalink raw reply
* [PATCH 01/58] LSM: Infrastructure management of the superblock
From: Casey Schaufler @ 2019-05-31 23:09 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20190531231020.628-1-casey@schaufler-ca.com>
Move management of the superblock->sb_security blob out
of the individual security modules and into the security
infrastructure. Instead of allocating the blobs from within
the modules the modules tell the infrastructure how much
space is required, and the space is allocated there.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
include/linux/lsm_hooks.h | 1 +
security/security.c | 46 ++++++++++++++++++++----
security/selinux/hooks.c | 58 ++++++++++++-------------------
security/selinux/include/objsec.h | 6 ++++
security/selinux/ss/services.c | 3 +-
security/smack/smack.h | 6 ++++
security/smack/smack_lsm.c | 35 +++++--------------
7 files changed, 85 insertions(+), 70 deletions(-)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index a240a3fc5fc4..f9222a04968d 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2047,6 +2047,7 @@ struct lsm_blob_sizes {
int lbs_cred;
int lbs_file;
int lbs_inode;
+ int lbs_superblock;
int lbs_ipc;
int lbs_msg_msg;
int lbs_task;
diff --git a/security/security.c b/security/security.c
index 23cbb1a295a3..550988a0f024 100644
--- a/security/security.c
+++ b/security/security.c
@@ -172,6 +172,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
lsm_set_blob_size(&needed->lbs_inode, &blob_sizes.lbs_inode);
lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
+ lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
}
@@ -300,12 +301,13 @@ static void __init ordered_lsm_init(void)
for (lsm = ordered_lsms; *lsm; lsm++)
prepare_lsm(*lsm);
- init_debug("cred blob size = %d\n", blob_sizes.lbs_cred);
- init_debug("file blob size = %d\n", blob_sizes.lbs_file);
- init_debug("inode blob size = %d\n", blob_sizes.lbs_inode);
- init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
- init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
- init_debug("task blob size = %d\n", blob_sizes.lbs_task);
+ init_debug("cred blob size = %d\n", blob_sizes.lbs_cred);
+ init_debug("file blob size = %d\n", blob_sizes.lbs_file);
+ init_debug("inode blob size = %d\n", blob_sizes.lbs_inode);
+ init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
+ init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
+ init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
+ init_debug("task blob size = %d\n", blob_sizes.lbs_task);
/*
* Create any kmem_caches needed for blobs
@@ -603,6 +605,27 @@ static void __init lsm_early_task(struct task_struct *task)
panic("%s: Early task alloc failed.\n", __func__);
}
+/**
+ * lsm_superblock_alloc - allocate a composite superblock blob
+ * @sb: the superblock that needs a blob
+ *
+ * Allocate the superblock blob for all the modules
+ *
+ * Returns 0, or -ENOMEM if memory can't be allocated.
+ */
+int lsm_superblock_alloc(struct super_block *sb)
+{
+ if (blob_sizes.lbs_superblock == 0) {
+ sb->s_security = NULL;
+ return 0;
+ }
+
+ sb->s_security = kzalloc(blob_sizes.lbs_superblock, GFP_KERNEL);
+ if (sb->s_security == NULL)
+ return -ENOMEM;
+ return 0;
+}
+
/*
* Hook list operation macros.
*
@@ -776,12 +799,21 @@ int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *
int security_sb_alloc(struct super_block *sb)
{
- return call_int_hook(sb_alloc_security, 0, sb);
+ int rc = lsm_superblock_alloc(sb);
+
+ if (unlikely(rc))
+ return rc;
+ rc = call_int_hook(sb_alloc_security, 0, sb);
+ if (unlikely(rc))
+ security_sb_free(sb);
+ return rc;
}
void security_sb_free(struct super_block *sb)
{
call_void_hook(sb_free_security, sb);
+ kfree(sb->s_security);
+ sb->s_security = NULL;
}
void security_free_mnt_opts(void **mnt_opts)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 1d0b37af2444..7478d8eda00a 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -335,7 +335,7 @@ static void inode_free_security(struct inode *inode)
if (!isec)
return;
- sbsec = inode->i_sb->s_security;
+ sbsec = selinux_superblock(inode->i_sb);
/*
* As not all inode security structures are in a list, we check for
* empty list outside of the lock to make sure that we won't waste
@@ -366,11 +366,7 @@ static int file_alloc_security(struct file *file)
static int superblock_alloc_security(struct super_block *sb)
{
- struct superblock_security_struct *sbsec;
-
- sbsec = kzalloc(sizeof(struct superblock_security_struct), GFP_KERNEL);
- if (!sbsec)
- return -ENOMEM;
+ struct superblock_security_struct *sbsec = selinux_superblock(sb);
mutex_init(&sbsec->lock);
INIT_LIST_HEAD(&sbsec->isec_head);
@@ -379,18 +375,10 @@ static int superblock_alloc_security(struct super_block *sb)
sbsec->sid = SECINITSID_UNLABELED;
sbsec->def_sid = SECINITSID_FILE;
sbsec->mntpoint_sid = SECINITSID_UNLABELED;
- sb->s_security = sbsec;
return 0;
}
-static void superblock_free_security(struct super_block *sb)
-{
- struct superblock_security_struct *sbsec = sb->s_security;
- sb->s_security = NULL;
- kfree(sbsec);
-}
-
struct selinux_mnt_opts {
const char *fscontext, *context, *rootcontext, *defcontext;
};
@@ -507,7 +495,7 @@ static int selinux_is_genfs_special_handling(struct super_block *sb)
static int selinux_is_sblabel_mnt(struct super_block *sb)
{
- struct superblock_security_struct *sbsec = sb->s_security;
+ struct superblock_security_struct *sbsec = selinux_superblock(sb);
/*
* IMPORTANT: Double-check logic in this function when adding a new
@@ -535,7 +523,7 @@ static int selinux_is_sblabel_mnt(struct super_block *sb)
static int sb_finish_set_opts(struct super_block *sb)
{
- struct superblock_security_struct *sbsec = sb->s_security;
+ struct superblock_security_struct *sbsec = selinux_superblock(sb);
struct dentry *root = sb->s_root;
struct inode *root_inode = d_backing_inode(root);
int rc = 0;
@@ -648,7 +636,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
unsigned long *set_kern_flags)
{
const struct cred *cred = current_cred();
- struct superblock_security_struct *sbsec = sb->s_security;
+ struct superblock_security_struct *sbsec = selinux_superblock(sb);
struct dentry *root = sbsec->sb->s_root;
struct selinux_mnt_opts *opts = mnt_opts;
struct inode_security_struct *root_isec;
@@ -881,8 +869,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
static int selinux_cmp_sb_context(const struct super_block *oldsb,
const struct super_block *newsb)
{
- struct superblock_security_struct *old = oldsb->s_security;
- struct superblock_security_struct *new = newsb->s_security;
+ struct superblock_security_struct *old = selinux_superblock(oldsb);
+ struct superblock_security_struct *new = selinux_superblock(newsb);
char oldflags = old->flags & SE_MNTMASK;
char newflags = new->flags & SE_MNTMASK;
@@ -914,8 +902,9 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
unsigned long *set_kern_flags)
{
int rc = 0;
- const struct superblock_security_struct *oldsbsec = oldsb->s_security;
- struct superblock_security_struct *newsbsec = newsb->s_security;
+ const struct superblock_security_struct *oldsbsec =
+ selinux_superblock(oldsb);
+ struct superblock_security_struct *newsbsec = selinux_superblock(newsb);
int set_fscontext = (oldsbsec->flags & FSCONTEXT_MNT);
int set_context = (oldsbsec->flags & CONTEXT_MNT);
@@ -1085,7 +1074,7 @@ static int show_sid(struct seq_file *m, u32 sid)
static int selinux_sb_show_options(struct seq_file *m, struct super_block *sb)
{
- struct superblock_security_struct *sbsec = sb->s_security;
+ struct superblock_security_struct *sbsec = selinux_superblock(sb);
int rc;
if (!(sbsec->flags & SE_SBINITIALIZED))
@@ -1377,7 +1366,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
if (isec->sclass == SECCLASS_FILE)
isec->sclass = inode_mode_to_security_class(inode->i_mode);
- sbsec = inode->i_sb->s_security;
+ sbsec = selinux_superblock(inode->i_sb);
if (!(sbsec->flags & SE_SBINITIALIZED)) {
/* Defer initialization until selinux_complete_init,
after the initial policy is loaded and the security
@@ -1767,7 +1756,8 @@ selinux_determine_inode_label(const struct task_security_struct *tsec,
const struct qstr *name, u16 tclass,
u32 *_new_isid)
{
- const struct superblock_security_struct *sbsec = dir->i_sb->s_security;
+ const struct superblock_security_struct *sbsec =
+ selinux_superblock(dir->i_sb);
if ((sbsec->flags & SE_SBINITIALIZED) &&
(sbsec->behavior == SECURITY_FS_USE_MNTPOINT)) {
@@ -1798,7 +1788,7 @@ static int may_create(struct inode *dir,
int rc;
dsec = inode_security(dir);
- sbsec = dir->i_sb->s_security;
+ sbsec = selinux_superblock(dir->i_sb);
sid = tsec->sid;
@@ -1947,7 +1937,7 @@ static int superblock_has_perm(const struct cred *cred,
struct superblock_security_struct *sbsec;
u32 sid = cred_sid(cred);
- sbsec = sb->s_security;
+ sbsec = selinux_superblock(sb);
return avc_has_perm(&selinux_state,
sid, sbsec->sid, SECCLASS_FILESYSTEM, perms, ad);
}
@@ -2578,11 +2568,6 @@ static int selinux_sb_alloc_security(struct super_block *sb)
return superblock_alloc_security(sb);
}
-static void selinux_sb_free_security(struct super_block *sb)
-{
- superblock_free_security(sb);
-}
-
static inline int opt_len(const char *s)
{
bool open_quote = false;
@@ -2653,7 +2638,7 @@ static int selinux_sb_eat_lsm_opts(char *options, void **mnt_opts)
static int selinux_sb_remount(struct super_block *sb, void *mnt_opts)
{
struct selinux_mnt_opts *opts = mnt_opts;
- struct superblock_security_struct *sbsec = sb->s_security;
+ struct superblock_security_struct *sbsec = selinux_superblock(sb);
u32 sid;
int rc;
@@ -2877,7 +2862,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
int rc;
char *context;
- sbsec = dir->i_sb->s_security;
+ sbsec = selinux_superblock(dir->i_sb);
newsid = tsec->create_sid;
@@ -3115,7 +3100,7 @@ static int selinux_inode_setxattr(struct dentry *dentry, const char *name,
return dentry_has_perm(current_cred(), dentry, FILE__SETATTR);
}
- sbsec = inode->i_sb->s_security;
+ sbsec = selinux_superblock(inode->i_sb);
if (!(sbsec->flags & SBLABEL_MNT))
return -EOPNOTSUPP;
@@ -3296,13 +3281,14 @@ static int selinux_inode_setsecurity(struct inode *inode, const char *name,
const void *value, size_t size, int flags)
{
struct inode_security_struct *isec = inode_security_novalidate(inode);
- struct superblock_security_struct *sbsec = inode->i_sb->s_security;
+ struct superblock_security_struct *sbsec;
u32 newsid;
int rc;
if (strcmp(name, XATTR_SELINUX_SUFFIX))
return -EOPNOTSUPP;
+ sbsec = selinux_superblock(inode->i_sb);
if (!(sbsec->flags & SBLABEL_MNT))
return -EOPNOTSUPP;
@@ -6647,6 +6633,7 @@ struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
.lbs_inode = sizeof(struct inode_security_struct),
.lbs_ipc = sizeof(struct ipc_security_struct),
.lbs_msg_msg = sizeof(struct msg_security_struct),
+ .lbs_superblock = sizeof(struct superblock_security_struct),
};
static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
@@ -6675,7 +6662,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(fs_context_parse_param, selinux_fs_context_parse_param),
LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security),
- LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security),
LSM_HOOK_INIT(sb_eat_lsm_opts, selinux_sb_eat_lsm_opts),
LSM_HOOK_INIT(sb_free_mnt_opts, selinux_free_mnt_opts),
LSM_HOOK_INIT(sb_remount, selinux_sb_remount),
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 231262d8eac9..d08d7e5d2f93 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -188,4 +188,10 @@ static inline struct ipc_security_struct *selinux_ipc(
return ipc->security + selinux_blob_sizes.lbs_ipc;
}
+static inline struct superblock_security_struct *selinux_superblock(
+ const struct super_block *superblock)
+{
+ return superblock->s_security + selinux_blob_sizes.lbs_superblock;
+}
+
#endif /* _SELINUX_OBJSEC_H_ */
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index ec62918521b1..e3f5d6aece66 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -50,6 +50,7 @@
#include <linux/audit.h>
#include <linux/mutex.h>
#include <linux/vmalloc.h>
+#include <linux/lsm_hooks.h>
#include <net/netlabel.h>
#include "flask.h"
@@ -2751,7 +2752,7 @@ int security_fs_use(struct selinux_state *state, struct super_block *sb)
struct sidtab *sidtab;
int rc = 0;
struct ocontext *c;
- struct superblock_security_struct *sbsec = sb->s_security;
+ struct superblock_security_struct *sbsec = selinux_superblock(sb);
const char *fstype = sb->s_type->name;
read_lock(&state->ss->policy_rwlock);
diff --git a/security/smack/smack.h b/security/smack/smack.h
index cf52af77d15e..caecbcba9942 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -375,6 +375,12 @@ static inline struct smack_known **smack_ipc(const struct kern_ipc_perm *ipc)
return ipc->security + smack_blob_sizes.lbs_ipc;
}
+static inline struct superblock_smack *smack_superblock(
+ const struct super_block *superblock)
+{
+ return superblock->s_security + smack_blob_sizes.lbs_superblock;
+}
+
/*
* Is the directory transmuting?
*/
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 5c1613519d5a..807eff2ccce9 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -540,12 +540,7 @@ static int smack_syslog(int typefrom_file)
*/
static int smack_sb_alloc_security(struct super_block *sb)
{
- struct superblock_smack *sbsp;
-
- sbsp = kzalloc(sizeof(struct superblock_smack), GFP_KERNEL);
-
- if (sbsp == NULL)
- return -ENOMEM;
+ struct superblock_smack *sbsp = smack_superblock(sb);
sbsp->smk_root = &smack_known_floor;
sbsp->smk_default = &smack_known_floor;
@@ -554,22 +549,10 @@ static int smack_sb_alloc_security(struct super_block *sb)
/*
* SMK_SB_INITIALIZED will be zero from kzalloc.
*/
- sb->s_security = sbsp;
return 0;
}
-/**
- * smack_sb_free_security - free a superblock blob
- * @sb: the superblock getting the blob
- *
- */
-static void smack_sb_free_security(struct super_block *sb)
-{
- kfree(sb->s_security);
- sb->s_security = NULL;
-}
-
struct smack_mnt_opts {
const char *fsdefault, *fsfloor, *fshat, *fsroot, *fstransmute;
};
@@ -781,7 +764,7 @@ static int smack_set_mnt_opts(struct super_block *sb,
{
struct dentry *root = sb->s_root;
struct inode *inode = d_backing_inode(root);
- struct superblock_smack *sp = sb->s_security;
+ struct superblock_smack *sp = smack_superblock(sb);
struct inode_smack *isp;
struct smack_known *skp;
struct smack_mnt_opts *opts = mnt_opts;
@@ -880,7 +863,7 @@ static int smack_set_mnt_opts(struct super_block *sb,
*/
static int smack_sb_statfs(struct dentry *dentry)
{
- struct superblock_smack *sbp = dentry->d_sb->s_security;
+ struct superblock_smack *sbp = smack_superblock(dentry->d_sb);
int rc;
struct smk_audit_info ad;
@@ -917,7 +900,7 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
if (isp->smk_task == NULL || isp->smk_task == bsp->smk_task)
return 0;
- sbsp = inode->i_sb->s_security;
+ sbsp = smack_superblock(inode->i_sb);
if ((sbsp->smk_flags & SMK_SB_UNTRUSTED) &&
isp->smk_task != sbsp->smk_root)
return 0;
@@ -1168,7 +1151,7 @@ static int smack_inode_rename(struct inode *old_inode,
*/
static int smack_inode_permission(struct inode *inode, int mask)
{
- struct superblock_smack *sbsp = inode->i_sb->s_security;
+ struct superblock_smack *sbsp = smack_superblock(inode->i_sb);
struct smk_audit_info ad;
int no_block = mask & MAY_NOT_BLOCK;
int rc;
@@ -1410,7 +1393,7 @@ static int smack_inode_removexattr(struct dentry *dentry, const char *name)
*/
if (strcmp(name, XATTR_NAME_SMACK) == 0) {
struct super_block *sbp = dentry->d_sb;
- struct superblock_smack *sbsp = sbp->s_security;
+ struct superblock_smack *sbsp = smack_superblock(sbp);
isp->smk_inode = sbsp->smk_default;
} else if (strcmp(name, XATTR_NAME_SMACKEXEC) == 0)
@@ -1680,7 +1663,7 @@ static int smack_mmap_file(struct file *file,
isp = smack_inode(file_inode(file));
if (isp->smk_mmap == NULL)
return 0;
- sbsp = file_inode(file)->i_sb->s_security;
+ sbsp = smack_superblock(file_inode(file)->i_sb);
if (sbsp->smk_flags & SMK_SB_UNTRUSTED &&
isp->smk_mmap != sbsp->smk_root)
return -EACCES;
@@ -3288,7 +3271,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
goto unlockandout;
sbp = inode->i_sb;
- sbsp = sbp->s_security;
+ sbsp = smack_superblock(sbp);
/*
* We're going to use the superblock default label
* if there's no label on the file.
@@ -4575,6 +4558,7 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
.lbs_inode = sizeof(struct inode_smack),
.lbs_ipc = sizeof(struct smack_known *),
.lbs_msg_msg = sizeof(struct smack_known *),
+ .lbs_superblock = sizeof(struct superblock_smack),
};
static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
@@ -4586,7 +4570,6 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(fs_context_parse_param, smack_fs_context_parse_param),
LSM_HOOK_INIT(sb_alloc_security, smack_sb_alloc_security),
- LSM_HOOK_INIT(sb_free_security, smack_sb_free_security),
LSM_HOOK_INIT(sb_free_mnt_opts, smack_free_mnt_opts),
LSM_HOOK_INIT(sb_eat_lsm_opts, smack_sb_eat_lsm_opts),
LSM_HOOK_INIT(sb_statfs, smack_sb_statfs),
--
2.19.1
^ 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