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: <DB7PR04MB4490859B718948F4AABCEBE788720@DB7PR04MB4490.eurprd04.prod.outlook.com>
Date:   Tue, 5 Mar 2019 01:31:43 +0000
From:   Peng Fan <peng.fan@....com>
To:     "dennis@...nel.org" <dennis@...nel.org>
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



> -----Original Message-----
> From: dennis@...nel.org [mailto:dennis@...nel.org]
> Sent: 2019年3月5日 2:57
> To: Peng Fan <peng.fan@....com>
> Cc: tj@...nel.org; cl@...ux.com; linux-mm@...ck.org;
> linux-kernel@...r.kernel.org; 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://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> hwork.kernel.org%2Fcover%2F10832459%2F&amp;data=02%7C01%7Cpeng.f
> an%40nxp.com%7C6546dfcc85f0492d7c7508d6a0d33076%7C686ea1d3bc2b
> 4c6fa92cd99c5c301635%7C0%7C0%7C636873226241185534&amp;sdata=9
> azIw8vXJ8eqqd0T0znmEN6jR2cWhFghKBfg0zIJMDM%3D&amp;reserved=0
> 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.

I missed to consider the case the when contig_hint_start matches
right_free area, the right_free area will be take into consideration
into next loop.

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

I'll be careful for future patches.

Thanks,
Peng.

> 
> Thanks,
> Dennis