linux-kernel - Re: [PATCH] mm: Add WARN_ON for possibility of infinite loop if empty lists in free_pcppages

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: <529861221.2662345.1471942120576.JavaMail.zimbra@redhat.com>
Date:   Tue, 23 Aug 2016 04:48:40 -0400 (EDT)
From:   Pankaj Gupta <pagupta@...hat.com>
To:     Mel Gorman <mgorman@...hsingularity.net>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        akpm@...ux-foundation.org, vbabka@...e.cz, riel@...hat.com,
        hannes@...xchg.org, iamjoonsoo kim <iamjoonsoo.kim@....com>,
        kirill shutemov <kirill.shutemov@...ux.intel.com>,
        izumi taku <izumi.taku@...fujitsu.com>
Subject: Re: [PATCH] mm: Add WARN_ON for possibility of infinite loop if
 empty lists in free_pcppages_bulk'


> > > > While debugging issue in realtime kernel i found a scenario
> > > > which resulted in infinite loop resulting because of empty pcp->lists
> > > > and valid 'to_free' value. This patch is to add 'WARN_ON' in function
> > > > 'free_pcppages_bulk' if there is possibility of infinite loop because
> > > > of any bug in code.
> > > > 
> > > 
> > > What was the bug that allowed this situation to occur? It would imply
> > > the pcp count was somehow out of sync.
> > 
> > Yes pcp count was out of sync. It was a bug in the downstream code.
> 
> If the bug is not in the mainline code, I think it would be inappropriate
> to add unnecessary code to a relatively hot path. At most, it should be
> a VM_BUG_ON but the soft lockup should be clear enough.

yes 'VM_BUG_ON' is right thing here. This could help in realtime kernel where
'free_pcppages_bulk' is divided into two functions 'isolate_pcp_pages' and 
'free_pcppages_bulk' where 'isolate_pcp_pages' isolate the 'batch/count' number
of pages and 'free_pcppages_bulk' just free these pages.

I was just thinking if there is any possibility of out of sync with count and temporary
list this might help. But looking more at the code does not seems like there is any 
chance until any other potential bug somewhere else in code result this scenario.

I will drop this patch.

Thanks for the review.

> 
> --
> Mel Gorman
> SUSE Labs
>