Subject: [PATCH] Smack: correct behavior in the mmap hook
The mmap policy enforcement was not properly handling the
interaction between the global and local rule lists.
Instead of going through one and then the other, which
missed the important case where a rule specified that
there should be no access, combine the access limitations
where there is a rule in each list.
Signed-off-by: Casey Schaufler <[email protected]>
---
security/smack/smack_lsm.c | 85 +++++++++++++++++++++++++------------------
1 files changed, 49 insertions(+), 36 deletions(-)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 123a499..92cb715 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1110,38 +1110,6 @@ static int smack_file_fcntl(struct file *file, unsigned int cmd,
}
/**
- * smk_mmap_list_check - the mmap check
- * @sub: subject label
- * @obj: object label
- * @access: access mode
- * @local: the task specific rule list
- *
- * Returns 0 if acces is permitted, -EACCES otherwise
- */
-static int smk_mmap_list_check(char *sub, char *obj, int access,
- struct list_head *local)
-{
- int may;
-
- /*
- * If there is not a global rule that
- * allows access say no.
- */
- may = smk_access_entry(sub, obj, &smack_rule_list);
- if (may == -ENOENT || (may & access) != access)
- return -EACCES;
- /*
- * If there is a task local rule that
- * denies access say no.
- */
- may = smk_access_entry(sub, obj, local);
- if (may != -ENOENT && (may & access) != access)
- return -EACCES;
-
- return 0;
-}
-
-/**
* smack_file_mmap :
* Check permissions for a mmap operation. The @file may be NULL, e.g.
* if mapping anonymous memory.
@@ -1160,8 +1128,12 @@ static int smack_file_mmap(struct file *file,
struct task_smack *tsp;
char *sp;
char *msmack;
+ char *osmack;
struct inode_smack *isp;
struct dentry *dp;
+ int may;
+ int mmay;
+ int tmay;
int rc;
/* do DAC check on address space usage */
@@ -1199,16 +1171,57 @@ static int smack_file_mmap(struct file *file,
list_for_each_entry_rcu(srp, &smack_rule_list, list) {
if (srp->smk_subject != sp)
continue;
+
+ osmack = srp->smk_object;
/*
* Matching labels always allows access.
*/
- if (msmack == srp->smk_object)
+ if (msmack == osmack)
+ continue;
+ /*
+ * If there is a matching local rule take
+ * that into account as well.
+ */
+ may = smk_access_entry(srp->smk_subject, osmack,
+ &tsp->smk_rules);
+ if (may == -ENOENT)
+ may = srp->smk_access;
+ else
+ may &= srp->smk_access;
+ /*
+ * If may is zero the SMACK64MMAP subject can't
+ * possibly have less access.
+ */
+ if (may == 0)
continue;
- rc = smk_mmap_list_check(msmack, srp->smk_object,
- srp->smk_access, &tsp->smk_rules);
- if (rc != 0)
+ /*
+ * Fetch the global list entry.
+ * If there isn't one a SMACK64MMAP subject
+ * can't have as much access as current.
+ */
+ mmay = smk_access_entry(msmack, osmack, &smack_rule_list);
+ if (mmay == -ENOENT) {
+ rc = -EACCES;
break;
+ }
+ /*
+ * If there is a local entry it modifies the
+ * potential access, too.
+ */
+ tmay = smk_access_entry(msmack, osmack, &tsp->smk_rules);
+ if (tmay != -ENOENT)
+ mmay &= tmay;
+
+ /*
+ * If there is any access available to current that is
+ * not available to a SMACK64MMAP subject
+ * deny access.
+ */
+ if ((may | mmay) != may) {
+ rc = -EACCES;
+ break;
+ }
}
rcu_read_unlock();
On Tue, 8 Feb 2011, Casey Schaufler wrote:
>
> Subject: [PATCH] Smack: correct behavior in the mmap hook
>
> The mmap policy enforcement was not properly handling the
> interaction between the global and local rule lists.
> Instead of going through one and then the other, which
> missed the important case where a rule specified that
> there should be no access, combine the access limitations
> where there is a rule in each list.
>
> Signed-off-by: Casey Schaufler <[email protected]>
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
--
James Morris
<[email protected]>
On 2/8/2011 11:51 PM, James Morris wrote:
> On Tue, 8 Feb 2011, Casey Schaufler wrote:
>
> Subject: [PATCH] Smack: correct behavior in the mmap hook
>
> The mmap policy enforcement was not properly handling the
> interaction between the global and local rule lists.
> Instead of going through one and then the other, which
> missed the important case where a rule specified that
> there should be no access, combine the access limitations
> where there is a rule in each list.
>
> Signed-off-by: Casey Schaufler <[email protected]>
> Applied to
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
>
James, I made an error and failed to say that this and Shan Wei's clean-up
should be pulled from git://gitorious.org/smack-next/kernel.git
On Wed, 9 Feb 2011, Casey Schaufler wrote:
> > Signed-off-by: Casey Schaufler <[email protected]>
> > Applied to
> > git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
> >
>
> James, I made an error and failed to say that this and Shan Wei's clean-up
> should be pulled from git://gitorious.org/smack-next/kernel.git
Please use git-request-pull to generate the message.
--
James Morris
<[email protected]>
On 2/9/2011 1:53 PM, James Morris wrote:
> On Wed, 9 Feb 2011, Casey Schaufler wrote:
>
>>> Signed-off-by: Casey Schaufler <[email protected]>
>>> Applied to
>>> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
>>>
>> James, I made an error and failed to say that this and Shan Wei's clean-up
>> should be pulled from git://gitorious.org/smack-next/kernel.git
> Please use git-request-pull to generate the message.
The following changes since commit 7898e1f8e9eb1bee88c92d636e0ab93f2cbe31c6:
Casey Schaufler (1):
Subject: [PATCH] Smack: mmap controls for library containment
are available in the git repository at:
git://gitorious.org/smack-next/kernel.git master
Casey Schaufler (2):
Smack: correct behavior in the mmap hook
Smack: correct final mmap check comparison
Shan Wei (1):
security:smack: kill unused SMACK_LIST_MAX, MAY_ANY and MAY_ANYWRITE
security/smack/smack.h | 8 ----
security/smack/smack_lsm.c | 85 +++++++++++++++++++++++++------------------
2 files changed, 49 insertions(+), 44 deletions(-)
On Wed, 9 Feb 2011, Casey Schaufler wrote:
> Shan Wei (1):
> security:smack: kill unused SMACK_LIST_MAX, MAY_ANY and MAY_ANYWRITE
This one is missing a signoff by you. You need to use git-am -s
Also, I already have 'Smack: correct behavior in the mmap hook' in my
tree. Create a for-security-next (or whatever) branch for me to pull
from, based on my current tree, without that patch. (Otherwise I have to
ask everyone else to rebase).
--
James Morris
<[email protected]>
The following changes since commit 0e0a070d3a47d279de66e08244769556deae2eee:
Casey Schaufler (1):
Smack: correct behavior in the mmap hook
are available in the git repository at:
git://gitorious.org/smack-next/kernel.git next
Casey Schaufler (1):
Smack: correct final mmap check comparison
Shan Wei (1):
security:smack: kill unused SMACK_LIST_MAX, MAY_ANY and MAY_ANYWRITE
security/smack/smack.h | 8 --------
security/smack/smack_lsm.c | 2 +-
2 files changed, 1 insertions(+), 9 deletions(-)
On Wed, 9 Feb 2011, Casey Schaufler wrote:
> The following changes since commit 0e0a070d3a47d279de66e08244769556deae2eee:
> Casey Schaufler (1):
> Smack: correct behavior in the mmap hook
>
> are available in the git repository at:
>
> git://gitorious.org/smack-next/kernel.git next
>
> Casey Schaufler (1):
> Smack: correct final mmap check comparison
>
> Shan Wei (1):
> security:smack: kill unused SMACK_LIST_MAX, MAY_ANY and MAY_ANYWRITE
Thanks, looks good :-)
--
James Morris
<[email protected]>