linux-kernel - Re: [PATCH] fs: Fix a missing check bug

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ada75c60-aefb-86ff-0f3a-33825fb4c958@gmx.com>
Date:   Sun, 8 Dec 2019 13:02:11 +0800
From:   Qu Wenruo <quwenruo.btrfs@....com>
To:     Dinghao Liu <dinghao.liu@....edu.cn>, kjlu@....edu
Cc:     pakki001@....edu, Chris Mason <clm@...com>,
        Josef Bacik <josef@...icpanda.com>,
        David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fs: Fix a missing check bug



On 2019/12/7 下午10:41, Dinghao Liu wrote:
> The return value of link_free_space(ctl, info) is checked out-sync. Only one branch of an if statement checks this return value after WARN_ON(ret).
> 
> Since this path pair is similar in semantic, there might be a missing check bug.
> 
> Fix this by simply adding a check on ret.

The main failure mode for link_free_space() is -EEXIST, which means
there is already free space in the cache.

Here EEXIST may not be a big problem, and we may really want to continue
the iteration other than error out.


Would you explain in details about why you believe error out is the
correct way other than current continue behavior?

Thanks,
Qu

> 
> Signed-off-by: Dinghao Liu <dinghao.liu@....edu.cn>
> ---
>  fs/btrfs/free-space-cache.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 3283da419200..acbb3a59d344 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -2437,6 +2437,8 @@ int btrfs_remove_free_space(struct btrfs_block_group *block_group,
>  			if (info->bytes) {
>  				ret = link_free_space(ctl, info);
>  				WARN_ON(ret);
> +				if (ret)
> +					goto out_lock;
>  			} else {
>  				kmem_cache_free(btrfs_free_space_cachep, info);
>  			}
> 



Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)