| Message ID | 20190411201154.167617-1-mortonm@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Thu, Apr 11, 2019 at 1:12 PM Micah Morton <mortonm@chromium.org> wrote: > > From: Jann Horn <jannh@google.com> > > For debugging a running system, it is very helpful to be able to see what > policy the system is using. Add a read handler that can dump out a copy of > the loaded policy. > > Signed-off-by: Jann Horn <jannh@google.com> > Signed-off-by: Micah Morton <mortonm@chromium.org> Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > --- > Changes since the last patch set: Instead of doing refcounting, change > policy_update_lock to a mutex and hold the mutex across the policy read. > security/safesetid/lsm.h | 1 + > security/safesetid/securityfs.c | 35 +++++++++++++++++++++++++++++---- > 2 files changed, 32 insertions(+), 4 deletions(-) > > diff --git a/security/safesetid/lsm.h b/security/safesetid/lsm.h > index 4a34f558d964..db6d16e6bbc3 100644 > --- a/security/safesetid/lsm.h > +++ b/security/safesetid/lsm.h > @@ -41,6 +41,7 @@ struct setuid_rule { > > struct setuid_ruleset { > DECLARE_HASHTABLE(rules, SETID_HASH_BITS); > + char *policy_str; > struct rcu_head rcu; > }; > > diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c > index 250d59e046c1..997b403c6255 100644 > --- a/security/safesetid/securityfs.c > +++ b/security/safesetid/securityfs.c > @@ -19,7 +19,7 @@ > > #include "lsm.h" > > -static DEFINE_SPINLOCK(policy_update_lock); > +static DEFINE_MUTEX(policy_update_lock); > > /* > * In the case the input buffer contains one or more invalid UIDs, the kuid_t > @@ -67,6 +67,7 @@ static void __release_ruleset(struct rcu_head *rcu) > > hash_for_each_safe(pol->rules, bucket, tmp, rule, next) > kfree(rule); > + kfree(pol->policy_str); > kfree(pol); > } > > @@ -85,6 +86,7 @@ static ssize_t handle_policy_update(struct file *file, > pol = kmalloc(sizeof(struct setuid_ruleset), GFP_KERNEL); > if (!pol) > return -ENOMEM; > + pol->policy_str = NULL; > hash_init(pol->rules); > > p = buf = memdup_user_nul(ubuf, len); > @@ -92,6 +94,11 @@ static ssize_t handle_policy_update(struct file *file, > err = PTR_ERR(buf); > goto out_free_pol; > } > + pol->policy_str = kstrdup(buf, GFP_KERNEL); > + if (pol->policy_str == NULL) { > + err = -ENOMEM; > + goto out_free_buf; > + } > > /* policy lines, including the last one, end with \n */ > while (*p != '\0') { > @@ -135,10 +142,10 @@ static ssize_t handle_policy_update(struct file *file, > * What we really want here is an xchg() wrapper for RCU, but since that > * doesn't currently exist, just use a spinlock for now. > */ > - spin_lock(&policy_update_lock); > + mutex_lock(&policy_update_lock); > rcu_swap_protected(safesetid_setuid_rules, pol, > lockdep_is_held(&policy_update_lock)); > - spin_unlock(&policy_update_lock); > + mutex_unlock(&policy_update_lock); > err = len; > > out_free_buf: > @@ -162,7 +169,27 @@ static ssize_t safesetid_file_write(struct file *file, > return handle_policy_update(file, buf, len); > } > > +static ssize_t safesetid_file_read(struct file *file, char __user *buf, > + size_t len, loff_t *ppos) > +{ > + ssize_t res = 0; > + struct setuid_ruleset *pol; > + const char *kbuf; > + > + mutex_lock(&policy_update_lock); > + pol = rcu_dereference_protected(safesetid_setuid_rules, > + lockdep_is_held(&policy_update_lock)); > + if (pol) { > + kbuf = pol->policy_str; > + res = simple_read_from_buffer(buf, len, ppos, > + kbuf, strlen(kbuf)); > + } > + mutex_unlock(&policy_update_lock); > + return res; > +} > + > static const struct file_operations safesetid_file_fops = { > + .read = safesetid_file_read, > .write = safesetid_file_write, > }; > > @@ -181,7 +208,7 @@ static int __init safesetid_init_securityfs(void) > goto error; > } > > - policy_file = securityfs_create_file("whitelist_policy", 0200, > + policy_file = securityfs_create_file("whitelist_policy", 0600, > policy_dir, NULL, &safesetid_file_fops); > if (IS_ERR(policy_file)) { > ret = PTR_ERR(policy_file); > -- > 2.21.0.392.gf8f6787159e-goog
Ready for merge. On Thu, Apr 11, 2019 at 1:38 PM Kees Cook <keescook@chromium.org> wrote: > > On Thu, Apr 11, 2019 at 1:12 PM Micah Morton <mortonm@chromium.org> wrote: > > > > From: Jann Horn <jannh@google.com> > > > > For debugging a running system, it is very helpful to be able to see what > > policy the system is using. Add a read handler that can dump out a copy of > > the loaded policy. > > > > Signed-off-by: Jann Horn <jannh@google.com> > > Signed-off-by: Micah Morton <mortonm@chromium.org> > > Reviewed-by: Kees Cook <keescook@chromium.org> > > -Kees > > > --- > > Changes since the last patch set: Instead of doing refcounting, change > > policy_update_lock to a mutex and hold the mutex across the policy read. > > security/safesetid/lsm.h | 1 + > > security/safesetid/securityfs.c | 35 +++++++++++++++++++++++++++++---- > > 2 files changed, 32 insertions(+), 4 deletions(-) > > > > diff --git a/security/safesetid/lsm.h b/security/safesetid/lsm.h > > index 4a34f558d964..db6d16e6bbc3 100644 > > --- a/security/safesetid/lsm.h > > +++ b/security/safesetid/lsm.h > > @@ -41,6 +41,7 @@ struct setuid_rule { > > > > struct setuid_ruleset { > > DECLARE_HASHTABLE(rules, SETID_HASH_BITS); > > + char *policy_str; > > struct rcu_head rcu; > > }; > > > > diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c > > index 250d59e046c1..997b403c6255 100644 > > --- a/security/safesetid/securityfs.c > > +++ b/security/safesetid/securityfs.c > > @@ -19,7 +19,7 @@ > > > > #include "lsm.h" > > > > -static DEFINE_SPINLOCK(policy_update_lock); > > +static DEFINE_MUTEX(policy_update_lock); > > > > /* > > * In the case the input buffer contains one or more invalid UIDs, the kuid_t > > @@ -67,6 +67,7 @@ static void __release_ruleset(struct rcu_head *rcu) > > > > hash_for_each_safe(pol->rules, bucket, tmp, rule, next) > > kfree(rule); > > + kfree(pol->policy_str); > > kfree(pol); > > } > > > > @@ -85,6 +86,7 @@ static ssize_t handle_policy_update(struct file *file, > > pol = kmalloc(sizeof(struct setuid_ruleset), GFP_KERNEL); > > if (!pol) > > return -ENOMEM; > > + pol->policy_str = NULL; > > hash_init(pol->rules); > > > > p = buf = memdup_user_nul(ubuf, len); > > @@ -92,6 +94,11 @@ static ssize_t handle_policy_update(struct file *file, > > err = PTR_ERR(buf); > > goto out_free_pol; > > } > > + pol->policy_str = kstrdup(buf, GFP_KERNEL); > > + if (pol->policy_str == NULL) { > > + err = -ENOMEM; > > + goto out_free_buf; > > + } > > > > /* policy lines, including the last one, end with \n */ > > while (*p != '\0') { > > @@ -135,10 +142,10 @@ static ssize_t handle_policy_update(struct file *file, > > * What we really want here is an xchg() wrapper for RCU, but since that > > * doesn't currently exist, just use a spinlock for now. > > */ > > - spin_lock(&policy_update_lock); > > + mutex_lock(&policy_update_lock); > > rcu_swap_protected(safesetid_setuid_rules, pol, > > lockdep_is_held(&policy_update_lock)); > > - spin_unlock(&policy_update_lock); > > + mutex_unlock(&policy_update_lock); > > err = len; > > > > out_free_buf: > > @@ -162,7 +169,27 @@ static ssize_t safesetid_file_write(struct file *file, > > return handle_policy_update(file, buf, len); > > } > > > > +static ssize_t safesetid_file_read(struct file *file, char __user *buf, > > + size_t len, loff_t *ppos) > > +{ > > + ssize_t res = 0; > > + struct setuid_ruleset *pol; > > + const char *kbuf; > > + > > + mutex_lock(&policy_update_lock); > > + pol = rcu_dereference_protected(safesetid_setuid_rules, > > + lockdep_is_held(&policy_update_lock)); > > + if (pol) { > > + kbuf = pol->policy_str; > > + res = simple_read_from_buffer(buf, len, ppos, > > + kbuf, strlen(kbuf)); > > + } > > + mutex_unlock(&policy_update_lock); > > + return res; > > +} > > + > > static const struct file_operations safesetid_file_fops = { > > + .read = safesetid_file_read, > > .write = safesetid_file_write, > > }; > > > > @@ -181,7 +208,7 @@ static int __init safesetid_init_securityfs(void) > > goto error; > > } > > > > - policy_file = securityfs_create_file("whitelist_policy", 0200, > > + policy_file = securityfs_create_file("whitelist_policy", 0600, > > policy_dir, NULL, &safesetid_file_fops); > > if (IS_ERR(policy_file)) { > > ret = PTR_ERR(policy_file); > > -- > > 2.21.0.392.gf8f6787159e-goog > > > > -- > Kees Cook
diff --git a/security/safesetid/lsm.h b/security/safesetid/lsm.h index 4a34f558d964..db6d16e6bbc3 100644 --- a/security/safesetid/lsm.h +++ b/security/safesetid/lsm.h @@ -41,6 +41,7 @@ struct setuid_rule { struct setuid_ruleset { DECLARE_HASHTABLE(rules, SETID_HASH_BITS); + char *policy_str; struct rcu_head rcu; }; diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c index 250d59e046c1..997b403c6255 100644 --- a/security/safesetid/securityfs.c +++ b/security/safesetid/securityfs.c @@ -19,7 +19,7 @@ #include "lsm.h" -static DEFINE_SPINLOCK(policy_update_lock); +static DEFINE_MUTEX(policy_update_lock); /* * In the case the input buffer contains one or more invalid UIDs, the kuid_t @@ -67,6 +67,7 @@ static void __release_ruleset(struct rcu_head *rcu) hash_for_each_safe(pol->rules, bucket, tmp, rule, next) kfree(rule); + kfree(pol->policy_str); kfree(pol); } @@ -85,6 +86,7 @@ static ssize_t handle_policy_update(struct file *file, pol = kmalloc(sizeof(struct setuid_ruleset), GFP_KERNEL); if (!pol) return -ENOMEM; + pol->policy_str = NULL; hash_init(pol->rules); p = buf = memdup_user_nul(ubuf, len); @@ -92,6 +94,11 @@ static ssize_t handle_policy_update(struct file *file, err = PTR_ERR(buf); goto out_free_pol; } + pol->policy_str = kstrdup(buf, GFP_KERNEL); + if (pol->policy_str == NULL) { + err = -ENOMEM; + goto out_free_buf; + } /* policy lines, including the last one, end with \n */ while (*p != '\0') { @@ -135,10 +142,10 @@ static ssize_t handle_policy_update(struct file *file, * What we really want here is an xchg() wrapper for RCU, but since that * doesn't currently exist, just use a spinlock for now. */ - spin_lock(&policy_update_lock); + mutex_lock(&policy_update_lock); rcu_swap_protected(safesetid_setuid_rules, pol, lockdep_is_held(&policy_update_lock)); - spin_unlock(&policy_update_lock); + mutex_unlock(&policy_update_lock); err = len; out_free_buf: @@ -162,7 +169,27 @@ static ssize_t safesetid_file_write(struct file *file, return handle_policy_update(file, buf, len); } +static ssize_t safesetid_file_read(struct file *file, char __user *buf, + size_t len, loff_t *ppos) +{ + ssize_t res = 0; + struct setuid_ruleset *pol; + const char *kbuf; + + mutex_lock(&policy_update_lock); + pol = rcu_dereference_protected(safesetid_setuid_rules, + lockdep_is_held(&policy_update_lock)); + if (pol) { + kbuf = pol->policy_str; + res = simple_read_from_buffer(buf, len, ppos, + kbuf, strlen(kbuf)); + } + mutex_unlock(&policy_update_lock); + return res; +} + static const struct file_operations safesetid_file_fops = { + .read = safesetid_file_read, .write = safesetid_file_write, }; @@ -181,7 +208,7 @@ static int __init safesetid_init_securityfs(void) goto error; } - policy_file = securityfs_create_file("whitelist_policy", 0200, + policy_file = securityfs_create_file("whitelist_policy", 0600, policy_dir, NULL, &safesetid_file_fops); if (IS_ERR(policy_file)) { ret = PTR_ERR(policy_file);