From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751653AbbLUWYl (ORCPT ); Mon, 21 Dec 2015 17:24:41 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:38451 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751201AbbLUWYk (ORCPT ); Mon, 21 Dec 2015 17:24:40 -0500 Date: Mon, 21 Dec 2015 14:24:38 -0800 From: Andrew Morton To: Sasha Levin Cc: "Kirill A. Shutemov" , mhocko@suse.com, mgorman@suse.de, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm, oom: initiallize all new zap_details fields before use Message-Id: <20151221142438.cbd34f0e663a795e649cdfbc@linux-foundation.org> In-Reply-To: <5675D423.6020806@oracle.com> References: <1450487091-7822-1-git-send-email-sasha.levin@oracle.com> <20151219195237.GA31380@node.shutemov.name> <5675D423.6020806@oracle.com> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 19 Dec 2015 17:03:15 -0500 Sasha Levin wrote: > On 12/19/2015 02:52 PM, Kirill A. Shutemov wrote: > > On Fri, Dec 18, 2015 at 08:04:51PM -0500, Sasha Levin wrote: > >> > Commit "mm, oom: introduce oom reaper" forgot to initialize the two new fields > >> > of struct zap_details in unmap_mapping_range(). This caused using stack garbage > >> > on the call to unmap_mapping_range_tree(). > >> > > >> > Signed-off-by: Sasha Levin > >> > --- > >> > mm/memory.c | 1 + > >> > 1 file changed, 1 insertion(+) > >> > > >> > diff --git a/mm/memory.c b/mm/memory.c > >> > index 206c8cd..0e32993 100644 > >> > --- a/mm/memory.c > >> > +++ b/mm/memory.c > >> > @@ -2431,6 +2431,7 @@ void unmap_mapping_range(struct address_space *mapping, > >> > details.last_index = hba + hlen - 1; > >> > if (details.last_index < details.first_index) > >> > details.last_index = ULONG_MAX; > >> > + details.check_swap_entries = details.ignore_dirty = false; > > Should we use c99 initializer instead to make it future-proof? > > I didn't do that to make these sort of failures obvious. In this case, if we would have > used an initializer and it would default to the "wrong" values it would be much harder > to find this bug. > If we're to make that approach useful and debuggable we should poison the structure at the outset with some well-known and crazy pattern. Or use kasan. But I don't think we need any special treatment here so yes, the conventional way of zapping everything is best, IMO. --- a/mm/memory.c~mm-oom-introduce-oom-reaper-fix-5-fix +++ a/mm/memory.c @@ -2414,7 +2414,7 @@ static inline void unmap_mapping_range_t void unmap_mapping_range(struct address_space *mapping, loff_t const holebegin, loff_t const holelen, int even_cows) { - struct zap_details details; + struct zap_details details = { }; pgoff_t hba = holebegin >> PAGE_SHIFT; pgoff_t hlen = (holelen + PAGE_SIZE - 1) >> PAGE_SHIFT;