* [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels
@ 2014-12-18 12:49 Jan Kara
2014-12-18 12:49 ` [PATCH 1/5] fs: Fix S_NOSEC handling Jan Kara
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Jan Kara @ 2014-12-18 12:49 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, xfs, linux-security-module, Jan Kara
Hello,
warning in XFS made me look into detail into how clearing of suid / sgid
bits and security labels is done. And I've spotted a few issues:
1) MS_NOSEC handling is broken - we set it after each file_remove_suid() call.
However we needn't have removed suid bit simply because we have
CAP_SYS_FSID and further writes to the file from processes without this
capability still need to clear the suid bit.
2) file_remove_suid() is a misnomer since it also handles removing of
security labels. It is even more confusing because should_remove_suid()
doesn't return whether file_remove_suid() is needed or not.
3) On truncate we do clear suid bits but not security labels. According to
documentation in include/linux/security.h that's a bug but please correct
me if I'm wrong.
4) ocfs2 doesn't clear security labels - hard to fix, I left it alone for now.
5) XFS didn't provide proper exclusion for clearing mode bits.
This series aims at fixing above issues.
Since v1 I have removed bogus patch changing inode_set_flags(), I have
updated changelog of patch 4/5 to better explain why ->inode_killpriv should
be called and I have included a fix for MS_NOSEC handling in this series.
Al, can you please merge the patches? Thanks!
Honza
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/5] fs: Fix S_NOSEC handling
2014-12-18 12:49 [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels Jan Kara
@ 2014-12-18 12:49 ` Jan Kara
2014-12-18 12:49 ` [PATCH 2/5] fs: Rename file_remove_suid() to file_remove_privs() Jan Kara
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2014-12-18 12:49 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, linux-security-module, Jan Kara, stable, xfs
file_remove_suid() could mistakenly set S_NOSEC inode bit when root was
modifying the file. As a result following writes to the file by ordinary
user would avoid clearing suid or sgid bits.
Fix the bug by checking actual mode bits before setting S_NOSEC.
CC: stable@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/inode.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/inode.c b/fs/inode.c
index 26753ba7b6d6..f5e01704a5c8 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1631,7 +1631,8 @@ int file_remove_suid(struct file *file)
error = security_inode_killpriv(dentry);
if (!error && killsuid)
error = __remove_suid(dentry, killsuid);
- if (!error && (inode->i_sb->s_flags & MS_NOSEC))
+ if (!error && (inode->i_sb->s_flags & MS_NOSEC) &&
+ !is_sxid(inode->i_mode))
inode->i_flags |= S_NOSEC;
return error;
--
1.8.4.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/5] fs: Rename file_remove_suid() to file_remove_privs()
2014-12-18 12:49 [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels Jan Kara
2014-12-18 12:49 ` [PATCH 1/5] fs: Fix S_NOSEC handling Jan Kara
@ 2014-12-18 12:49 ` Jan Kara
2014-12-18 12:49 ` [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything Jan Kara
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2014-12-18 12:49 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, linux-security-module, Jan Kara, xfs
file_remove_suid() is a misnomer since it removes also security related
xattrs and sets S_NOSEC flag. Also should_remove_suid() tells something
else than whether file_remove_suid() call is necessary which leads to
bugs.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/btrfs/file.c | 2 +-
fs/ceph/file.c | 2 +-
fs/fuse/file.c | 2 +-
fs/inode.c | 13 ++++++++-----
fs/ntfs/file.c | 2 +-
fs/xfs/xfs_file.c | 2 +-
include/linux/fs.h | 2 +-
mm/filemap.c | 2 +-
mm/filemap_xip.c | 2 +-
9 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index a18ceabd99a8..123795de7103 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1752,7 +1752,7 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
iov_iter_truncate(from, count);
- err = file_remove_suid(file);
+ err = file_remove_privs(file);
if (err) {
mutex_unlock(&inode->i_mutex);
goto out;
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index d7e0da8366e6..566c04d7ce68 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -901,7 +901,7 @@ static ssize_t ceph_write_iter(struct kiocb *iocb, struct iov_iter *from)
goto out;
iov_iter_truncate(from, count);
- err = file_remove_suid(file);
+ err = file_remove_privs(file);
if (err)
goto out;
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index caa8d95b24e8..dca9311e628f 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1214,7 +1214,7 @@ static ssize_t fuse_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
goto out;
iov_iter_truncate(from, count);
- err = file_remove_suid(file);
+ err = file_remove_privs(file);
if (err)
goto out;
diff --git a/fs/inode.c b/fs/inode.c
index f5e01704a5c8..fcc0886c5824 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1610,7 +1610,11 @@ static int __remove_suid(struct dentry *dentry, int kill)
return notify_change(dentry, &newattrs, NULL);
}
-int file_remove_suid(struct file *file)
+/*
+ * Remove special file priviledges (suid, security tags) when file is written
+ * to or truncated.
+ */
+int file_remove_privs(struct file *file)
{
struct dentry *dentry = file->f_path.dentry;
struct inode *inode = dentry->d_inode;
@@ -1637,7 +1641,7 @@ int file_remove_suid(struct file *file)
return error;
}
-EXPORT_SYMBOL(file_remove_suid);
+EXPORT_SYMBOL(file_remove_privs);
/**
* file_update_time - update mtime and ctime time
@@ -1906,9 +1910,8 @@ EXPORT_SYMBOL(inode_dio_done);
* inode is being instantiated). The reason for the cmpxchg() loop
* --- which wouldn't be necessary if all code paths which modify
* i_flags actually followed this rule, is that there is at least one
- * code path which doesn't today --- for example,
- * __generic_file_aio_write() calls file_remove_suid() without holding
- * i_mutex --- so we use cmpxchg() out of an abundance of caution.
+ * code path which doesn't today so we use cmpxchg() out of an abundance
+ * of caution.
*
* In the long run, i_mutex is overkill, and we should probably look
* at using the i_lock spinlock to protect i_flags, and then make sure
diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index 643faa44f22b..d9b8f3d953a9 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -2098,7 +2098,7 @@ static ssize_t ntfs_file_aio_write_nolock(struct kiocb *iocb,
goto out;
if (!count)
goto out;
- err = file_remove_suid(file);
+ err = file_remove_privs(file);
if (err)
goto out;
err = file_update_time(file);
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index eb596b419942..5c9e8296ebb3 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -557,7 +557,7 @@ restart:
* setgid bits if the process is not being run by root. This keeps
* people from modifying setuid and setgid binaries.
*/
- return file_remove_suid(file);
+ return file_remove_privs(file);
}
/*
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9ab779e8a63c..96b2d6a9a6cb 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2428,7 +2428,7 @@ extern struct inode *new_inode_pseudo(struct super_block *sb);
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_suid(struct file *);
+extern int file_remove_privs(struct file *);
extern void __insert_inode_hash(struct inode *, unsigned long hashval);
static inline void insert_inode_hash(struct inode *inode)
diff --git a/mm/filemap.c b/mm/filemap.c
index 14b4642279f1..da23d6fc102c 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2580,7 +2580,7 @@ ssize_t __generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
iov_iter_truncate(from, count);
- err = file_remove_suid(file);
+ err = file_remove_privs(file);
if (err)
goto out;
diff --git a/mm/filemap_xip.c b/mm/filemap_xip.c
index d8d9fe3f685c..8c0425b20b8a 100644
--- a/mm/filemap_xip.c
+++ b/mm/filemap_xip.c
@@ -423,7 +423,7 @@ xip_file_write(struct file *filp, const char __user *buf, size_t len,
if (count == 0)
goto out_backing;
- ret = file_remove_suid(filp);
+ ret = file_remove_privs(filp);
if (ret)
goto out_backing;
--
1.8.4.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything
2014-12-18 12:49 [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels Jan Kara
2014-12-18 12:49 ` [PATCH 1/5] fs: Fix S_NOSEC handling Jan Kara
2014-12-18 12:49 ` [PATCH 2/5] fs: Rename file_remove_suid() to file_remove_privs() Jan Kara
@ 2014-12-18 12:49 ` Jan Kara
2014-12-18 12:49 ` [PATCH 4/5] fs: Call security_ops->inode_killpriv on truncate Jan Kara
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2014-12-18 12:49 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, linux-security-module, Jan Kara, xfs
Provide function telling whether file_remove_privs() will do anything.
Currently we only have should_remove_suid() and that does something
slightly different.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/inode.c | 44 ++++++++++++++++++++++++++++++++------------
include/linux/fs.h | 1 +
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index fcc0886c5824..77942fac4121 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1598,7 +1598,32 @@ int should_remove_suid(struct dentry *dentry)
}
EXPORT_SYMBOL(should_remove_suid);
-static int __remove_suid(struct dentry *dentry, int kill)
+/*
+ * Return mask of changes for notify_change() that need to be done as a
+ * 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)
+{
+ struct dentry *dentry = file->f_path.dentry;
+ struct inode *inode = dentry->d_inode;
+ int mask = 0;
+ int ret;
+
+ if (IS_NOSEC(inode))
+ return 0;
+
+ mask = should_remove_suid(dentry);
+ ret = security_inode_need_killpriv(dentry);
+ if (ret < 0)
+ return ret;
+ if (ret)
+ mask |= ATTR_KILL_PRIV;
+ return mask;
+}
+EXPORT_SYMBOL(file_needs_remove_privs);
+
+static int __remove_privs(struct dentry *dentry, int kill)
{
struct iattr newattrs;
@@ -1618,23 +1643,18 @@ int file_remove_privs(struct file *file)
{
struct dentry *dentry = file->f_path.dentry;
struct inode *inode = dentry->d_inode;
- int killsuid;
- int killpriv;
+ int kill;
int error = 0;
/* Fast path for nothing security related */
if (IS_NOSEC(inode))
return 0;
- killsuid = should_remove_suid(dentry);
- killpriv = security_inode_need_killpriv(dentry);
-
- if (killpriv < 0)
- return killpriv;
- if (killpriv)
- error = security_inode_killpriv(dentry);
- if (!error && killsuid)
- error = __remove_suid(dentry, killsuid);
+ kill = file_needs_remove_privs(file);
+ if (kill < 0)
+ return kill;
+ if (kill)
+ error = __remove_privs(dentry, kill);
if (!error && (inode->i_sb->s_flags & MS_NOSEC) &&
!is_sxid(inode->i_mode))
inode->i_flags |= S_NOSEC;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 96b2d6a9a6cb..aac707cced66 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2429,6 +2429,7 @@ 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 void __insert_inode_hash(struct inode *, unsigned long hashval);
static inline void insert_inode_hash(struct inode *inode)
--
1.8.4.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/5] fs: Call security_ops->inode_killpriv on truncate
2014-12-18 12:49 [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels Jan Kara
` (2 preceding siblings ...)
2014-12-18 12:49 ` [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything Jan Kara
@ 2014-12-18 12:49 ` Jan Kara
2014-12-18 12:49 ` [PATCH 5/5] xfs: Correctly lock inode when removing suid and security marks Jan Kara
2015-01-08 9:18 ` [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels Jan Kara
5 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2014-12-18 12:49 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, xfs, linux-security-module, Jan Kara
Comment in include/linux/security.h says that ->inode_killpriv() should
be call 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@suse.cz>
---
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 77942fac4121..b5211a541437 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1603,9 +1603,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;
@@ -1621,7 +1620,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 de92c13b58be..e4e0863855d0 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 aac707cced66..c5ccc311e8fb 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2429,7 +2429,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)
--
1.8.4.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/5] xfs: Correctly lock inode when removing suid and security marks
2014-12-18 12:49 [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels Jan Kara
` (3 preceding siblings ...)
2014-12-18 12:49 ` [PATCH 4/5] fs: Call security_ops->inode_killpriv on truncate Jan Kara
@ 2014-12-18 12:49 ` Jan Kara
2015-01-08 9:18 ` [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels Jan Kara
5 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2014-12-18 12:49 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, xfs, linux-security-module, Jan Kara
Currently XFS calls file_remove_privs() without holding i_mutex. This is
wrong because that function can end up messing with file permissions and
security xattrs for which we need i_mutex held.
Fix the problem by grabbing iolock exclusively when we will need to
change anything in permissions / xattrs.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/xfs/xfs_file.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 5c9e8296ebb3..c6622aa1f8af 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -521,6 +521,13 @@ restart:
if (error)
return error;
+ /* For changing security info in file_remove_privs() we need i_mutex */
+ if (*iolock == XFS_IOLOCK_SHARED && !IS_NOSEC(inode)) {
+ xfs_rw_iunlock(ip, *iolock);
+ *iolock = XFS_IOLOCK_EXCL;
+ xfs_rw_ilock(ip, *iolock);
+ goto restart;
+ }
/*
* If the offset is beyond the size of the file, we need to zero any
* blocks that fall between the existing EOF and the start of this
@@ -557,7 +564,9 @@ restart:
* setgid bits if the process is not being run by root. This keeps
* people from modifying setuid and setgid binaries.
*/
- return file_remove_privs(file);
+ if (!IS_NOSEC(inode))
+ return file_remove_privs(file);
+ return 0;
}
/*
--
1.8.4.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels
2014-12-18 12:49 [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels Jan Kara
` (4 preceding siblings ...)
2014-12-18 12:49 ` [PATCH 5/5] xfs: Correctly lock inode when removing suid and security marks Jan Kara
@ 2015-01-08 9:18 ` Jan Kara
5 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2015-01-08 9:18 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, xfs, linux-security-module, Jan Kara
Hello!
Al, can you have a look at these fixes please? Thanks!
Honza
On Thu 18-12-14 13:49:01, Jan Kara wrote:
> Hello,
>
> warning in XFS made me look into detail into how clearing of suid / sgid
> bits and security labels is done. And I've spotted a few issues:
> 1) MS_NOSEC handling is broken - we set it after each file_remove_suid() call.
> However we needn't have removed suid bit simply because we have
> CAP_SYS_FSID and further writes to the file from processes without this
> capability still need to clear the suid bit.
> 2) file_remove_suid() is a misnomer since it also handles removing of
> security labels. It is even more confusing because should_remove_suid()
> doesn't return whether file_remove_suid() is needed or not.
> 3) On truncate we do clear suid bits but not security labels. According to
> documentation in include/linux/security.h that's a bug but please correct
> me if I'm wrong.
> 4) ocfs2 doesn't clear security labels - hard to fix, I left it alone for now.
> 5) XFS didn't provide proper exclusion for clearing mode bits.
>
> This series aims at fixing above issues.
>
> Since v1 I have removed bogus patch changing inode_set_flags(), I have
> updated changelog of patch 4/5 to better explain why ->inode_killpriv should
> be called and I have included a fix for MS_NOSEC handling in this series.
> Al, can you please merge the patches? Thanks!
>
> Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything
2015-05-21 14:05 [PATCH 0/5 v4] " Jan Kara
@ 2015-05-21 14:05 ` Jan Kara
0 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2015-05-21 14:05 UTC (permalink / raw)
To: Al Viro
Cc: Linus Torvalds, linux-fsdevel, dchinner, Serge Hallyn,
linux-security-module, Jan Kara
Provide function telling whether file_remove_privs() will do anything.
Currently we only have should_remove_suid() and that does something
slightly different.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/inode.c | 44 ++++++++++++++++++++++++++++++++------------
include/linux/fs.h | 1 +
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 48313a1444fb..620cb7686242 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1660,7 +1660,32 @@ int should_remove_suid(struct dentry *dentry)
}
EXPORT_SYMBOL(should_remove_suid);
-static int __remove_suid(struct dentry *dentry, int kill)
+/*
+ * Return mask of changes for notify_change() that need to be done as a
+ * 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)
+{
+ struct dentry *dentry = file->f_path.dentry;
+ struct inode *inode = d_inode(dentry);
+ int mask = 0;
+ int ret;
+
+ if (IS_NOSEC(inode))
+ return 0;
+
+ mask = should_remove_suid(dentry);
+ ret = security_inode_need_killpriv(dentry);
+ if (ret < 0)
+ return ret;
+ if (ret)
+ mask |= ATTR_KILL_PRIV;
+ return mask;
+}
+EXPORT_SYMBOL(file_needs_remove_privs);
+
+static int __remove_privs(struct dentry *dentry, int kill)
{
struct iattr newattrs;
@@ -1680,23 +1705,18 @@ int file_remove_privs(struct file *file)
{
struct dentry *dentry = file->f_path.dentry;
struct inode *inode = d_inode(dentry);
- int killsuid;
- int killpriv;
+ int kill;
int error = 0;
/* Fast path for nothing security related */
if (IS_NOSEC(inode))
return 0;
- killsuid = should_remove_suid(dentry);
- killpriv = security_inode_need_killpriv(dentry);
-
- if (killpriv < 0)
- return killpriv;
- if (killpriv)
- error = security_inode_killpriv(dentry);
- if (!error && killsuid)
- error = __remove_suid(dentry, killsuid);
+ kill = file_needs_remove_privs(file);
+ if (kill < 0)
+ return kill;
+ if (kill)
+ error = __remove_privs(dentry, kill);
if (!error)
inode_has_no_xattr(inode);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 082f66274e63..a0b21f1ae7cb 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2553,6 +2553,7 @@ 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 void __insert_inode_hash(struct inode *, unsigned long hashval);
static inline void insert_inode_hash(struct inode *inode)
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything
2015-05-19 9:46 [PATCH 0/5 v3] fs: Fixes for removing xid bits and security labels Jan Kara
@ 2015-05-19 9:46 ` Jan Kara
0 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2015-05-19 9:46 UTC (permalink / raw)
To: Al Viro
Cc: Linus Torvalds, linux-fsdevel, dchinner, Serge Hallyn,
linux-security-module, Jan Kara
Provide function telling whether file_remove_privs() will do anything.
Currently we only have should_remove_suid() and that does something
slightly different.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/inode.c | 44 ++++++++++++++++++++++++++++++++------------
include/linux/fs.h | 1 +
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index aa140f7bf336..547bcd7033c5 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1660,7 +1660,32 @@ int should_remove_suid(struct dentry *dentry)
}
EXPORT_SYMBOL(should_remove_suid);
-static int __remove_suid(struct dentry *dentry, int kill)
+/*
+ * Return mask of changes for notify_change() that need to be done as a
+ * 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)
+{
+ struct dentry *dentry = file->f_path.dentry;
+ struct inode *inode = d_inode(dentry);
+ int mask = 0;
+ int ret;
+
+ if (IS_NOSEC(inode))
+ return 0;
+
+ mask = should_remove_suid(dentry);
+ ret = security_inode_need_killpriv(dentry);
+ if (ret < 0)
+ return ret;
+ if (ret)
+ mask |= ATTR_KILL_PRIV;
+ return mask;
+}
+EXPORT_SYMBOL(file_needs_remove_privs);
+
+static int __remove_privs(struct dentry *dentry, int kill)
{
struct iattr newattrs;
@@ -1680,23 +1705,18 @@ int file_remove_privs(struct file *file)
{
struct dentry *dentry = file->f_path.dentry;
struct inode *inode = d_inode(dentry);
- int killsuid;
- int killpriv;
+ int kill;
int error = 0;
/* Fast path for nothing security related */
if (IS_NOSEC(inode))
return 0;
- killsuid = should_remove_suid(dentry);
- killpriv = security_inode_need_killpriv(dentry);
-
- if (killpriv < 0)
- return killpriv;
- if (killpriv)
- error = security_inode_killpriv(dentry);
- if (!error && killsuid)
- error = __remove_suid(dentry, killsuid);
+ kill = file_needs_remove_privs(file);
+ if (kill < 0)
+ return kill;
+ if (kill)
+ error = __remove_privs(dentry, kill);
if (!error)
inode_has_no_xattr(inode);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 082f66274e63..a0b21f1ae7cb 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2553,6 +2553,7 @@ 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 void __insert_inode_hash(struct inode *, unsigned long hashval);
static inline void insert_inode_hash(struct inode *inode)
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything
2015-03-03 10:38 [PATCH 0/5 v2 RESEND] fs: Fixes for removing xid bits and security labels Jan Kara
@ 2015-03-03 10:38 ` Jan Kara
0 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2015-03-03 10:38 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, xfs, Jan Kara
Provide function telling whether file_remove_privs() will do anything.
Currently we only have should_remove_suid() and that does something
slightly different.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/inode.c | 44 ++++++++++++++++++++++++++++++++------------
include/linux/fs.h | 1 +
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index bc5fe06b8784..3407bedc1c7b 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1660,7 +1660,32 @@ int should_remove_suid(struct dentry *dentry)
}
EXPORT_SYMBOL(should_remove_suid);
-static int __remove_suid(struct dentry *dentry, int kill)
+/*
+ * Return mask of changes for notify_change() that need to be done as a
+ * 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)
+{
+ struct dentry *dentry = file->f_path.dentry;
+ struct inode *inode = dentry->d_inode;
+ int mask = 0;
+ int ret;
+
+ if (IS_NOSEC(inode))
+ return 0;
+
+ mask = should_remove_suid(dentry);
+ ret = security_inode_need_killpriv(dentry);
+ if (ret < 0)
+ return ret;
+ if (ret)
+ mask |= ATTR_KILL_PRIV;
+ return mask;
+}
+EXPORT_SYMBOL(file_needs_remove_privs);
+
+static int __remove_privs(struct dentry *dentry, int kill)
{
struct iattr newattrs;
@@ -1680,23 +1705,18 @@ int file_remove_privs(struct file *file)
{
struct dentry *dentry = file->f_path.dentry;
struct inode *inode = dentry->d_inode;
- int killsuid;
- int killpriv;
+ int kill;
int error = 0;
/* Fast path for nothing security related */
if (IS_NOSEC(inode))
return 0;
- killsuid = should_remove_suid(dentry);
- killpriv = security_inode_need_killpriv(dentry);
-
- if (killpriv < 0)
- return killpriv;
- if (killpriv)
- error = security_inode_killpriv(dentry);
- if (!error && killsuid)
- error = __remove_suid(dentry, killsuid);
+ kill = file_needs_remove_privs(file);
+ if (kill < 0)
+ return kill;
+ if (kill)
+ error = __remove_privs(dentry, kill);
if (!error && (inode->i_sb->s_flags & MS_NOSEC) &&
!is_sxid(inode->i_mode))
inode->i_flags |= S_NOSEC;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 676e431247bd..78ff19f0c323 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2518,6 +2518,7 @@ 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 void __insert_inode_hash(struct inode *, unsigned long hashval);
static inline void insert_inode_hash(struct inode *inode)
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything
2014-12-04 13:27 [PATCH 0/5] fs: Fixes for removing xid bits and security labels Jan Kara
@ 2014-12-04 13:27 ` Jan Kara
0 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2014-12-04 13:27 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, xfs, linux-security-module, Jan Kara
Provide function telling whether file_remove_privs() will do anything.
Currently we only have should_remove_suid() and that does something
slightly different.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/inode.c | 44 ++++++++++++++++++++++++++++++++------------
include/linux/fs.h | 1 +
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 3490389dc813..6807a2707828 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1598,7 +1598,32 @@ int should_remove_suid(struct dentry *dentry)
}
EXPORT_SYMBOL(should_remove_suid);
-static int __remove_suid(struct dentry *dentry, int kill)
+/*
+ * Return mask of changes for notify_change() that need to be done as a
+ * 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)
+{
+ struct dentry *dentry = file->f_path.dentry;
+ struct inode *inode = dentry->d_inode;
+ int mask = 0;
+ int ret;
+
+ if (IS_NOSEC(inode))
+ return 0;
+
+ mask = should_remove_suid(dentry);
+ ret = security_inode_need_killpriv(dentry);
+ if (ret < 0)
+ return ret;
+ if (ret)
+ mask |= ATTR_KILL_PRIV;
+ return mask;
+}
+EXPORT_SYMBOL(file_needs_remove_privs);
+
+static int __remove_privs(struct dentry *dentry, int kill)
{
struct iattr newattrs;
@@ -1618,23 +1643,18 @@ int file_remove_privs(struct file *file)
{
struct dentry *dentry = file->f_path.dentry;
struct inode *inode = dentry->d_inode;
- int killsuid;
- int killpriv;
+ int kill;
int error = 0;
/* Fast path for nothing security related */
if (IS_NOSEC(inode))
return 0;
- killsuid = should_remove_suid(dentry);
- killpriv = security_inode_need_killpriv(dentry);
-
- if (killpriv < 0)
- return killpriv;
- if (killpriv)
- error = security_inode_killpriv(dentry);
- if (!error && killsuid)
- error = __remove_suid(dentry, killsuid);
+ kill = file_needs_remove_privs(file);
+ if (kill < 0)
+ return kill;
+ if (kill)
+ error = __remove_privs(dentry, kill);
if (!error && (inode->i_sb->s_flags & MS_NOSEC) &&
!is_sxid(inode->i_mode))
inode->i_flags |= S_NOSEC;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 96b2d6a9a6cb..aac707cced66 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2429,6 +2429,7 @@ 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 void __insert_inode_hash(struct inode *, unsigned long hashval);
static inline void insert_inode_hash(struct inode *inode)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-05-21 14:05 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-18 12:49 [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels Jan Kara
2014-12-18 12:49 ` [PATCH 1/5] fs: Fix S_NOSEC handling Jan Kara
2014-12-18 12:49 ` [PATCH 2/5] fs: Rename file_remove_suid() to file_remove_privs() Jan Kara
2014-12-18 12:49 ` [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything Jan Kara
2014-12-18 12:49 ` [PATCH 4/5] fs: Call security_ops->inode_killpriv on truncate Jan Kara
2014-12-18 12:49 ` [PATCH 5/5] xfs: Correctly lock inode when removing suid and security marks Jan Kara
2015-01-08 9:18 ` [PATCH 0/5 v2] fs: Fixes for removing xid bits and security labels Jan Kara
-- strict thread matches above, loose matches on Subject: below --
2015-05-21 14:05 [PATCH 0/5 v4] " Jan Kara
2015-05-21 14:05 ` [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything Jan Kara
2015-05-19 9:46 [PATCH 0/5 v3] fs: Fixes for removing xid bits and security labels Jan Kara
2015-05-19 9:46 ` [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything Jan Kara
2015-03-03 10:38 [PATCH 0/5 v2 RESEND] fs: Fixes for removing xid bits and security labels Jan Kara
2015-03-03 10:38 ` [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything Jan Kara
2014-12-04 13:27 [PATCH 0/5] fs: Fixes for removing xid bits and security labels Jan Kara
2014-12-04 13:27 ` [PATCH 3/5] fs: Provide function telling whether file_remove_privs() will do anything Jan Kara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).