Commit d932805b3dc8c6d80d8948f7d7d0d8336d53b2ed
Committed by
Bob Copeland
Bob Copeland
1 parent
cdb26496db
Exists in
master
and in
4 other branches
omfs: merge unlink() and rmdir(), close leak in rename()
In case of directory-overwriting rename(), omfs forgot to mark the victim doomed, so omfs_evict_inode() didn't free it. We could fix that by calling omfs_rmdir() for directory victims instead of doing omfs_unlink(), but it's easier to merge omfs_unlink() and omfs_rmdir() instead. Note that we have no hardlinks here. It also makes the checks in omfs_rename() go away - they fold into what omfs_remove() does when it runs into a directory. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Bob Copeland <me@bobcopeland.com>
Showing 1 changed file with 13 additions and 40 deletions Side-by-side Diff
fs/omfs/dir.c
| ... | ... | @@ -235,35 +235,24 @@ |
| 235 | 235 | return *ptr != ~0; |
| 236 | 236 | } |
| 237 | 237 | |
| 238 | -static int omfs_unlink(struct inode *dir, struct dentry *dentry) | |
| 238 | +static int omfs_remove(struct inode *dir, struct dentry *dentry) | |
| 239 | 239 | { |
| 240 | - int ret; | |
| 241 | 240 | struct inode *inode = dentry->d_inode; |
| 241 | + int ret; | |
| 242 | 242 | |
| 243 | + if (S_ISDIR(inode->i_mode) && !omfs_dir_is_empty(inode)) | |
| 244 | + return -ENOTEMPTY; | |
| 245 | + | |
| 243 | 246 | ret = omfs_delete_entry(dentry); |
| 244 | 247 | if (ret) |
| 245 | - goto end_unlink; | |
| 246 | - | |
| 247 | - inode_dec_link_count(inode); | |
| 248 | + return ret; | |
| 249 | + | |
| 250 | + clear_nlink(inode); | |
| 251 | + mark_inode_dirty(inode); | |
| 248 | 252 | mark_inode_dirty(dir); |
| 249 | - | |
| 250 | -end_unlink: | |
| 251 | - return ret; | |
| 253 | + return 0; | |
| 252 | 254 | } |
| 253 | 255 | |
| 254 | -static int omfs_rmdir(struct inode *dir, struct dentry *dentry) | |
| 255 | -{ | |
| 256 | - int err = -ENOTEMPTY; | |
| 257 | - struct inode *inode = dentry->d_inode; | |
| 258 | - | |
| 259 | - if (omfs_dir_is_empty(inode)) { | |
| 260 | - err = omfs_unlink(dir, dentry); | |
| 261 | - if (!err) | |
| 262 | - inode_dec_link_count(inode); | |
| 263 | - } | |
| 264 | - return err; | |
| 265 | -} | |
| 266 | - | |
| 267 | 256 | static int omfs_add_node(struct inode *dir, struct dentry *dentry, int mode) |
| 268 | 257 | { |
| 269 | 258 | int err; |
| 270 | 259 | |
| 271 | 260 | |
| 272 | 261 | |
| ... | ... | @@ -385,33 +374,17 @@ |
| 385 | 374 | { |
| 386 | 375 | struct inode *new_inode = new_dentry->d_inode; |
| 387 | 376 | struct inode *old_inode = old_dentry->d_inode; |
| 388 | - struct buffer_head *bh; | |
| 389 | - int is_dir; | |
| 390 | 377 | int err; |
| 391 | 378 | |
| 392 | - is_dir = S_ISDIR(old_inode->i_mode); | |
| 393 | - | |
| 394 | 379 | if (new_inode) { |
| 395 | 380 | /* overwriting existing file/dir */ |
| 396 | - err = -ENOTEMPTY; | |
| 397 | - if (is_dir && !omfs_dir_is_empty(new_inode)) | |
| 398 | - goto out; | |
| 399 | - | |
| 400 | - err = -ENOENT; | |
| 401 | - bh = omfs_find_entry(new_dir, new_dentry->d_name.name, | |
| 402 | - new_dentry->d_name.len); | |
| 403 | - if (IS_ERR(bh)) | |
| 404 | - goto out; | |
| 405 | - brelse(bh); | |
| 406 | - | |
| 407 | - err = omfs_unlink(new_dir, new_dentry); | |
| 381 | + err = omfs_remove(new_dir, new_dentry); | |
| 408 | 382 | if (err) |
| 409 | 383 | goto out; |
| 410 | 384 | } |
| 411 | 385 | |
| 412 | 386 | /* since omfs locates files by name, we need to unlink _before_ |
| 413 | 387 | * adding the new link or we won't find the old one */ |
| 414 | - inode_inc_link_count(old_inode); | |
| 415 | 388 | err = omfs_delete_entry(old_dentry); |
| 416 | 389 | if (err) |
| 417 | 390 | goto out; |
| ... | ... | @@ -488,8 +461,8 @@ |
| 488 | 461 | .mkdir = omfs_mkdir, |
| 489 | 462 | .rename = omfs_rename, |
| 490 | 463 | .create = omfs_create, |
| 491 | - .unlink = omfs_unlink, | |
| 492 | - .rmdir = omfs_rmdir, | |
| 464 | + .unlink = omfs_remove, | |
| 465 | + .rmdir = omfs_remove, | |
| 493 | 466 | }; |
| 494 | 467 | |
| 495 | 468 | const struct file_operations omfs_dir_operations = { |