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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 4 Aug 2021 15:35:29 +0200
From:   Jan Kara <jack@...e.cz>
To:     yangerkun <yangerkun@...wei.com>
Cc:     tytso@....edu, jack@...e.cz, adilger.kernel@...ger.ca,
        linux-ext4@...r.kernel.org, yukuai3@...wei.com
Subject: Re: [PATCH] ext4: stop return ENOSPC from ext4_issue_zeroout

On Wed 04-08-21 20:50:44, yangerkun wrote:
> Our testcase(briefly described as fsstress on dm thin-provisioning which
> ext4 see volume size with 100G but actual size 10G) trigger a hungtask
> bug since ext4_writepages fall into a infinite loop:
> 
> static int ext4_writepages(xxx)
> {
>     ...
>    while (!done && mpd.first_page <= mpd.last_page) {
>        ...
>        ret = mpage_prepare_extent_to_map(&mpd);
>        if (!ret) {
>            ...
>            ret = mpage_map_and_submit_extent(handle,
> &mpd,&give_up_on_write);
>            <----- will return -ENOSPC
>            ...
>        }
>        ...
>        if (ret == -ENOSPC && sbi->s_journal) {
>            <------ we cannot break since we will get ENOSPC forever
>            jbd2_journal_force_commit_nested(sbi->s_journal);
>            ret = 0;
>            continue;
>        }
>        ...
>    }
> }
> 
> Got ENOSPC with follow stack:
> ...
> ext4_ext_map_blocks
>   ext4_ext_convert_to_initialized
>     ext4_ext_zeroout
>       ext4_issue_zeroout
>         ...
>         submit_bio_wait <-- bio to thinpool will return ENOSPC
> 
> 'df22291ff0fd ("ext4: Retry block allocation if we have free blocks
> left")' add the logic to retry block allcate since we may get free block
> after we commit a transaction. But the ENOSPC from thin-provisioning
> will confuse ext4, and lead the upper infinite loop. Fix it by convert
> the err to EIO.
> 
> Signed-off-by: yangerkun <yangerkun@...wei.com>

Thanks for the patch. As a quick fix for the problem this is probably fine.
But longer term we might need to implement a configurable behavior for this
because just dropping data on the floor (which is what would happen here)
need not be what sysadmin wants and blocking until space is provisioned may be
actually a preferable behavior. Anyway for now feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

									Honza

> ---
>  fs/ext4/inode.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 038aebd7eb2f..d9ded699a88c 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -428,6 +428,9 @@ int ext4_issue_zeroout(struct inode *inode, ext4_lblk_t lblk, ext4_fsblk_t pblk,
>  	if (ret > 0)
>  		ret = 0;
>  
> +	if (ret == -ENOSPC)
> +		ret = -EIO;
> +
>  	return ret;
>  }
>  
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

Powered by blists - more mailing lists