From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934235AbcI2Qoh (ORCPT ); Thu, 29 Sep 2016 12:44:37 -0400 Received: from mail-yw0-f194.google.com ([209.85.161.194]:36038 "EHLO mail-yw0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932469AbcI2Qo2 (ORCPT ); Thu, 29 Sep 2016 12:44:28 -0400 Date: Thu, 29 Sep 2016 18:44:22 +0200 From: Tejun Heo To: zijun_hu Cc: Andrew Morton , zijun_hu@htc.com, cl@linux.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 1/1] mm/percpu.c: fix potential memory leakage for pcpu_embed_first_chunk() Message-ID: <20160929164422.GA3773@mtj.duckdns.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.0 (2016-08-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On Fri, Sep 30, 2016 at 12:03:20AM +0800, zijun_hu wrote: > From: zijun_hu > > it will cause memory leakage for pcpu_embed_first_chunk() to go to > label @out_free if the chunk spans over 3/4 VMALLOC area. all memory > are allocated and recorded into array @areas for each CPU group, but > the memory allocated aren't be freed before returning after going to > label @out_free > > in order to fix this bug, we check chunk spanned area immediately > after completing memory allocation for all CPU group, we go to label > @out_free_areas other than @out_free to free all memory allocated if > the checking is failed. > > Signed-off-by: zijun_hu ... > @@ -2000,6 +2001,21 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size, > areas[group] = ptr; > > base = min(ptr, base); > + if (ptr > areas[j]) > + j = group; > + } > + max_distance = areas[j] - base; > + max_distance += ai->unit_size * ai->groups[j].nr_units; > + > + /* warn if maximum distance is further than 75% of vmalloc space */ > + if (max_distance > VMALLOC_TOTAL * 3 / 4) { > + pr_warn("max_distance=0x%lx too large for vmalloc space 0x%lx\n", > + max_distance, VMALLOC_TOTAL); > +#ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK > + /* and fail if we have fallback */ > + rc = -EINVAL; > + goto out_free_areas; > +#endif Isn't it way simpler to make the error path jump to out_free_areas? There's another similar case after pcpu_setup_first_chunk() failure too. Also, can you please explain how you tested the changes? Thanks. -- tejun