[PATCH] unbugger ext2_empty_dir()

[PATCH] unbugger ext2_empty_dir()

[Al Viro]

	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>
---
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;

Re: [PATCH] unbugger ext2_empty_dir()

[Jan Kara]

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;
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR