linux-kernel - Re: [PATCH 2/2] percpu: pcpu_next_md_free_region: inclusive check for PCPU_BITMAP_BLOCK

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190304185657.GA17970@dennisz-mbp.dhcp.thefacebook.com>
Date:   Mon, 4 Mar 2019 13:56:57 -0500
From:   "dennis@...nel.org" <dennis@...nel.org>
To:     Peng Fan <peng.fan@....com>
Cc:     "tj@...nel.org" <tj@...nel.org>, "cl@...ux.com" <cl@...ux.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "van.freenix@...il.com" <van.freenix@...il.com>
Subject: Re: [PATCH 2/2] percpu: pcpu_next_md_free_region: inclusive check
 for PCPU_BITMAP_BLOCK_BITS

Hi Peng,

On Mon, Mar 04, 2019 at 10:33:55AM +0000, Peng Fan wrote:
> If the block [contig_hint_start, contig_hint_start + contig_hint)
> matches block->right_free area, need use "<=", not "<".
> 
> Signed-off-by: Peng Fan <peng.fan@....com>
> ---
> 
> V1:
>   Based on https://patchwork.kernel.org/cover/10832459/ applied linux-next
>   boot test on qemu aarch64
> 
>  mm/percpu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 5ee90fc34ea3..0f91f1d883c6 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -390,7 +390,8 @@ static void pcpu_next_md_free_region(struct pcpu_chunk *chunk, int *bit_off,
>  		 */
>  		*bits = block->contig_hint;
>  		if (*bits && block->contig_hint_start >= block_off &&
> -		    *bits + block->contig_hint_start < PCPU_BITMAP_BLOCK_BITS) {
> +		    *bits + block->contig_hint_start <=
> +		    PCPU_BITMAP_BLOCK_BITS) {
>  			*bit_off = pcpu_block_off_to_off(i,
>  					block->contig_hint_start);
>  			return;
> -- 
> 2.16.4
> 

This is wrong. This iterator is for updating contig hints and not for
finding fit.

Have you tried reproducing and proving the issue you are seeing? In
general, making changes to percpu carries a lot of risk. I really only
want to be taking code that is provably solving a problem and not
supported by just code inspection. Boot testing for a change like this
is really not enough as we need to be sure changes like these are
correct.

Thanks,
Dennis