From 8c9e80ed276fc4b9c9fadf29d8bf6b3576112f1a Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Thu, 21 Apr 2011 17:23:19 -0700 Subject: SECURITY: Move exec_permission RCU checks into security modules Right now all RCU walks fall back to reference walk when CONFIG_SECURITY is enabled, even though just the standard capability module is active. This is because security_inode_exec_permission unconditionally fails RCU walks. Move this decision to the low level security module. This requires passing the RCU flags down the security hook. This way at least the capability module and a few easy cases in selinux/smack work with RCU walks with CONFIG_SECURITY=y Signed-off-by: Andi Kleen Acked-by: Eric Paris Signed-off-by: Linus Torvalds --- security/selinux/hooks.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index f9c3764e485..a73f4e46377 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2635,7 +2635,7 @@ static int selinux_inode_follow_link(struct dentry *dentry, struct nameidata *na return dentry_has_perm(cred, NULL, dentry, FILE__READ); } -static int selinux_inode_permission(struct inode *inode, int mask) +static int selinux_inode_permission(struct inode *inode, int mask, unsigned flags) { const struct cred *cred = current_cred(); struct common_audit_data ad; @@ -2649,6 +2649,10 @@ static int selinux_inode_permission(struct inode *inode, int mask) if (!mask) return 0; + /* May be droppable after audit */ + if (flags & IPERM_FLAG_RCU) + return -ECHILD; + COMMON_AUDIT_DATA_INIT(&ad, FS); ad.u.fs.inode = inode; -- cgit v1.2.3-70-g09d2 From a35c6c8368d88deae6890205e73ed330b6df1db7 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Wed, 20 Apr 2011 10:21:28 -0400 Subject: SELinux: silence build warning when !CONFIG_BUG If one builds a kernel without CONFIG_BUG there are a number of 'may be used uninitialized' warnings. Silence these by returning after the BUG(). Signed-off-by: Eric Paris Reviewed-by: James Morris --- security/selinux/hooks.c | 2 ++ security/selinux/netnode.c | 1 + 2 files changed, 3 insertions(+) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index d52a9250741..7a630a8a5ce 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -989,6 +989,7 @@ static void selinux_write_opts(struct seq_file *m, continue; default: BUG(); + return; }; /* we need a comma before each option */ seq_putc(m, ','); @@ -1442,6 +1443,7 @@ static int task_has_capability(struct task_struct *tsk, printk(KERN_ERR "SELinux: out of range capability %d\n", cap); BUG(); + return -EINVAL; } rc = avc_has_perm_noaudit(sid, sid, sclass, av, 0, &avd); diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c index 65ebfe954f8..3618251d0fd 100644 --- a/security/selinux/netnode.c +++ b/security/selinux/netnode.c @@ -141,6 +141,7 @@ static struct sel_netnode *sel_netnode_find(const void *addr, u16 family) break; default: BUG(); + return NULL; } list_for_each_entry_rcu(node, &sel_netnode_hash[idx].list, list) -- cgit v1.2.3-70-g09d2 From 1c9904297451f558191e211a48d8838b4bf792b0 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Thu, 21 Apr 2011 17:23:19 -0700 Subject: SECURITY: Move exec_permission RCU checks into security modules Right now all RCU walks fall back to reference walk when CONFIG_SECURITY is enabled, even though just the standard capability module is active. This is because security_inode_exec_permission unconditionally fails RCU walks. Move this decision to the low level security module. This requires passing the RCU flags down the security hook. This way at least the capability module and a few easy cases in selinux/smack work with RCU walks with CONFIG_SECURITY=y Signed-off-by: Andi Kleen Signed-off-by: Eric Paris --- include/linux/security.h | 2 +- security/capability.c | 2 +- security/security.c | 6 ++---- security/selinux/hooks.c | 6 +++++- security/smack/smack_lsm.c | 6 +++++- 5 files changed, 14 insertions(+), 8 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/security.h b/include/linux/security.h index 84a202ac3de..2f99ecd0fb2 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1454,7 +1454,7 @@ struct security_operations { struct inode *new_dir, struct dentry *new_dentry); int (*inode_readlink) (struct dentry *dentry); int (*inode_follow_link) (struct dentry *dentry, struct nameidata *nd); - int (*inode_permission) (struct inode *inode, int mask); + int (*inode_permission) (struct inode *inode, int mask, unsigned flags); int (*inode_setattr) (struct dentry *dentry, struct iattr *attr); int (*inode_getattr) (struct vfsmount *mnt, struct dentry *dentry); int (*inode_setxattr) (struct dentry *dentry, const char *name, diff --git a/security/capability.c b/security/capability.c index ab3d807accc..56bb1605fd7 100644 --- a/security/capability.c +++ b/security/capability.c @@ -181,7 +181,7 @@ static int cap_inode_follow_link(struct dentry *dentry, return 0; } -static int cap_inode_permission(struct inode *inode, int mask) +static int cap_inode_permission(struct inode *inode, int mask, unsigned flags) { return 0; } diff --git a/security/security.c b/security/security.c index 47b8a447118..7e34f98bf43 100644 --- a/security/security.c +++ b/security/security.c @@ -514,16 +514,14 @@ int security_inode_permission(struct inode *inode, int mask) { if (unlikely(IS_PRIVATE(inode))) return 0; - return security_ops->inode_permission(inode, mask); + return security_ops->inode_permission(inode, mask, 0); } int security_inode_exec_permission(struct inode *inode, unsigned int flags) { if (unlikely(IS_PRIVATE(inode))) return 0; - if (flags) - return -ECHILD; - return security_ops->inode_permission(inode, MAY_EXEC); + return security_ops->inode_permission(inode, MAY_EXEC, flags); } int security_inode_setattr(struct dentry *dentry, struct iattr *attr) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 7a630a8a5ce..9a220be17a3 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2635,7 +2635,7 @@ static int selinux_inode_follow_link(struct dentry *dentry, struct nameidata *na return dentry_has_perm(cred, NULL, dentry, FILE__READ); } -static int selinux_inode_permission(struct inode *inode, int mask) +static int selinux_inode_permission(struct inode *inode, int mask, unsigned flags) { const struct cred *cred = current_cred(); struct common_audit_data ad; @@ -2649,6 +2649,10 @@ static int selinux_inode_permission(struct inode *inode, int mask) if (!mask) return 0; + /* May be droppable after audit */ + if (flags & IPERM_FLAG_RCU) + return -ECHILD; + COMMON_AUDIT_DATA_INIT(&ad, FS); ad.u.fs.inode = inode; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 23c7a6d0c80..42fcb47747a 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -686,7 +686,7 @@ static int smack_inode_rename(struct inode *old_inode, * * Returns 0 if access is permitted, -EACCES otherwise */ -static int smack_inode_permission(struct inode *inode, int mask) +static int smack_inode_permission(struct inode *inode, int mask, unsigned flags) { struct smk_audit_info ad; @@ -696,6 +696,10 @@ static int smack_inode_permission(struct inode *inode, int mask) */ if (mask == 0) return 0; + + /* May be droppable after audit */ + if (flags & IPERM_FLAG_RCU) + return -ECHILD; smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); smk_ad_setfield_u_fs_inode(&ad, inode); return smk_curacc(smk_of_inode(inode), mask, &ad); -- cgit v1.2.3-70-g09d2 From 0dc1ba24f7fff659725eecbba2c9ad679a0954cd Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Thu, 21 Apr 2011 17:23:20 -0700 Subject: SELINUX: Make selinux cache VFS RCU walks safe Now that the security modules can decide whether they support the dcache RCU walk or not it's possible to make selinux a bit more RCU friendly. The SELinux AVC and security server access decision code is RCU safe. A specific piece of the LSM audit code may not be RCU safe. This patch makes the VFS RCU walk retry if it would hit the non RCU safe chunk of code. It will normally just work under RCU. This is done simply by passing the VFS RCU state as a flag down into the avc_audit() code and returning ECHILD there if it would have an issue. Based-on-patch-by: Andi Kleen Signed-off-by: Eric Paris --- security/selinux/avc.c | 36 +++++++++++++++++++++++++++++------- security/selinux/hooks.c | 26 +++++++++++++------------- security/selinux/include/avc.h | 18 +++++++++++++----- 3 files changed, 55 insertions(+), 25 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/avc.c b/security/selinux/avc.c index 9da6420e205..1d027e29ce8 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -471,6 +471,7 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a) * @avd: access vector decisions * @result: result from avc_has_perm_noaudit * @a: auxiliary audit data + * @flags: VFS walk flags * * Audit the granting or denial of permissions in accordance * with the policy. This function is typically called by @@ -481,9 +482,10 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a) * be performed under a lock, to allow the lock to be released * before calling the auditing code. */ -void avc_audit(u32 ssid, u32 tsid, +int avc_audit(u32 ssid, u32 tsid, u16 tclass, u32 requested, - struct av_decision *avd, int result, struct common_audit_data *a) + struct av_decision *avd, int result, struct common_audit_data *a, + unsigned flags) { struct common_audit_data stack_data; u32 denied, audited; @@ -515,11 +517,24 @@ void avc_audit(u32 ssid, u32 tsid, else audited = requested & avd->auditallow; if (!audited) - return; + return 0; + if (!a) { a = &stack_data; COMMON_AUDIT_DATA_INIT(a, NONE); } + + /* + * When in a RCU walk do the audit on the RCU retry. This is because + * the collection of the dname in an inode audit message is not RCU + * safe. Note this may drop some audits when the situation changes + * during retry. However this is logically just as if the operation + * happened a little later. + */ + if ((a->type == LSM_AUDIT_DATA_FS) && + (flags & IPERM_FLAG_RCU)) + return -ECHILD; + a->selinux_audit_data.tclass = tclass; a->selinux_audit_data.requested = requested; a->selinux_audit_data.ssid = ssid; @@ -529,6 +544,7 @@ void avc_audit(u32 ssid, u32 tsid, a->lsm_pre_audit = avc_audit_pre_callback; a->lsm_post_audit = avc_audit_post_callback; common_lsm_audit(a); + return 0; } /** @@ -793,6 +809,7 @@ int avc_has_perm_noaudit(u32 ssid, u32 tsid, * @tclass: target security class * @requested: requested permissions, interpreted based on @tclass * @auditdata: auxiliary audit data + * @flags: VFS walk flags * * Check the AVC to determine whether the @requested permissions are granted * for the SID pair (@ssid, @tsid), interpreting the permissions @@ -802,14 +819,19 @@ int avc_has_perm_noaudit(u32 ssid, u32 tsid, * permissions are granted, -%EACCES if any permissions are denied, or * another -errno upon other errors. */ -int avc_has_perm(u32 ssid, u32 tsid, u16 tclass, - u32 requested, struct common_audit_data *auditdata) +int avc_has_perm_flags(u32 ssid, u32 tsid, u16 tclass, + u32 requested, struct common_audit_data *auditdata, + unsigned flags) { struct av_decision avd; - int rc; + int rc, rc2; rc = avc_has_perm_noaudit(ssid, tsid, tclass, requested, 0, &avd); - avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata); + + rc2 = avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata, + flags); + if (rc2) + return rc2; return rc; } diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 9a220be17a3..ed5f29aa0a3 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1447,8 +1447,11 @@ static int task_has_capability(struct task_struct *tsk, } rc = avc_has_perm_noaudit(sid, sid, sclass, av, 0, &avd); - if (audit == SECURITY_CAP_AUDIT) - avc_audit(sid, sid, sclass, av, &avd, rc, &ad); + if (audit == SECURITY_CAP_AUDIT) { + int rc2 = avc_audit(sid, sid, sclass, av, &avd, rc, &ad, 0); + if (rc2) + return rc2; + } return rc; } @@ -1468,7 +1471,8 @@ static int task_has_system(struct task_struct *tsk, static int inode_has_perm(const struct cred *cred, struct inode *inode, u32 perms, - struct common_audit_data *adp) + struct common_audit_data *adp, + unsigned flags) { struct inode_security_struct *isec; struct common_audit_data ad; @@ -1488,7 +1492,7 @@ static int inode_has_perm(const struct cred *cred, ad.u.fs.inode = inode; } - return avc_has_perm(sid, isec->sid, isec->sclass, perms, adp); + return avc_has_perm_flags(sid, isec->sid, isec->sclass, perms, adp, flags); } /* Same as inode_has_perm, but pass explicit audit data containing @@ -1505,7 +1509,7 @@ static inline int dentry_has_perm(const struct cred *cred, COMMON_AUDIT_DATA_INIT(&ad, FS); ad.u.fs.path.mnt = mnt; ad.u.fs.path.dentry = dentry; - return inode_has_perm(cred, inode, av, &ad); + return inode_has_perm(cred, inode, av, &ad, 0); } /* Check whether a task can use an open file descriptor to @@ -1541,7 +1545,7 @@ static int file_has_perm(const struct cred *cred, /* av is zero if only checking access to the descriptor. */ rc = 0; if (av) - rc = inode_has_perm(cred, inode, av, &ad); + rc = inode_has_perm(cred, inode, av, &ad, 0); out: return rc; @@ -2103,7 +2107,7 @@ static inline void flush_unauthorized_files(const struct cred *cred, file = file_priv->file; inode = file->f_path.dentry->d_inode; if (inode_has_perm(cred, inode, - FILE__READ | FILE__WRITE, NULL)) { + FILE__READ | FILE__WRITE, NULL, 0)) { drop_tty = 1; } } @@ -2649,10 +2653,6 @@ static int selinux_inode_permission(struct inode *inode, int mask, unsigned flag if (!mask) return 0; - /* May be droppable after audit */ - if (flags & IPERM_FLAG_RCU) - return -ECHILD; - COMMON_AUDIT_DATA_INIT(&ad, FS); ad.u.fs.inode = inode; @@ -2661,7 +2661,7 @@ static int selinux_inode_permission(struct inode *inode, int mask, unsigned flag perms = file_mask_to_av(inode->i_mode, mask); - return inode_has_perm(cred, inode, perms, &ad); + return inode_has_perm(cred, inode, perms, &ad, flags); } static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr) @@ -3208,7 +3208,7 @@ static int selinux_dentry_open(struct file *file, const struct cred *cred) * new inode label or new policy. * This check is not redundant - do not remove. */ - return inode_has_perm(cred, inode, open_file_to_av(file), NULL); + return inode_has_perm(cred, inode, open_file_to_av(file), NULL, 0); } /* task security operations */ diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h index 5615081b73e..e77b2ac2908 100644 --- a/security/selinux/include/avc.h +++ b/security/selinux/include/avc.h @@ -54,11 +54,11 @@ struct avc_cache_stats { void __init avc_init(void); -void avc_audit(u32 ssid, u32 tsid, +int avc_audit(u32 ssid, u32 tsid, u16 tclass, u32 requested, struct av_decision *avd, int result, - struct common_audit_data *a); + struct common_audit_data *a, unsigned flags); #define AVC_STRICT 1 /* Ignore permissive mode. */ int avc_has_perm_noaudit(u32 ssid, u32 tsid, @@ -66,9 +66,17 @@ int avc_has_perm_noaudit(u32 ssid, u32 tsid, unsigned flags, struct av_decision *avd); -int avc_has_perm(u32 ssid, u32 tsid, - u16 tclass, u32 requested, - struct common_audit_data *auditdata); +int avc_has_perm_flags(u32 ssid, u32 tsid, + u16 tclass, u32 requested, + struct common_audit_data *auditdata, + unsigned); + +static inline int avc_has_perm(u32 ssid, u32 tsid, + u16 tclass, u32 requested, + struct common_audit_data *auditdata) +{ + return avc_has_perm_flags(ssid, tsid, tclass, requested, auditdata, 0); +} u32 avc_policy_seqno(void); -- cgit v1.2.3-70-g09d2 From f48b7399840b453e7282b523f535561fe9638a2d Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Mon, 25 Apr 2011 12:54:27 -0400 Subject: LSM: split LSM_AUDIT_DATA_FS into _PATH and _INODE The lsm common audit code has wacky contortions making sure which pieces of information are set based on if it was given a path, dentry, or inode. Split this into path and inode to get rid of some of the code complexity. Signed-off-by: Eric Paris Acked-by: Casey Schaufler --- include/linux/lsm_audit.h | 9 ++++----- security/lsm_audit.c | 50 ++++++++++++++++++++++++++-------------------- security/selinux/avc.c | 2 +- security/selinux/hooks.c | 50 +++++++++++++++++++++++----------------------- security/smack/smack.h | 8 ++++---- security/smack/smack_lsm.c | 32 ++++++++++++++--------------- 6 files changed, 78 insertions(+), 73 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h index 112a5503335..bbaceab83a6 100644 --- a/include/linux/lsm_audit.h +++ b/include/linux/lsm_audit.h @@ -27,7 +27,7 @@ /* Auxiliary data to use in generating the audit record. */ struct common_audit_data { char type; -#define LSM_AUDIT_DATA_FS 1 +#define LSM_AUDIT_DATA_PATH 1 #define LSM_AUDIT_DATA_NET 2 #define LSM_AUDIT_DATA_CAP 3 #define LSM_AUDIT_DATA_IPC 4 @@ -35,12 +35,11 @@ struct common_audit_data { #define LSM_AUDIT_DATA_KEY 6 #define LSM_AUDIT_DATA_NONE 7 #define LSM_AUDIT_DATA_KMOD 8 +#define LSM_AUDIT_DATA_INODE 9 struct task_struct *tsk; union { - struct { - struct path path; - struct inode *inode; - } fs; + struct path path; + struct inode *inode; struct { int netif; struct sock *sk; diff --git a/security/lsm_audit.c b/security/lsm_audit.c index 908aa712816..2e846052cbf 100644 --- a/security/lsm_audit.c +++ b/security/lsm_audit.c @@ -210,7 +210,6 @@ static inline void print_ipv4_addr(struct audit_buffer *ab, __be32 addr, static void dump_common_audit_data(struct audit_buffer *ab, struct common_audit_data *a) { - struct inode *inode = NULL; struct task_struct *tsk = current; if (a->tsk) @@ -229,33 +228,40 @@ static void dump_common_audit_data(struct audit_buffer *ab, case LSM_AUDIT_DATA_CAP: audit_log_format(ab, " capability=%d ", a->u.cap); break; - case LSM_AUDIT_DATA_FS: - if (a->u.fs.path.dentry) { - struct dentry *dentry = a->u.fs.path.dentry; - if (a->u.fs.path.mnt) { - audit_log_d_path(ab, "path=", &a->u.fs.path); - } else { - audit_log_format(ab, " name="); - audit_log_untrustedstring(ab, - dentry->d_name.name); - } - inode = dentry->d_inode; - } else if (a->u.fs.inode) { - struct dentry *dentry; - inode = a->u.fs.inode; - dentry = d_find_alias(inode); - if (dentry) { - audit_log_format(ab, " name="); - audit_log_untrustedstring(ab, - dentry->d_name.name); - dput(dentry); - } + case LSM_AUDIT_DATA_PATH: { + struct dentry *dentry = a->u.path.dentry; + struct inode *inode; + + if (a->u.path.mnt) { + audit_log_d_path(ab, "path=", &a->u.path); + } else { + audit_log_format(ab, " name="); + audit_log_untrustedstring(ab, + dentry->d_name.name); } + inode = dentry->d_inode; if (inode) audit_log_format(ab, " dev=%s ino=%lu", inode->i_sb->s_id, inode->i_ino); break; + } + case LSM_AUDIT_DATA_INODE: { + struct dentry *dentry; + struct inode *inode; + + inode = a->u.inode; + dentry = d_find_alias(inode); + if (dentry) { + audit_log_format(ab, " name="); + audit_log_untrustedstring(ab, + dentry->d_name.name); + dput(dentry); + } + audit_log_format(ab, " dev=%s ino=%lu", inode->i_sb->s_id, + inode->i_ino); + break; + } case LSM_AUDIT_DATA_TASK: tsk = a->u.tsk; if (tsk && tsk->pid) { diff --git a/security/selinux/avc.c b/security/selinux/avc.c index 1d027e29ce8..ce742f1778e 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -531,7 +531,7 @@ int avc_audit(u32 ssid, u32 tsid, * during retry. However this is logically just as if the operation * happened a little later. */ - if ((a->type == LSM_AUDIT_DATA_FS) && + if ((a->type == LSM_AUDIT_DATA_INODE) && (flags & IPERM_FLAG_RCU)) return -ECHILD; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ed5f29aa0a3..ad664d3056e 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1488,8 +1488,8 @@ static int inode_has_perm(const struct cred *cred, if (!adp) { adp = &ad; - COMMON_AUDIT_DATA_INIT(&ad, FS); - ad.u.fs.inode = inode; + COMMON_AUDIT_DATA_INIT(&ad, INODE); + ad.u.inode = inode; } return avc_has_perm_flags(sid, isec->sid, isec->sclass, perms, adp, flags); @@ -1506,9 +1506,9 @@ static inline int dentry_has_perm(const struct cred *cred, struct inode *inode = dentry->d_inode; struct common_audit_data ad; - COMMON_AUDIT_DATA_INIT(&ad, FS); - ad.u.fs.path.mnt = mnt; - ad.u.fs.path.dentry = dentry; + COMMON_AUDIT_DATA_INIT(&ad, PATH); + ad.u.path.mnt = mnt; + ad.u.path.dentry = dentry; return inode_has_perm(cred, inode, av, &ad, 0); } @@ -1530,8 +1530,8 @@ static int file_has_perm(const struct cred *cred, u32 sid = cred_sid(cred); int rc; - COMMON_AUDIT_DATA_INIT(&ad, FS); - ad.u.fs.path = file->f_path; + COMMON_AUDIT_DATA_INIT(&ad, PATH); + ad.u.path = file->f_path; if (sid != fsec->sid) { rc = avc_has_perm(sid, fsec->sid, @@ -1569,8 +1569,8 @@ static int may_create(struct inode *dir, sid = tsec->sid; newsid = tsec->create_sid; - COMMON_AUDIT_DATA_INIT(&ad, FS); - ad.u.fs.path.dentry = dentry; + COMMON_AUDIT_DATA_INIT(&ad, PATH); + ad.u.path.dentry = dentry; rc = avc_has_perm(sid, dsec->sid, SECCLASS_DIR, DIR__ADD_NAME | DIR__SEARCH, @@ -1621,8 +1621,8 @@ static int may_link(struct inode *dir, dsec = dir->i_security; isec = dentry->d_inode->i_security; - COMMON_AUDIT_DATA_INIT(&ad, FS); - ad.u.fs.path.dentry = dentry; + COMMON_AUDIT_DATA_INIT(&ad, PATH); + ad.u.path.dentry = dentry; av = DIR__SEARCH; av |= (kind ? DIR__REMOVE_NAME : DIR__ADD_NAME); @@ -1667,9 +1667,9 @@ static inline int may_rename(struct inode *old_dir, old_is_dir = S_ISDIR(old_dentry->d_inode->i_mode); new_dsec = new_dir->i_security; - COMMON_AUDIT_DATA_INIT(&ad, FS); + COMMON_AUDIT_DATA_INIT(&ad, PATH); - ad.u.fs.path.dentry = old_dentry; + ad.u.path.dentry = old_dentry; rc = avc_has_perm(sid, old_dsec->sid, SECCLASS_DIR, DIR__REMOVE_NAME | DIR__SEARCH, &ad); if (rc) @@ -1685,7 +1685,7 @@ static inline int may_rename(struct inode *old_dir, return rc; } - ad.u.fs.path.dentry = new_dentry; + ad.u.path.dentry = new_dentry; av = DIR__ADD_NAME | DIR__SEARCH; if (new_dentry->d_inode) av |= DIR__REMOVE_NAME; @@ -1991,8 +1991,8 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm) return rc; } - COMMON_AUDIT_DATA_INIT(&ad, FS); - ad.u.fs.path = bprm->file->f_path; + COMMON_AUDIT_DATA_INIT(&ad, PATH); + ad.u.path = bprm->file->f_path; if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID) new_tsec->sid = old_tsec->sid; @@ -2120,7 +2120,7 @@ static inline void flush_unauthorized_files(const struct cred *cred, /* Revalidate access to inherited open files. */ - COMMON_AUDIT_DATA_INIT(&ad, FS); + COMMON_AUDIT_DATA_INIT(&ad, INODE); spin_lock(&files->file_lock); for (;;) { @@ -2468,8 +2468,8 @@ static int selinux_sb_kern_mount(struct super_block *sb, int flags, void *data) if (flags & MS_KERNMOUNT) return 0; - COMMON_AUDIT_DATA_INIT(&ad, FS); - ad.u.fs.path.dentry = sb->s_root; + COMMON_AUDIT_DATA_INIT(&ad, PATH); + ad.u.path.dentry = sb->s_root; return superblock_has_perm(cred, sb, FILESYSTEM__MOUNT, &ad); } @@ -2478,8 +2478,8 @@ static int selinux_sb_statfs(struct dentry *dentry) const struct cred *cred = current_cred(); struct common_audit_data ad; - COMMON_AUDIT_DATA_INIT(&ad, FS); - ad.u.fs.path.dentry = dentry->d_sb->s_root; + COMMON_AUDIT_DATA_INIT(&ad, PATH); + ad.u.path.dentry = dentry->d_sb->s_root; return superblock_has_perm(cred, dentry->d_sb, FILESYSTEM__GETATTR, &ad); } @@ -2653,8 +2653,8 @@ static int selinux_inode_permission(struct inode *inode, int mask, unsigned flag if (!mask) return 0; - COMMON_AUDIT_DATA_INIT(&ad, FS); - ad.u.fs.inode = inode; + COMMON_AUDIT_DATA_INIT(&ad, INODE); + ad.u.inode = inode; if (from_access) ad.selinux_audit_data.auditdeny |= FILE__AUDIT_ACCESS; @@ -2732,8 +2732,8 @@ static int selinux_inode_setxattr(struct dentry *dentry, const char *name, if (!is_owner_or_cap(inode)) return -EPERM; - COMMON_AUDIT_DATA_INIT(&ad, FS); - ad.u.fs.path.dentry = dentry; + COMMON_AUDIT_DATA_INIT(&ad, PATH); + ad.u.path.dentry = dentry; rc = avc_has_perm(sid, isec->sid, isec->sclass, FILE__RELABELFROM, &ad); diff --git a/security/smack/smack.h b/security/smack/smack.h index b449cfdad21..a16925c0e91 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -316,22 +316,22 @@ static inline void smk_ad_setfield_u_tsk(struct smk_audit_info *a, static inline void smk_ad_setfield_u_fs_path_dentry(struct smk_audit_info *a, struct dentry *d) { - a->a.u.fs.path.dentry = d; + a->a.u.path.dentry = d; } static inline void smk_ad_setfield_u_fs_path_mnt(struct smk_audit_info *a, struct vfsmount *m) { - a->a.u.fs.path.mnt = m; + a->a.u.path.mnt = m; } static inline void smk_ad_setfield_u_fs_inode(struct smk_audit_info *a, struct inode *i) { - a->a.u.fs.inode = i; + a->a.u.inode = i; } static inline void smk_ad_setfield_u_fs_path(struct smk_audit_info *a, struct path p) { - a->a.u.fs.path = p; + a->a.u.path = p; } static inline void smk_ad_setfield_u_net_sk(struct smk_audit_info *a, struct sock *sk) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 42fcb47747a..eeb393fbf92 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -383,7 +383,7 @@ static int smack_sb_statfs(struct dentry *dentry) int rc; struct smk_audit_info ad; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); rc = smk_curacc(sbp->smk_floor, MAY_READ, &ad); @@ -407,7 +407,7 @@ static int smack_sb_mount(char *dev_name, struct path *path, struct superblock_smack *sbp = path->mnt->mnt_sb->s_security; struct smk_audit_info ad; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path(&ad, *path); return smk_curacc(sbp->smk_floor, MAY_WRITE, &ad); @@ -426,7 +426,7 @@ static int smack_sb_umount(struct vfsmount *mnt, int flags) struct superblock_smack *sbp; struct smk_audit_info ad; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path_dentry(&ad, mnt->mnt_root); smk_ad_setfield_u_fs_path_mnt(&ad, mnt); @@ -563,7 +563,7 @@ static int smack_inode_link(struct dentry *old_dentry, struct inode *dir, struct smk_audit_info ad; int rc; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path_dentry(&ad, old_dentry); isp = smk_of_inode(old_dentry->d_inode); @@ -592,7 +592,7 @@ static int smack_inode_unlink(struct inode *dir, struct dentry *dentry) struct smk_audit_info ad; int rc; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); /* @@ -623,7 +623,7 @@ static int smack_inode_rmdir(struct inode *dir, struct dentry *dentry) struct smk_audit_info ad; int rc; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); /* @@ -663,7 +663,7 @@ static int smack_inode_rename(struct inode *old_inode, char *isp; struct smk_audit_info ad; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path_dentry(&ad, old_dentry); isp = smk_of_inode(old_dentry->d_inode); @@ -700,7 +700,7 @@ static int smack_inode_permission(struct inode *inode, int mask, unsigned flags) /* May be droppable after audit */ if (flags & IPERM_FLAG_RCU) return -ECHILD; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_INODE); smk_ad_setfield_u_fs_inode(&ad, inode); return smk_curacc(smk_of_inode(inode), mask, &ad); } @@ -720,7 +720,7 @@ static int smack_inode_setattr(struct dentry *dentry, struct iattr *iattr) */ if (iattr->ia_valid & ATTR_FORCE) return 0; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); return smk_curacc(smk_of_inode(dentry->d_inode), MAY_WRITE, &ad); @@ -737,7 +737,7 @@ static int smack_inode_getattr(struct vfsmount *mnt, struct dentry *dentry) { struct smk_audit_info ad; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); smk_ad_setfield_u_fs_path_mnt(&ad, mnt); return smk_curacc(smk_of_inode(dentry->d_inode), MAY_READ, &ad); @@ -784,7 +784,7 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name, } else rc = cap_inode_setxattr(dentry, name, value, size, flags); - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); if (rc == 0) @@ -845,7 +845,7 @@ static int smack_inode_getxattr(struct dentry *dentry, const char *name) { struct smk_audit_info ad; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); return smk_curacc(smk_of_inode(dentry->d_inode), MAY_READ, &ad); @@ -877,7 +877,7 @@ static int smack_inode_removexattr(struct dentry *dentry, const char *name) } else rc = cap_inode_removexattr(dentry, name); - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); if (rc == 0) rc = smk_curacc(smk_of_inode(dentry->d_inode), MAY_WRITE, &ad); @@ -1047,7 +1047,7 @@ static int smack_file_ioctl(struct file *file, unsigned int cmd, int rc = 0; struct smk_audit_info ad; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path(&ad, file->f_path); if (_IOC_DIR(cmd) & _IOC_WRITE) @@ -1070,7 +1070,7 @@ static int smack_file_lock(struct file *file, unsigned int cmd) { struct smk_audit_info ad; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path_dentry(&ad, file->f_path.dentry); return smk_curacc(file->f_security, MAY_WRITE, &ad); } @@ -1089,7 +1089,7 @@ static int smack_file_fcntl(struct file *file, unsigned int cmd, struct smk_audit_info ad; int rc; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_FS); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path(&ad, file->f_path); switch (cmd) { -- cgit v1.2.3-70-g09d2 From a269434d2fb48a4d66c1d7bf821b7874b59c5b41 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Mon, 25 Apr 2011 13:10:27 -0400 Subject: LSM: separate LSM_AUDIT_DATA_DENTRY from LSM_AUDIT_DATA_PATH This patch separates and audit message that only contains a dentry from one that contains a full path. This allows us to make it harder to misuse the interfaces or for the interfaces to be implemented wrong. Signed-off-by: Eric Paris Acked-by: Casey Schaufler --- include/linux/lsm_audit.h | 2 ++ security/lsm_audit.c | 25 ++++++++++++++++--------- security/selinux/hooks.c | 26 +++++++++++++------------- security/smack/smack.h | 7 +------ security/smack/smack_lsm.c | 34 ++++++++++++++++++++-------------- 5 files changed, 52 insertions(+), 42 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h index bbaceab83a6..88e78dedc2e 100644 --- a/include/linux/lsm_audit.h +++ b/include/linux/lsm_audit.h @@ -36,9 +36,11 @@ struct common_audit_data { #define LSM_AUDIT_DATA_NONE 7 #define LSM_AUDIT_DATA_KMOD 8 #define LSM_AUDIT_DATA_INODE 9 +#define LSM_AUDIT_DATA_DENTRY 10 struct task_struct *tsk; union { struct path path; + struct dentry *dentry; struct inode *inode; struct { int netif; diff --git a/security/lsm_audit.c b/security/lsm_audit.c index 2e846052cbf..893af8a2fa1 100644 --- a/security/lsm_audit.c +++ b/security/lsm_audit.c @@ -229,17 +229,24 @@ static void dump_common_audit_data(struct audit_buffer *ab, audit_log_format(ab, " capability=%d ", a->u.cap); break; case LSM_AUDIT_DATA_PATH: { - struct dentry *dentry = a->u.path.dentry; struct inode *inode; - if (a->u.path.mnt) { - audit_log_d_path(ab, "path=", &a->u.path); - } else { - audit_log_format(ab, " name="); - audit_log_untrustedstring(ab, - dentry->d_name.name); - } - inode = dentry->d_inode; + audit_log_d_path(ab, "path=", &a->u.path); + + inode = a->u.path.dentry->d_inode; + if (inode) + audit_log_format(ab, " dev=%s ino=%lu", + inode->i_sb->s_id, + inode->i_ino); + break; + } + case LSM_AUDIT_DATA_DENTRY: { + struct inode *inode; + + audit_log_format(ab, " name="); + audit_log_untrustedstring(ab, a->u.dentry->d_name.name); + + inode = a->u.dentry->d_inode; if (inode) audit_log_format(ab, " dev=%s ino=%lu", inode->i_sb->s_id, diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ad664d3056e..9e8078a42a9 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1569,8 +1569,8 @@ static int may_create(struct inode *dir, sid = tsec->sid; newsid = tsec->create_sid; - COMMON_AUDIT_DATA_INIT(&ad, PATH); - ad.u.path.dentry = dentry; + COMMON_AUDIT_DATA_INIT(&ad, DENTRY); + ad.u.dentry = dentry; rc = avc_has_perm(sid, dsec->sid, SECCLASS_DIR, DIR__ADD_NAME | DIR__SEARCH, @@ -1621,8 +1621,8 @@ static int may_link(struct inode *dir, dsec = dir->i_security; isec = dentry->d_inode->i_security; - COMMON_AUDIT_DATA_INIT(&ad, PATH); - ad.u.path.dentry = dentry; + COMMON_AUDIT_DATA_INIT(&ad, DENTRY); + ad.u.dentry = dentry; av = DIR__SEARCH; av |= (kind ? DIR__REMOVE_NAME : DIR__ADD_NAME); @@ -1667,9 +1667,9 @@ static inline int may_rename(struct inode *old_dir, old_is_dir = S_ISDIR(old_dentry->d_inode->i_mode); new_dsec = new_dir->i_security; - COMMON_AUDIT_DATA_INIT(&ad, PATH); + COMMON_AUDIT_DATA_INIT(&ad, DENTRY); - ad.u.path.dentry = old_dentry; + ad.u.dentry = old_dentry; rc = avc_has_perm(sid, old_dsec->sid, SECCLASS_DIR, DIR__REMOVE_NAME | DIR__SEARCH, &ad); if (rc) @@ -1685,7 +1685,7 @@ static inline int may_rename(struct inode *old_dir, return rc; } - ad.u.path.dentry = new_dentry; + ad.u.dentry = new_dentry; av = DIR__ADD_NAME | DIR__SEARCH; if (new_dentry->d_inode) av |= DIR__REMOVE_NAME; @@ -2468,8 +2468,8 @@ static int selinux_sb_kern_mount(struct super_block *sb, int flags, void *data) if (flags & MS_KERNMOUNT) return 0; - COMMON_AUDIT_DATA_INIT(&ad, PATH); - ad.u.path.dentry = sb->s_root; + COMMON_AUDIT_DATA_INIT(&ad, DENTRY); + ad.u.dentry = sb->s_root; return superblock_has_perm(cred, sb, FILESYSTEM__MOUNT, &ad); } @@ -2478,8 +2478,8 @@ static int selinux_sb_statfs(struct dentry *dentry) const struct cred *cred = current_cred(); struct common_audit_data ad; - COMMON_AUDIT_DATA_INIT(&ad, PATH); - ad.u.path.dentry = dentry->d_sb->s_root; + COMMON_AUDIT_DATA_INIT(&ad, DENTRY); + ad.u.dentry = dentry->d_sb->s_root; return superblock_has_perm(cred, dentry->d_sb, FILESYSTEM__GETATTR, &ad); } @@ -2732,8 +2732,8 @@ static int selinux_inode_setxattr(struct dentry *dentry, const char *name, if (!is_owner_or_cap(inode)) return -EPERM; - COMMON_AUDIT_DATA_INIT(&ad, PATH); - ad.u.path.dentry = dentry; + COMMON_AUDIT_DATA_INIT(&ad, DENTRY); + ad.u.dentry = dentry; rc = avc_has_perm(sid, isec->sid, isec->sclass, FILE__RELABELFROM, &ad); diff --git a/security/smack/smack.h b/security/smack/smack.h index a16925c0e91..2b6c6a51612 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -316,12 +316,7 @@ static inline void smk_ad_setfield_u_tsk(struct smk_audit_info *a, static inline void smk_ad_setfield_u_fs_path_dentry(struct smk_audit_info *a, struct dentry *d) { - a->a.u.path.dentry = d; -} -static inline void smk_ad_setfield_u_fs_path_mnt(struct smk_audit_info *a, - struct vfsmount *m) -{ - a->a.u.path.mnt = m; + a->a.u.dentry = d; } static inline void smk_ad_setfield_u_fs_inode(struct smk_audit_info *a, struct inode *i) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index eeb393fbf92..a3bdd138392 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -383,7 +383,7 @@ static int smack_sb_statfs(struct dentry *dentry) int rc; struct smk_audit_info ad; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); rc = smk_curacc(sbp->smk_floor, MAY_READ, &ad); @@ -425,10 +425,13 @@ static int smack_sb_umount(struct vfsmount *mnt, int flags) { struct superblock_smack *sbp; struct smk_audit_info ad; + struct path path; + + path.dentry = mnt->mnt_root; + path.mnt = mnt; smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); - smk_ad_setfield_u_fs_path_dentry(&ad, mnt->mnt_root); - smk_ad_setfield_u_fs_path_mnt(&ad, mnt); + smk_ad_setfield_u_fs_path(&ad, path); sbp = mnt->mnt_sb->s_security; return smk_curacc(sbp->smk_floor, MAY_WRITE, &ad); @@ -563,7 +566,7 @@ static int smack_inode_link(struct dentry *old_dentry, struct inode *dir, struct smk_audit_info ad; int rc; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY); smk_ad_setfield_u_fs_path_dentry(&ad, old_dentry); isp = smk_of_inode(old_dentry->d_inode); @@ -592,7 +595,7 @@ static int smack_inode_unlink(struct inode *dir, struct dentry *dentry) struct smk_audit_info ad; int rc; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); /* @@ -623,7 +626,7 @@ static int smack_inode_rmdir(struct inode *dir, struct dentry *dentry) struct smk_audit_info ad; int rc; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); /* @@ -663,7 +666,7 @@ static int smack_inode_rename(struct inode *old_inode, char *isp; struct smk_audit_info ad; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY); smk_ad_setfield_u_fs_path_dentry(&ad, old_dentry); isp = smk_of_inode(old_dentry->d_inode); @@ -720,7 +723,7 @@ static int smack_inode_setattr(struct dentry *dentry, struct iattr *iattr) */ if (iattr->ia_valid & ATTR_FORCE) return 0; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); return smk_curacc(smk_of_inode(dentry->d_inode), MAY_WRITE, &ad); @@ -736,10 +739,13 @@ static int smack_inode_setattr(struct dentry *dentry, struct iattr *iattr) static int smack_inode_getattr(struct vfsmount *mnt, struct dentry *dentry) { struct smk_audit_info ad; + struct path path; + + path.dentry = dentry; + path.mnt = mnt; smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); - smk_ad_setfield_u_fs_path_dentry(&ad, dentry); - smk_ad_setfield_u_fs_path_mnt(&ad, mnt); + smk_ad_setfield_u_fs_path(&ad, path); return smk_curacc(smk_of_inode(dentry->d_inode), MAY_READ, &ad); } @@ -784,7 +790,7 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name, } else rc = cap_inode_setxattr(dentry, name, value, size, flags); - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); if (rc == 0) @@ -845,7 +851,7 @@ static int smack_inode_getxattr(struct dentry *dentry, const char *name) { struct smk_audit_info ad; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); return smk_curacc(smk_of_inode(dentry->d_inode), MAY_READ, &ad); @@ -877,7 +883,7 @@ static int smack_inode_removexattr(struct dentry *dentry, const char *name) } else rc = cap_inode_removexattr(dentry, name); - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY); smk_ad_setfield_u_fs_path_dentry(&ad, dentry); if (rc == 0) rc = smk_curacc(smk_of_inode(dentry->d_inode), MAY_WRITE, &ad); @@ -1070,7 +1076,7 @@ static int smack_file_lock(struct file *file, unsigned int cmd) { struct smk_audit_info ad; - smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); + smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY); smk_ad_setfield_u_fs_path_dentry(&ad, file->f_path.dentry); return smk_curacc(file->f_security, MAY_WRITE, &ad); } -- cgit v1.2.3-70-g09d2 From 9ade0cf440a1e5800dc68eef2e77b8d9d83a6dff Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Mon, 25 Apr 2011 16:26:29 -0400 Subject: SELINUX: Make selinux cache VFS RCU walks safe Now that the security modules can decide whether they support the dcache RCU walk or not it's possible to make selinux a bit more RCU friendly. The SELinux AVC and security server access decision code is RCU safe. A specific piece of the LSM audit code may not be RCU safe. This patch makes the VFS RCU walk retry if it would hit the non RCU safe chunk of code. It will normally just work under RCU. This is done simply by passing the VFS RCU state as a flag down into the avc_audit() code and returning ECHILD there if it would have an issue. Based-on-patch-by: Andi Kleen Signed-off-by: Eric Paris Signed-off-by: Linus Torvalds --- security/selinux/avc.c | 36 +++++++++++++++++++++++++++++------- security/selinux/hooks.c | 26 +++++++++++++------------- security/selinux/include/avc.h | 18 +++++++++++++----- 3 files changed, 55 insertions(+), 25 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/avc.c b/security/selinux/avc.c index 9da6420e205..1d027e29ce8 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -471,6 +471,7 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a) * @avd: access vector decisions * @result: result from avc_has_perm_noaudit * @a: auxiliary audit data + * @flags: VFS walk flags * * Audit the granting or denial of permissions in accordance * with the policy. This function is typically called by @@ -481,9 +482,10 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a) * be performed under a lock, to allow the lock to be released * before calling the auditing code. */ -void avc_audit(u32 ssid, u32 tsid, +int avc_audit(u32 ssid, u32 tsid, u16 tclass, u32 requested, - struct av_decision *avd, int result, struct common_audit_data *a) + struct av_decision *avd, int result, struct common_audit_data *a, + unsigned flags) { struct common_audit_data stack_data; u32 denied, audited; @@ -515,11 +517,24 @@ void avc_audit(u32 ssid, u32 tsid, else audited = requested & avd->auditallow; if (!audited) - return; + return 0; + if (!a) { a = &stack_data; COMMON_AUDIT_DATA_INIT(a, NONE); } + + /* + * When in a RCU walk do the audit on the RCU retry. This is because + * the collection of the dname in an inode audit message is not RCU + * safe. Note this may drop some audits when the situation changes + * during retry. However this is logically just as if the operation + * happened a little later. + */ + if ((a->type == LSM_AUDIT_DATA_FS) && + (flags & IPERM_FLAG_RCU)) + return -ECHILD; + a->selinux_audit_data.tclass = tclass; a->selinux_audit_data.requested = requested; a->selinux_audit_data.ssid = ssid; @@ -529,6 +544,7 @@ void avc_audit(u32 ssid, u32 tsid, a->lsm_pre_audit = avc_audit_pre_callback; a->lsm_post_audit = avc_audit_post_callback; common_lsm_audit(a); + return 0; } /** @@ -793,6 +809,7 @@ int avc_has_perm_noaudit(u32 ssid, u32 tsid, * @tclass: target security class * @requested: requested permissions, interpreted based on @tclass * @auditdata: auxiliary audit data + * @flags: VFS walk flags * * Check the AVC to determine whether the @requested permissions are granted * for the SID pair (@ssid, @tsid), interpreting the permissions @@ -802,14 +819,19 @@ int avc_has_perm_noaudit(u32 ssid, u32 tsid, * permissions are granted, -%EACCES if any permissions are denied, or * another -errno upon other errors. */ -int avc_has_perm(u32 ssid, u32 tsid, u16 tclass, - u32 requested, struct common_audit_data *auditdata) +int avc_has_perm_flags(u32 ssid, u32 tsid, u16 tclass, + u32 requested, struct common_audit_data *auditdata, + unsigned flags) { struct av_decision avd; - int rc; + int rc, rc2; rc = avc_has_perm_noaudit(ssid, tsid, tclass, requested, 0, &avd); - avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata); + + rc2 = avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata, + flags); + if (rc2) + return rc2; return rc; } diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a73f4e46377..f7cf0ea6fae 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1446,8 +1446,11 @@ static int task_has_capability(struct task_struct *tsk, } rc = avc_has_perm_noaudit(sid, sid, sclass, av, 0, &avd); - if (audit == SECURITY_CAP_AUDIT) - avc_audit(sid, sid, sclass, av, &avd, rc, &ad); + if (audit == SECURITY_CAP_AUDIT) { + int rc2 = avc_audit(sid, sid, sclass, av, &avd, rc, &ad, 0); + if (rc2) + return rc2; + } return rc; } @@ -1467,7 +1470,8 @@ static int task_has_system(struct task_struct *tsk, static int inode_has_perm(const struct cred *cred, struct inode *inode, u32 perms, - struct common_audit_data *adp) + struct common_audit_data *adp, + unsigned flags) { struct inode_security_struct *isec; struct common_audit_data ad; @@ -1487,7 +1491,7 @@ static int inode_has_perm(const struct cred *cred, ad.u.fs.inode = inode; } - return avc_has_perm(sid, isec->sid, isec->sclass, perms, adp); + return avc_has_perm_flags(sid, isec->sid, isec->sclass, perms, adp, flags); } /* Same as inode_has_perm, but pass explicit audit data containing @@ -1504,7 +1508,7 @@ static inline int dentry_has_perm(const struct cred *cred, COMMON_AUDIT_DATA_INIT(&ad, FS); ad.u.fs.path.mnt = mnt; ad.u.fs.path.dentry = dentry; - return inode_has_perm(cred, inode, av, &ad); + return inode_has_perm(cred, inode, av, &ad, 0); } /* Check whether a task can use an open file descriptor to @@ -1540,7 +1544,7 @@ static int file_has_perm(const struct cred *cred, /* av is zero if only checking access to the descriptor. */ rc = 0; if (av) - rc = inode_has_perm(cred, inode, av, &ad); + rc = inode_has_perm(cred, inode, av, &ad, 0); out: return rc; @@ -2103,7 +2107,7 @@ static inline void flush_unauthorized_files(const struct cred *cred, file = file_priv->file; inode = file->f_path.dentry->d_inode; if (inode_has_perm(cred, inode, - FILE__READ | FILE__WRITE, NULL)) { + FILE__READ | FILE__WRITE, NULL, 0)) { drop_tty = 1; } } @@ -2649,10 +2653,6 @@ static int selinux_inode_permission(struct inode *inode, int mask, unsigned flag if (!mask) return 0; - /* May be droppable after audit */ - if (flags & IPERM_FLAG_RCU) - return -ECHILD; - COMMON_AUDIT_DATA_INIT(&ad, FS); ad.u.fs.inode = inode; @@ -2661,7 +2661,7 @@ static int selinux_inode_permission(struct inode *inode, int mask, unsigned flag perms = file_mask_to_av(inode->i_mode, mask); - return inode_has_perm(cred, inode, perms, &ad); + return inode_has_perm(cred, inode, perms, &ad, flags); } static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr) @@ -3209,7 +3209,7 @@ static int selinux_dentry_open(struct file *file, const struct cred *cred) * new inode label or new policy. * This check is not redundant - do not remove. */ - return inode_has_perm(cred, inode, open_file_to_av(file), NULL); + return inode_has_perm(cred, inode, open_file_to_av(file), NULL, 0); } /* task security operations */ diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h index 5615081b73e..e77b2ac2908 100644 --- a/security/selinux/include/avc.h +++ b/security/selinux/include/avc.h @@ -54,11 +54,11 @@ struct avc_cache_stats { void __init avc_init(void); -void avc_audit(u32 ssid, u32 tsid, +int avc_audit(u32 ssid, u32 tsid, u16 tclass, u32 requested, struct av_decision *avd, int result, - struct common_audit_data *a); + struct common_audit_data *a, unsigned flags); #define AVC_STRICT 1 /* Ignore permissive mode. */ int avc_has_perm_noaudit(u32 ssid, u32 tsid, @@ -66,9 +66,17 @@ int avc_has_perm_noaudit(u32 ssid, u32 tsid, unsigned flags, struct av_decision *avd); -int avc_has_perm(u32 ssid, u32 tsid, - u16 tclass, u32 requested, - struct common_audit_data *auditdata); +int avc_has_perm_flags(u32 ssid, u32 tsid, + u16 tclass, u32 requested, + struct common_audit_data *auditdata, + unsigned); + +static inline int avc_has_perm(u32 ssid, u32 tsid, + u16 tclass, u32 requested, + struct common_audit_data *auditdata) +{ + return avc_has_perm_flags(ssid, tsid, tclass, requested, auditdata, 0); +} u32 avc_policy_seqno(void); -- cgit v1.2.3-70-g09d2 From 562abf624175e3f8487b7f064e516805e437e597 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Thu, 28 Apr 2011 15:11:21 -0400 Subject: SELinux: pass last path component in may_create New inodes are created in a two stage process. We first will compute the label on a new inode in security_inode_create() and check if the operation is allowed. We will then actually re-compute that same label and apply it in security_inode_init_security(). The change to do new label calculations based in part on the last component of the path name only passed the path component information all the way down the security_inode_init_security hook. Down the security_inode_create hook the path information did not make it past may_create. Thus the two calculations came up differently and the permissions check might not actually be against the label that is created. Pass and use the same information in both places to harmonize the calculations and checks. Reported-by: Dominick Grift Signed-off-by: Eric Paris --- security/selinux/hooks.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 9e8078a42a9..a6dd2bed8d7 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1579,7 +1579,8 @@ static int may_create(struct inode *dir, return rc; if (!newsid || !(sbsec->flags & SE_SBLABELSUPP)) { - rc = security_transition_sid(sid, dsec->sid, tclass, NULL, &newsid); + rc = security_transition_sid(sid, dsec->sid, tclass, + &dentry->d_name, &newsid); if (rc) return rc; } -- cgit v1.2.3-70-g09d2 From 2875fa00830be62431f5ac22d8f85d57f9fa3033 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Thu, 28 Apr 2011 16:04:24 -0400 Subject: SELinux: introduce path_has_perm We currently have inode_has_perm and dentry_has_perm. dentry_has_perm just calls inode_has_perm with additional audit data. But dentry_has_perm can take either a dentry or a path. Split those to make the code obvious and to fix the previous problem where I thought dentry_has_perm always had a valid dentry and mnt. Signed-off-by: Eric Paris --- security/selinux/hooks.c | 44 ++++++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 14 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a6dd2bed8d7..9f426b8a12b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1499,16 +1499,29 @@ static int inode_has_perm(const struct cred *cred, the dentry to help the auditing code to more easily generate the pathname if needed. */ static inline int dentry_has_perm(const struct cred *cred, - struct vfsmount *mnt, struct dentry *dentry, u32 av) { struct inode *inode = dentry->d_inode; struct common_audit_data ad; + COMMON_AUDIT_DATA_INIT(&ad, DENTRY); + ad.u.dentry = dentry; + return inode_has_perm(cred, inode, av, &ad, 0); +} + +/* Same as inode_has_perm, but pass explicit audit data containing + the path to help the auditing code to more easily generate the + pathname if needed. */ +static inline int path_has_perm(const struct cred *cred, + struct path *path, + u32 av) +{ + struct inode *inode = path->dentry->d_inode; + struct common_audit_data ad; + COMMON_AUDIT_DATA_INIT(&ad, PATH); - ad.u.path.mnt = mnt; - ad.u.path.dentry = dentry; + ad.u.path = *path; return inode_has_perm(cred, inode, av, &ad, 0); } @@ -1896,7 +1909,7 @@ static int selinux_quota_on(struct dentry *dentry) { const struct cred *cred = current_cred(); - return dentry_has_perm(cred, NULL, dentry, FILE__QUOTAON); + return dentry_has_perm(cred, dentry, FILE__QUOTAON); } static int selinux_syslog(int type) @@ -2496,8 +2509,7 @@ static int selinux_mount(char *dev_name, return superblock_has_perm(cred, path->mnt->mnt_sb, FILESYSTEM__REMOUNT, NULL); else - return dentry_has_perm(cred, path->mnt, path->dentry, - FILE__MOUNTON); + return path_has_perm(cred, path, FILE__MOUNTON); } static int selinux_umount(struct vfsmount *mnt, int flags) @@ -2630,14 +2642,14 @@ static int selinux_inode_readlink(struct dentry *dentry) { const struct cred *cred = current_cred(); - return dentry_has_perm(cred, NULL, dentry, FILE__READ); + return dentry_has_perm(cred, dentry, FILE__READ); } static int selinux_inode_follow_link(struct dentry *dentry, struct nameidata *nameidata) { const struct cred *cred = current_cred(); - return dentry_has_perm(cred, NULL, dentry, FILE__READ); + return dentry_has_perm(cred, dentry, FILE__READ); } static int selinux_inode_permission(struct inode *inode, int mask, unsigned flags) @@ -2680,16 +2692,20 @@ static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr) if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID | ATTR_ATIME_SET | ATTR_MTIME_SET | ATTR_TIMES_SET)) - return dentry_has_perm(cred, NULL, dentry, FILE__SETATTR); + return dentry_has_perm(cred, dentry, FILE__SETATTR); - return dentry_has_perm(cred, NULL, dentry, FILE__WRITE); + return dentry_has_perm(cred, dentry, FILE__WRITE); } static int selinux_inode_getattr(struct vfsmount *mnt, struct dentry *dentry) { const struct cred *cred = current_cred(); + struct path path; + + path.dentry = dentry; + path.mnt = mnt; - return dentry_has_perm(cred, mnt, dentry, FILE__GETATTR); + return path_has_perm(cred, &path, FILE__GETATTR); } static int selinux_inode_setotherxattr(struct dentry *dentry, const char *name) @@ -2710,7 +2726,7 @@ static int selinux_inode_setotherxattr(struct dentry *dentry, const char *name) /* Not an attribute we recognize, so just check the ordinary setattr permission. */ - return dentry_has_perm(cred, NULL, dentry, FILE__SETATTR); + return dentry_has_perm(cred, dentry, FILE__SETATTR); } static int selinux_inode_setxattr(struct dentry *dentry, const char *name, @@ -2797,14 +2813,14 @@ static int selinux_inode_getxattr(struct dentry *dentry, const char *name) { const struct cred *cred = current_cred(); - return dentry_has_perm(cred, NULL, dentry, FILE__GETATTR); + return dentry_has_perm(cred, dentry, FILE__GETATTR); } static int selinux_inode_listxattr(struct dentry *dentry) { const struct cred *cred = current_cred(); - return dentry_has_perm(cred, NULL, dentry, FILE__GETATTR); + return dentry_has_perm(cred, dentry, FILE__GETATTR); } static int selinux_inode_removexattr(struct dentry *dentry, const char *name) -- cgit v1.2.3-70-g09d2 From cb1e922fa104bb0bb3aa5fc6ca7f7e070f3b55e9 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Thu, 28 Apr 2011 15:11:21 -0400 Subject: SELinux: pass last path component in may_create New inodes are created in a two stage process. We first will compute the label on a new inode in security_inode_create() and check if the operation is allowed. We will then actually re-compute that same label and apply it in security_inode_init_security(). The change to do new label calculations based in part on the last component of the path name only passed the path component information all the way down the security_inode_init_security hook. Down the security_inode_create hook the path information did not make it past may_create. Thus the two calculations came up differently and the permissions check might not actually be against the label that is created. Pass and use the same information in both places to harmonize the calculations and checks. Reported-by: Dominick Grift Signed-off-by: Eric Paris --- security/selinux/hooks.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index d52a9250741..9a93af81a0c 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1573,7 +1573,8 @@ static int may_create(struct inode *dir, return rc; if (!newsid || !(sbsec->flags & SE_SBLABELSUPP)) { - rc = security_transition_sid(sid, dsec->sid, tclass, NULL, &newsid); + rc = security_transition_sid(sid, dsec->sid, tclass, + &dentry->d_name, &newsid); if (rc) return rc; } -- cgit v1.2.3-70-g09d2 From 95f4efb2d78661065aaf0be57f5bf00e4d2aea1d Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Wed, 8 Jun 2011 15:11:56 -0700 Subject: selinux: simplify and clean up inode_has_perm() This is a rather hot function that is called with a potentially NULL "struct common_audit_data" pointer argument. And in that case it has to provide and initialize its own dummy common_audit_data structure. However, all the _common_ cases already pass it a real audit-data structure, so that uncommon NULL case not only creates a silly run-time test, more importantly it causes that function to have a big stack frame for the dummy variable that isn't even used in the common case! So get rid of that stupid run-time behavior, and make the (few) functions that currently call with a NULL pointer just call a new helper function instead (naturally called inode_has_perm_noapd(), since it has no adp argument). This makes the run-time test be a static code generation issue instead, and allows for a much denser stack since none of the common callers need the dummy structure. And a denser stack not only means less stack space usage, it means better cache behavior. So we have a win-win-win from this simplification: less code executed, smaller stack footprint, and better cache behavior. Signed-off-by: Linus Torvalds --- security/selinux/hooks.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a0d38459d65..20219ef5439 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1476,7 +1476,6 @@ static int inode_has_perm(const struct cred *cred, unsigned flags) { struct inode_security_struct *isec; - struct common_audit_data ad; u32 sid; validate_creds(cred); @@ -1487,15 +1486,21 @@ static int inode_has_perm(const struct cred *cred, sid = cred_sid(cred); isec = inode->i_security; - if (!adp) { - adp = &ad; - COMMON_AUDIT_DATA_INIT(&ad, INODE); - ad.u.inode = inode; - } - return avc_has_perm_flags(sid, isec->sid, isec->sclass, perms, adp, flags); } +static int inode_has_perm_noadp(const struct cred *cred, + struct inode *inode, + u32 perms, + unsigned flags) +{ + struct common_audit_data ad; + + COMMON_AUDIT_DATA_INIT(&ad, INODE); + ad.u.inode = inode; + return inode_has_perm(cred, inode, perms, &ad, flags); +} + /* Same as inode_has_perm, but pass explicit audit data containing the dentry to help the auditing code to more easily generate the pathname if needed. */ @@ -2122,8 +2127,8 @@ static inline void flush_unauthorized_files(const struct cred *cred, struct tty_file_private, list); file = file_priv->file; inode = file->f_path.dentry->d_inode; - if (inode_has_perm(cred, inode, - FILE__READ | FILE__WRITE, NULL, 0)) { + if (inode_has_perm_noadp(cred, inode, + FILE__READ | FILE__WRITE, 0)) { drop_tty = 1; } } @@ -3228,7 +3233,7 @@ static int selinux_dentry_open(struct file *file, const struct cred *cred) * new inode label or new policy. * This check is not redundant - do not remove. */ - return inode_has_perm(cred, inode, open_file_to_av(file), NULL, 0); + return inode_has_perm_noadp(cred, inode, open_file_to_av(file), 0); } /* task security operations */ -- cgit v1.2.3-70-g09d2