xfs
[Top] [All Lists]

[PATCH 4/5] fs: Call security_ops->inode_killpriv on truncate

To: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Subject: [PATCH 4/5] fs: Call security_ops->inode_killpriv on truncate
From: Jan Kara <jack@xxxxxxx>
Date: Tue, 3 Mar 2015 11:38:38 +0100
Cc: linux-fsdevel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, Jan Kara <jack@xxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1425379119-3773-1-git-send-email-jack@xxxxxxx>
References: <1425379119-3773-1-git-send-email-jack@xxxxxxx>
Comment in include/linux/security.h says that ->inode_killpriv() should
be called when setuid bit is being removed and that similar security
labels should be removed at this time as well. However we don't call
->inode_killpriv() when we remove suid bit on truncate.

We fix the problem by calling ->inode_need_killpriv() and subsequently
->inode_killpriv() on truncate the same way as we do it on file write.

After this patch there's only one user of should_remove_suid() - ocfs2 -
and indeed it's buggy because it doesn't clear security attributes on
write. However fixing it is difficult because of special locking
constraints.

Signed-off-by: Jan Kara <jack@xxxxxxx>
---
 fs/inode.c         | 5 ++---
 fs/open.c          | 6 ++++--
 include/linux/fs.h | 6 +++++-
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 3407bedc1c7b..7f4eabd72bd6 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1665,9 +1665,8 @@ EXPORT_SYMBOL(should_remove_suid);
  * response to write or truncate. Return 0 if nothing has to be changed.
  * Negative value on error (change should be denied).
  */
-int file_needs_remove_privs(struct file *file)
+int dentry_needs_remove_privs(struct dentry *dentry)
 {
-       struct dentry *dentry = file->f_path.dentry;
        struct inode *inode = dentry->d_inode;
        int mask = 0;
        int ret;
@@ -1683,7 +1682,7 @@ int file_needs_remove_privs(struct file *file)
                mask |= ATTR_KILL_PRIV;
        return mask;
 }
-EXPORT_SYMBOL(file_needs_remove_privs);
+EXPORT_SYMBOL(dentry_needs_remove_privs);
 
 static int __remove_privs(struct dentry *dentry, int kill)
 {
diff --git a/fs/open.c b/fs/open.c
index 33f9cbf2610b..14e2aeea4cee 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -51,8 +51,10 @@ int do_truncate(struct dentry *dentry, loff_t length, 
unsigned int time_attrs,
                newattrs.ia_valid |= ATTR_FILE;
        }
 
-       /* Remove suid/sgid on truncate too */
-       ret = should_remove_suid(dentry);
+       /* Remove suid/sgid and security markings on truncate too */
+       ret = dentry_needs_remove_privs(dentry);
+       if (ret < 0)
+               return ret;
        if (ret)
                newattrs.ia_valid |= ret | ATTR_FORCE;
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 78ff19f0c323..ddaceede20ad 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2518,7 +2518,11 @@ extern struct inode *new_inode(struct super_block *sb);
 extern void free_inode_nonrcu(struct inode *inode);
 extern int should_remove_suid(struct dentry *);
 extern int file_remove_privs(struct file *);
-extern int file_needs_remove_privs(struct file *file);
+extern int dentry_needs_remove_privs(struct dentry *dentry);
+static inline int file_needs_remove_privs(struct file *file)
+{
+       return dentry_needs_remove_privs(file->f_path.dentry);
+}
 
 extern void __insert_inode_hash(struct inode *, unsigned long hashval);
 static inline void insert_inode_hash(struct inode *inode)
-- 
2.1.4

<Prev in Thread] Current Thread [Next in Thread>