| Message ID | 20241008121152.3771906-1-libaokun@huaweicloud.com |
|---|---|
| State | New |
| Headers | show |
| Series | ext4: WARN if a full dir leaf block has only one dentry | expand |
On Tue 08-10-24 20:11:52, libaokun@huaweicloud.com wrote: > From: Baokun Li <libaokun1@huawei.com> > > The maximum length of a filename is 255 and the minimum block size is 1024, > so it is always guaranteed that the number of entries is greater than or > equal to 2 when do_split() is called. So unless ext4_dx_add_entry() and > make_indexed_dir() or some other functions are buggy, 'split == 0' will > not occur. > > Setting 'continued' to 0 in this case masks the problem that the file > system has become corrupted, even though it prevents possible out-of-bounds > access. Hence WARN_ON_ONCE() is used to check if 'split' is 0, and if it is > then warns and returns an error to abort split. > > Suggested-by: Theodore Ts'o <tytso@mit.edu> > Link: https://lore.kernel.org/r/20240823160518.GA424729@mit.edu > Signed-off-by: Baokun Li <libaokun1@huawei.com> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/ext4/namei.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 790db7eac6c2..08d15cd2b594 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -2000,8 +2000,17 @@ static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir, > else > split = count/2; > > + if (WARN_ON_ONCE(split == 0)) { > + /* Should never happen, but avoid out-of-bounds access below */ > + ext4_error_inode_block(dir, (*bh)->b_blocknr, 0, > + "bad indexed directory? hash=%08x:%08x count=%d move=%u", > + hinfo->hash, hinfo->minor_hash, count, move); > + err = -EFSCORRUPTED; > + goto out; > + } > + > hash2 = map[split].hash; > - continued = split > 0 ? hash2 == map[split - 1].hash : 0; > + continued = hash2 == map[split - 1].hash; > dxtrace(printk(KERN_INFO "Split block %lu at %x, %i/%i\n", > (unsigned long)dx_get_block(frame->at), > hash2, split, count-split)); > @@ -2043,10 +2052,11 @@ static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir, > return de; > > journal_error: > + ext4_std_error(dir->i_sb, err); > +out: > brelse(*bh); > brelse(bh2); > *bh = NULL; > - ext4_std_error(dir->i_sb, err); > return ERR_PTR(err); > } > > -- > 2.46.1 >
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 790db7eac6c2..08d15cd2b594 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -2000,8 +2000,17 @@ static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir, else split = count/2; + if (WARN_ON_ONCE(split == 0)) { + /* Should never happen, but avoid out-of-bounds access below */ + ext4_error_inode_block(dir, (*bh)->b_blocknr, 0, + "bad indexed directory? hash=%08x:%08x count=%d move=%u", + hinfo->hash, hinfo->minor_hash, count, move); + err = -EFSCORRUPTED; + goto out; + } + hash2 = map[split].hash; - continued = split > 0 ? hash2 == map[split - 1].hash : 0; + continued = hash2 == map[split - 1].hash; dxtrace(printk(KERN_INFO "Split block %lu at %x, %i/%i\n", (unsigned long)dx_get_block(frame->at), hash2, split, count-split)); @@ -2043,10 +2052,11 @@ static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir, return de; journal_error: + ext4_std_error(dir->i_sb, err); +out: brelse(*bh); brelse(bh2); *bh = NULL; - ext4_std_error(dir->i_sb, err); return ERR_PTR(err); }