See LKML posting http://marc.info/?l=linux-kernel&m=120534660832189&w=2 according to viro a call to sys_open like: open("/dev/null", 3) is supposed to mean: check read and write but not set FMODE_READ or FMODE_WRITE. This is exactly the security check we get through the security_inode_permission hook but not what happens through the security_dentry_open hook. This patch actually passes the bare flags into security_dentry_open so that we can do the correct permission checking there. What do people think? Untested, uncompiled, just thoughts and fleshing it out.... -Eric fs/open.c | 2 +- include/linux/security.h | 6 +++--- security/security.c | 4 ++-- security/selinux/hooks.c | 20 ++++++++++++++++++-- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/fs/open.c b/fs/open.c index 5419853..2560a3e 100644 --- a/fs/open.c +++ b/fs/open.c @@ -754,7 +754,7 @@ static struct file *__dentry_open(struct dentry *dentry, struct vfsmount *mnt, f->f_op = fops_get(inode->i_fop); file_move(f, &inode->i_sb->s_files); - error = security_dentry_open(f); + error = security_dentry_open(f, flags); if (error) goto cleanup_all; diff --git a/include/linux/security.h b/include/linux/security.h index b07357c..704a5f7 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1338,7 +1338,7 @@ struct security_operations { int (*file_send_sigiotask) (struct task_struct * tsk, struct fown_struct * fown, int sig); int (*file_receive) (struct file * file); - int (*dentry_open) (struct file *file); + int (*dentry_open) (struct file *file, int flags); int (*task_create) (unsigned long clone_flags); int (*task_alloc_security) (struct task_struct * p); @@ -1594,7 +1594,7 @@ int security_file_set_fowner(struct file *file); int security_file_send_sigiotask(struct task_struct *tsk, struct fown_struct *fown, int sig); int security_file_receive(struct file *file); -int security_dentry_open(struct file *file); +int security_dentry_open(struct file *file, int flags); int security_task_create(unsigned long clone_flags); int security_task_alloc(struct task_struct *p); void security_task_free(struct task_struct *p); @@ -2086,7 +2086,7 @@ static inline int security_file_receive (struct file *file) return 0; } -static inline int security_dentry_open (struct file *file) +static inline int security_dentry_open (struct file *file, int flags) { return 0; } diff --git a/security/security.c b/security/security.c index b1387a6..28a4f2a 100644 --- a/security/security.c +++ b/security/security.c @@ -582,9 +582,9 @@ int security_file_receive(struct file *file) return security_ops->file_receive(file); } -int security_dentry_open(struct file *file) +int security_dentry_open(struct file *file, int flags) { - return security_ops->dentry_open(file); + return security_ops->dentry_open(file, flags); } int security_task_create(unsigned long clone_flags) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 4bf4807..1966590 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3017,11 +3017,14 @@ static int selinux_file_receive(struct file *file) return file_has_perm(current, file, file_to_av(file)); } -static int selinux_dentry_open(struct file *file) +static int selinux_dentry_open(struct file *file, int flags) { struct file_security_struct *fsec; struct inode *inode; struct inode_security_struct *isec; + u32 av = 0; + mode_t f_mode; + inode = file->f_path.dentry->d_inode; fsec = file->f_security; isec = inode->i_security; @@ -3034,6 +3037,19 @@ static int selinux_dentry_open(struct file *file) */ fsec->isid = isec->sid; fsec->pseqno = avc_policy_seqno(); + + f_mode = flags & O_ACCMODE; + if ((f_mode+1) & O_ACCMODE) + f_mode++; + + if (f_mode & FMODE_READ) + av |= FILE__READ; + if (f_mode & FMODE_WRITE) { + if (file->f_flags & O_APPEND) + av |= FILE__APPEND; + else + av |= FILE__WRITE; + } /* * Since the inode label or policy seqno may have changed * between the selinux_inode_permission check and the saving @@ -3042,7 +3058,7 @@ static int selinux_dentry_open(struct file *file) * new inode label or new policy. * This check is not redundant - do not remove. */ - return inode_has_perm(current, inode, file_to_av(file), NULL); + return inode_has_perm(current, inode, av, NULL); } /* task security operations */ -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with the words "unsubscribe selinux" without quotes as the message.
