| Message ID | Y4GFPajUjIBOa5i2@ZenIV (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | unbugger ext2_empty_dir() | expand |
On Sat 26-11-22 03:17:17, Al Viro wrote: > In 27cfa258951a "ext2: fix fs corruption when trying to remove > a non-empty directory with IO error" a funny thing has happened: > > - page = ext2_get_page(inode, i, dir_has_error, &page_addr); > + page = ext2_get_page(inode, i, 0, &page_addr); > > - if (IS_ERR(page)) { > - dir_has_error = 1; > - continue; > - } > + if (IS_ERR(page)) > + goto not_empty; > > And at not_empty: we hit ext2_put_page(page, page_addr), which does > put_page(page). Which, unless I'm very mistaken, should oops > immediately when given ERR_PTR(-E...) as page. > > OK, shit happens, insufficiently tested patches included. But when > commit in question describes the fault-injection test that exercised > that particular failure exit... > > Ow. > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Right. I've clearly missed this when reviewing & merging the fix. And Ye Bin obviously didn't test this with his reproducer ;). Anyway, thanks for catching this! I've merged the patch to my tree. Honza > --- > diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c > index 8f597753ac12..5202eddfc3c0 100644 > --- a/fs/ext2/dir.c > +++ b/fs/ext2/dir.c > @@ -679,7 +679,7 @@ int ext2_empty_dir (struct inode * inode) > page = ext2_get_page(inode, i, 0, &page_addr); > > if (IS_ERR(page)) > - goto not_empty; > + return 0; > > kaddr = page_addr; > de = (ext2_dirent *)kaddr;
diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c index 8f597753ac12..5202eddfc3c0 100644 --- a/fs/ext2/dir.c +++ b/fs/ext2/dir.c @@ -679,7 +679,7 @@ int ext2_empty_dir (struct inode * inode) page = ext2_get_page(inode, i, 0, &page_addr); if (IS_ERR(page)) - goto not_empty; + return 0; kaddr = page_addr; de = (ext2_dirent *)kaddr;
In 27cfa258951a "ext2: fix fs corruption when trying to remove a non-empty directory with IO error" a funny thing has happened: - page = ext2_get_page(inode, i, dir_has_error, &page_addr); + page = ext2_get_page(inode, i, 0, &page_addr); - if (IS_ERR(page)) { - dir_has_error = 1; - continue; - } + if (IS_ERR(page)) + goto not_empty; And at not_empty: we hit ext2_put_page(page, page_addr), which does put_page(page). Which, unless I'm very mistaken, should oops immediately when given ERR_PTR(-E...) as page. OK, shit happens, insufficiently tested patches included. But when commit in question describes the fault-injection test that exercised that particular failure exit... Ow. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> ---