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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <Y4ko3OL57iyiRC0W@debian-BULLSEYE-live-builder-AMD64> Date: Thu, 1 Dec 2022 17:21:16 -0500 From: Eric Whitney <enwlinux@...il.com> To: Ye Bin <yebin@...weicloud.com> Cc: tytso@....edu, adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org, jack@...e.cz, Ye Bin <yebin10@...wei.com>, syzbot+05a0f0ccab4a25626e38@...kaller.appspotmail.com Subject: Re: [PATCH v2 1/3] ext4: fix incorrect calculate 'reserved' in '__es_remove_extent' when enable bigalloc feature * Ye Bin <yebin@...weicloud.com>: > From: Ye Bin <yebin10@...wei.com> > > Syzbot report issue as follows: > EXT4-fs error (device loop0): ext4_validate_block_bitmap:398: comm rep: bg 0: block 5: invalid block bitmap > EXT4-fs (loop0): Delayed block allocation failed for inode 18 at logical offset 0 with max blocks 32 with error 28 > EXT4-fs (loop0): This should not happen!! Data will be lost > > EXT4-fs (loop0): Total free blocks count 0 > EXT4-fs (loop0): Free/Dirty block details > EXT4-fs (loop0): free_blocks=0 > EXT4-fs (loop0): dirty_blocks=32 > EXT4-fs (loop0): Block reservation details > EXT4-fs (loop0): i_reserved_data_blocks=2 > EXT4-fs (loop0): Inode 18 (00000000845cd634): i_reserved_data_blocks (1) not cleared! > > Above issue happens as follows: > Assume: > sbi->s_cluster_ratio = 16 > Step1: Insert delay block [0, 31] -> ei->i_reserved_data_blocks=2 > Step2: > ext4_writepages > mpage_map_and_submit_extent -> return failed > mpage_release_unused_pages -> to release [0, 30] > ext4_es_remove_extent -> remove lblk=0 end=30 > __es_remove_extent -> len1=0 len2=31-30=1 > __es_remove_extent: > ... > if (len2 > 0) { > ... > if (len1 > 0) { > ... > } else { > es->es_lblk = end + 1; > es->es_len = len2; > ... > } > if (count_reserved) > count_rsvd(inode, lblk, orig_es.es_len - len1 - len2, &orig_es, &rc); > goto out; -> will return but didn't calculate 'reserved' > ... > Step3: ext4_destroy_inode -> trigger "i_reserved_data_blocks (1) not cleared!" > > To solve above issue if 'len2>0' call 'get_rsvd()' before goto out. > > Reported-by: syzbot+05a0f0ccab4a25626e38@...kaller.appspotmail.com > Signed-off-by: Ye Bin <yebin10@...wei.com> > --- > fs/ext4/extents_status.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c > index cd0a861853e3..4684eaea9471 100644 > --- a/fs/ext4/extents_status.c > +++ b/fs/ext4/extents_status.c > @@ -1371,7 +1371,7 @@ static int __es_remove_extent(struct inode *inode, ext4_lblk_t lblk, > if (count_reserved) > count_rsvd(inode, lblk, orig_es.es_len - len1 - len2, > &orig_es, &rc); > - goto out; > + goto count; > } > > if (len1 > 0) { > @@ -1413,6 +1413,7 @@ static int __es_remove_extent(struct inode *inode, ext4_lblk_t lblk, > } > } > > +count: > if (count_reserved) > *reserved = get_rsvd(inode, end, es, &rc); > out: > -- > 2.31.1 > I'm unable to find the sysbot report for this patch, so I can't verify that this fix works. The more serious problem would be whatever is causing the invalid block bitmap and delayed allocation failure messages before the i_reserved_data_blocks message. Perhaps that's simply what syzkaller set up, but it's not clear from this posting. Have you looked for the cause of those first two messages? However, by inspection this patch should fix an obvious bug causing that last message, introduced by 8fcc3a580651 ("ext4: rework reserved cluster accounting when invalidating pages"). A Fixes tag should be added to the patch. Also, the readability of the code should be improved by changing the label "count" to the more descriptive "out_get_reserved". With those two changes, feel free to add: Reviewed-by: Eric Whitney <enwlinux@...il.com> Eric
Powered by blists - more mailing lists