How to handle security_dentry_open when the open flags are 'special'

[Eric Paris <eparis@redhat.com>]

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@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.