Skip to contentSkip to conversation

ext2: fix a block leak

RFC PATCH In review Al Viro 2 participants 3 messages May 18, 2018 – May 23, 2018

Subsystems (2)
VFS ext2 file system
lore
Share & tools
lore

Al Viro

author maintainer
#0 lore raw

RFC PATCH 1/1 ext2: fix a block leak

open file, unlink it, then use ioctl(2) to make it immutable or append only. Now close it and watch the blocks *not* freed...

Immutable/append-only checks belong in ->setattr().
Note: the bug is old and backport to anything prior to 737f2e93b972 ("ext2: convert to use the new truncate convention") will need these checks lifted into ext2_setattr().

Cc [email protected]
Signed-off-by Al Viro <[email protected]>

---

fs/ext2/inode.c +0 -10
index 1e01fabef130..71635909df3b 100644
@@ -1264,21 +1264,11 @@ static void __ext2_truncate_blocks(struct inode *inode, loff_t offset)
 
 static void ext2_truncate_blocks(struct inode *inode, loff_t offset)
 {
-	/*
-	 * XXX: it seems like a bug here that we don't allow
-	 * IS_APPEND inode to have blocks-past-i_size trimmed off.
-	 * review and fix this.
-	 *
-	 * Also would be nice to be able to handle IO errors and such,
-	 * but that's probably too much to ask.
-	 */
 	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
 	    S_ISLNK(inode->i_mode)))
 		return;
 	if (ext2_inode_is_fast_symlink(inode))
 		return;
-	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
-		return;
 
 	dax_sem_down_write(EXT2_I(inode));
 	__ext2_truncate_blocks(inode, offset);
reviewer
#1 lore raw
Quoted 10 lines · Al Viro

open file, unlink it, then use ioctl(2) to make it immutable or append only. Now close it and watch the blocks *not* freed...

Immutable/append-only checks belong in ->setattr().
Note: the bug is old and backport to anything prior to 737f2e93b972 ("ext2: convert to use the new truncate convention") will need these checks lifted into ext2_setattr().

Cc [email protected]
Signed-off-by Al Viro <[email protected]>

Sorry, I've been busy with other stuff after LSF/MM and then forgot about this. Thanks for the patch! I've added the patch to my tree.

Honza

Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR
reviewer
#2 lore raw
Quoted 9 lines · Jan Kara
Quoted 8 lines · Al Viro

open file, unlink it, then use ioctl(2) to make it immutable or append only. Now close it and watch the blocks *not* freed...

Immutable/append-only checks belong in ->setattr().
Note: the bug is old and backport to anything prior to 737f2e93b972 ("ext2: convert to use the new truncate convention") will need these checks lifted into ext2_setattr().

Quoted 5 lines
Quoted 2 lines
Cc [email protected]
Signed-off-by Al Viro <[email protected]>

Sorry, I've been busy with other stuff after LSF/MM and then forgot about this. Thanks for the patch! I've added the patch to my tree.

Bah, and now I see (thanks Amir for the notice) you've already pushed it to Linus. Sorry for the noise.

Honza

Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR