linux-kernel - Re: [PATCH] fs/mbcache: make count

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20180104155152.GC2170@quack2.suse.cz>
Date:   Thu, 4 Jan 2018 16:51:52 +0100
From:   Jan Kara <jack@...e.cz>
To:     Jiang Biao <jiang.biao2@....com.cn>
Cc:     viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, zhong.weidong@....com.cn
Subject: Re: [PATCH] fs/mbcache: make count_objects more robust.

On Mon 27-11-17 11:30:19, Jiang Biao wrote:
> When running ltp stress test for 7*24 hours, the vmscan occasionally
> complains the following warning continuously,
> 
> mb_cache_scan+0x0/0x3f0 negative objects to delete
> nr=-9232265467809300450
> ...
> 
> The tracing result shows the freeable(mb_cache_count returns)
> is -1, which causes the continuous accumulation and overflow of
> total_scan.
> 
> This patch make sure the mb_cache_count not return negative value,
> which make the mbcache shrinker more robust.
> 
> Signed-off-by: Jiang Biao <jiang.biao2@....com.cn>

Going through some old email... 

a) c_entry_count is unsigned so your patch is a nop as Coverity properly
noticed.

b) c_entry_count being outside 0..2*cache->c_max_entries is a plain bug. I
went through the logic and cannot find out how that could happen though.

But in either case your patch just does not make sense.

								Honza

> ---
>  fs/mbcache.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/mbcache.c b/fs/mbcache.c
> index d818fd2..b8b8b9c 100644
> --- a/fs/mbcache.c
> +++ b/fs/mbcache.c
> @@ -269,6 +269,9 @@ static unsigned long mb_cache_count(struct shrinker *shrink,
>  	struct mb_cache *cache = container_of(shrink, struct mb_cache,
>  					      c_shrink);
>  
> +	/* Unlikely, but not impossible */
> +	if (unlikely(cache->c_entry_count < 0))
> +		return 0;
>  	return cache->c_entry_count;
>  }
>  
> -- 
> 2.7.4
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR