diff options
author | Christoph Hellwig <hch@lst.de> | 2010-06-04 05:30:04 -0400 |
---|---|---|
committer | Al Viro <viro@zeniv.linux.org.uk> | 2010-08-09 16:47:39 -0400 |
commit | 2c27c65ed0696f0b5df2dad2cf6462d72164d547 (patch) | |
tree | 7d9036e3dea98938f7fd7074366ee73929e9b2e5 /fs/ubifs | |
parent | db78b877f7744bec4a9d9f9e7d10da3931d7cd39 (diff) |
check ATTR_SIZE contraints in inode_change_ok
Make sure we check the truncate constraints early on in ->setattr by adding
those checks to inode_change_ok. Also clean up and document inode_change_ok
to make this obvious.
As a fallout we don't have to call inode_newsize_ok from simple_setsize and
simplify it down to a truncate_setsize which doesn't return an error. This
simplifies a lot of setattr implementations and means we use truncate_setsize
almost everywhere. Get rid of fat_setsize now that it's trivial and mark
ext2_setsize static to make the calling convention obvious.
Keep the inode_newsize_ok in vmtruncate for now as all callers need an
audit for its removal anyway.
Note: setattr code in ecryptfs doesn't call inode_change_ok at all and
needs a deeper audit, but that is left for later.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Diffstat (limited to 'fs/ubifs')
-rw-r--r-- | fs/ubifs/file.c | 23 | ||||
-rw-r--r-- | fs/ubifs/ubifs.h | 2 |
2 files changed, 9 insertions, 16 deletions
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c index 12f445cee9f7..03ae894c45de 100644 --- a/fs/ubifs/file.c +++ b/fs/ubifs/file.c | |||
@@ -967,14 +967,15 @@ static int do_writepage(struct page *page, int len) | |||
967 | * the page locked, and it locks @ui_mutex. However, write-back does take inode | 967 | * the page locked, and it locks @ui_mutex. However, write-back does take inode |
968 | * @i_mutex, which means other VFS operations may be run on this inode at the | 968 | * @i_mutex, which means other VFS operations may be run on this inode at the |
969 | * same time. And the problematic one is truncation to smaller size, from where | 969 | * same time. And the problematic one is truncation to smaller size, from where |
970 | * we have to call 'simple_setsize()', which first changes @inode->i_size, then | 970 | * we have to call 'truncate_setsize()', which first changes @inode->i_size, then |
971 | * drops the truncated pages. And while dropping the pages, it takes the page | 971 | * drops the truncated pages. And while dropping the pages, it takes the page |
972 | * lock. This means that 'do_truncation()' cannot call 'simple_setsize()' with | 972 | * lock. This means that 'do_truncation()' cannot call 'truncate_setsize()' with |
973 | * @ui_mutex locked, because it would deadlock with 'ubifs_writepage()'. This | 973 | * @ui_mutex locked, because it would deadlock with 'ubifs_writepage()'. This |
974 | * means that @inode->i_size is changed while @ui_mutex is unlocked. | 974 | * means that @inode->i_size is changed while @ui_mutex is unlocked. |
975 | * | 975 | * |
976 | * XXX: with the new truncate the above is not true anymore, the simple_setsize | 976 | * XXX(truncate): with the new truncate sequence this is not true anymore, |
977 | * calls can be replaced with the individual components. | 977 | * and the calls to truncate_setsize can be move around freely. They should |
978 | * be moved to the very end of the truncate sequence. | ||
978 | * | 979 | * |
979 | * But in 'ubifs_writepage()' we have to guarantee that we do not write beyond | 980 | * But in 'ubifs_writepage()' we have to guarantee that we do not write beyond |
980 | * inode size. How do we do this if @inode->i_size may became smaller while we | 981 | * inode size. How do we do this if @inode->i_size may became smaller while we |
@@ -1128,9 +1129,7 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode, | |||
1128 | budgeted = 0; | 1129 | budgeted = 0; |
1129 | } | 1130 | } |
1130 | 1131 | ||
1131 | err = simple_setsize(inode, new_size); | 1132 | truncate_setsize(inode, new_size); |
1132 | if (err) | ||
1133 | goto out_budg; | ||
1134 | 1133 | ||
1135 | if (offset) { | 1134 | if (offset) { |
1136 | pgoff_t index = new_size >> PAGE_CACHE_SHIFT; | 1135 | pgoff_t index = new_size >> PAGE_CACHE_SHIFT; |
@@ -1217,16 +1216,14 @@ static int do_setattr(struct ubifs_info *c, struct inode *inode, | |||
1217 | 1216 | ||
1218 | if (attr->ia_valid & ATTR_SIZE) { | 1217 | if (attr->ia_valid & ATTR_SIZE) { |
1219 | dbg_gen("size %lld -> %lld", inode->i_size, new_size); | 1218 | dbg_gen("size %lld -> %lld", inode->i_size, new_size); |
1220 | err = simple_setsize(inode, new_size); | 1219 | truncate_setsize(inode, new_size); |
1221 | if (err) | ||
1222 | goto out; | ||
1223 | } | 1220 | } |
1224 | 1221 | ||
1225 | mutex_lock(&ui->ui_mutex); | 1222 | mutex_lock(&ui->ui_mutex); |
1226 | if (attr->ia_valid & ATTR_SIZE) { | 1223 | if (attr->ia_valid & ATTR_SIZE) { |
1227 | /* Truncation changes inode [mc]time */ | 1224 | /* Truncation changes inode [mc]time */ |
1228 | inode->i_mtime = inode->i_ctime = ubifs_current_time(inode); | 1225 | inode->i_mtime = inode->i_ctime = ubifs_current_time(inode); |
1229 | /* 'simple_setsize()' changed @i_size, update @ui_size */ | 1226 | /* 'truncate_setsize()' changed @i_size, update @ui_size */ |
1230 | ui->ui_size = inode->i_size; | 1227 | ui->ui_size = inode->i_size; |
1231 | } | 1228 | } |
1232 | 1229 | ||
@@ -1248,10 +1245,6 @@ static int do_setattr(struct ubifs_info *c, struct inode *inode, | |||
1248 | if (IS_SYNC(inode)) | 1245 | if (IS_SYNC(inode)) |
1249 | err = inode->i_sb->s_op->write_inode(inode, NULL); | 1246 | err = inode->i_sb->s_op->write_inode(inode, NULL); |
1250 | return err; | 1247 | return err; |
1251 | |||
1252 | out: | ||
1253 | ubifs_release_budget(c, &req); | ||
1254 | return err; | ||
1255 | } | 1248 | } |
1256 | 1249 | ||
1257 | int ubifs_setattr(struct dentry *dentry, struct iattr *attr) | 1250 | int ubifs_setattr(struct dentry *dentry, struct iattr *attr) |
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 04310878f449..0c9876b396dd 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h | |||
@@ -379,7 +379,7 @@ struct ubifs_gced_idx_leb { | |||
379 | * The @ui_size is a "shadow" variable for @inode->i_size and UBIFS uses | 379 | * The @ui_size is a "shadow" variable for @inode->i_size and UBIFS uses |
380 | * @ui_size instead of @inode->i_size. The reason for this is that UBIFS cannot | 380 | * @ui_size instead of @inode->i_size. The reason for this is that UBIFS cannot |
381 | * make sure @inode->i_size is always changed under @ui_mutex, because it | 381 | * make sure @inode->i_size is always changed under @ui_mutex, because it |
382 | * cannot call 'simple_setsize()' with @ui_mutex locked, because it would deadlock | 382 | * cannot call 'truncate_setsize()' with @ui_mutex locked, because it would deadlock |
383 | * with 'ubifs_writepage()' (see file.c). All the other inode fields are | 383 | * with 'ubifs_writepage()' (see file.c). All the other inode fields are |
384 | * changed under @ui_mutex, so they do not need "shadow" fields. Note, one | 384 | * changed under @ui_mutex, so they do not need "shadow" fields. Note, one |
385 | * could consider to rework locking and base it on "shadow" fields. | 385 | * could consider to rework locking and base it on "shadow" fields. |