lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 12 Aug 2021 11:01:34 -0400 From: "Theodore Ts'o" <tytso@....edu> To: Jan Kara <jack@...e.cz> Cc: linux-ext4@...r.kernel.org Subject: Re: [PATCH 3/5] ext4: Speedup ext4 orphan inode handling On Wed, Aug 11, 2021 at 12:19:13PM +0200, Jan Kara wrote: > +static int ext4_orphan_file_del(handle_t *handle, struct inode *inode) > +{ > + struct ext4_orphan_info *oi = &EXT4_SB(inode->i_sb)->s_orphan_info; > + __le32 *bdata; > + int blk, off; > + int inodes_per_ob = ext4_inodes_per_orphan_block(inode->i_sb); > + int ret = 0; > + > + if (!handle) > + goto out; > + blk = EXT4_I(inode)->i_orphan_idx / inodes_per_ob; > + off = EXT4_I(inode)->i_orphan_idx % inodes_per_ob; > + if (WARN_ON_ONCE(blk >= oi->of_blocks)) > + goto out; > + > + ret = ext4_journal_get_write_access(handle, inode->i_sb, > + oi->of_binfo[blk].ob_bh, EXT4_JTR_ORPHAN_FILE); > + if (ret) > + goto out; If ext4_journal_get_write_access() fails, we effectively drop the inode from the orphan list (as far as the in-memory inode is concerned), although the inode will still be listed in the orphan file. This can be really unfortunate since if the inode gets reallocated for some other purpose, since its inode number is left in the orphan block, on the next remount, this could lead to data loss. In the orphan list code, we leave the inode on the linked list, which is not great, since that will prevent the inode from being freed, but at least we're keeping the in-memory and on-disk state in sync and we avoid the data loss scenario when the inode gets reused. I'll also note that all or at least most of the callers of ext4_orphan_del() are doing error checking, which also unfortunate (although what are we supposed to do in case of a failure here?). I think keeping things consistent with the existing non-optimal "error handle" at least makes things no worse than before, but looking at the error handling, I'm left with a sense of unease. What do you think? - Ted
Powered by blists - more mailing lists