Commit c884a1a3 authored by Andrew Morton's avatar Andrew Morton Committed by Linus Torvalds

[PATCH] remove_suid() should return error code

From: Nikita Danilov <Nikita@Namesys.COM>

remove_suid() ignores return value of notify_change()->i_op->setattr().
This mean, that even if file system fails to clear suid bit,
generic_file_aio_write_nolock() proceeds with write, which is unsafe.

Actually, even ext2's ->setattr() can fail, when trying to update ACL, for
example.

Attached patch modifies remove_suid() to return result of ->setattr(), and
updates in-tree callers.
parent 705e71d4
...@@ -1064,7 +1064,10 @@ ssize_t reiserfs_file_write( struct file *file, /* the file we are going to writ ...@@ -1064,7 +1064,10 @@ ssize_t reiserfs_file_write( struct file *file, /* the file we are going to writ
if ( count == 0 ) if ( count == 0 )
goto out; goto out;
remove_suid(file->f_dentry); res = remove_suid(file->f_dentry);
if (res)
goto out;
inode_update_time(inode, 1); /* Both mtime and ctime */ inode_update_time(inode, 1); /* Both mtime and ctime */
// Ok, we are done with all the checks. // Ok, we are done with all the checks.
......
...@@ -1297,7 +1297,7 @@ extern void __iget(struct inode * inode); ...@@ -1297,7 +1297,7 @@ extern void __iget(struct inode * inode);
extern void clear_inode(struct inode *); extern void clear_inode(struct inode *);
extern void destroy_inode(struct inode *); extern void destroy_inode(struct inode *);
extern struct inode *new_inode(struct super_block *); extern struct inode *new_inode(struct super_block *);
extern void remove_suid(struct dentry *); extern int remove_suid(struct dentry *);
extern void __insert_inode_hash(struct inode *, unsigned long hashval); extern void __insert_inode_hash(struct inode *, unsigned long hashval);
extern void remove_inode_hash(struct inode *); extern void remove_inode_hash(struct inode *);
......
...@@ -1502,10 +1502,11 @@ __grab_cache_page(struct address_space *mapping, unsigned long index, ...@@ -1502,10 +1502,11 @@ __grab_cache_page(struct address_space *mapping, unsigned long index,
* if suid or (sgid and xgrp) * if suid or (sgid and xgrp)
* remove privs * remove privs
*/ */
void remove_suid(struct dentry *dentry) int remove_suid(struct dentry *dentry)
{ {
mode_t mode = dentry->d_inode->i_mode; mode_t mode = dentry->d_inode->i_mode;
int kill = 0; int kill = 0;
int result = 0;
/* suid always must be killed */ /* suid always must be killed */
if (unlikely(mode & S_ISUID)) if (unlikely(mode & S_ISUID))
...@@ -1522,8 +1523,9 @@ void remove_suid(struct dentry *dentry) ...@@ -1522,8 +1523,9 @@ void remove_suid(struct dentry *dentry)
struct iattr newattrs; struct iattr newattrs;
newattrs.ia_valid = ATTR_FORCE | kill; newattrs.ia_valid = ATTR_FORCE | kill;
notify_change(dentry, &newattrs); result = notify_change(dentry, &newattrs);
} }
return result;
} }
EXPORT_SYMBOL(remove_suid); EXPORT_SYMBOL(remove_suid);
...@@ -1777,11 +1779,13 @@ generic_file_aio_write_nolock(struct kiocb *iocb, const struct iovec *iov, ...@@ -1777,11 +1779,13 @@ generic_file_aio_write_nolock(struct kiocb *iocb, const struct iovec *iov,
if (err) if (err)
goto out; goto out;
if (count == 0) if (count == 0)
goto out; goto out;
remove_suid(file->f_dentry); err = remove_suid(file->f_dentry);
if (err)
goto out;
inode_update_time(inode, 1); inode_update_time(inode, 1);
/* coalesce the iovecs and go direct-to-BIO for O_DIRECT */ /* coalesce the iovecs and go direct-to-BIO for O_DIRECT */
......
...@@ -1193,7 +1193,10 @@ shmem_file_write(struct file *file, const char __user *buf, size_t count, loff_t ...@@ -1193,7 +1193,10 @@ shmem_file_write(struct file *file, const char __user *buf, size_t count, loff_t
} }
} }
remove_suid(file->f_dentry); err = remove_suid(file->f_dentry);
if (err)
goto out;
inode->i_ctime = inode->i_mtime = CURRENT_TIME; inode->i_ctime = inode->i_mtime = CURRENT_TIME;
do { do {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment