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@xxxxxxxxxxxxxxxxxx>
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;
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
