From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ispman.iskranet.ru (ispman.iskranet.ru [62.213.33.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 9BF221A0E53 for ; Sat, 31 Jan 2015 15:19:35 +1100 (AEDT) Message-ID: <54CC579D.1070709@kb.kras.ru> Date: Sat, 31 Jan 2015 11:18:37 +0700 From: Arseny Solokha MIME-Version: 1.0 To: Scott Wood Subject: Re: [PATCH] powerpc/mm: bail out early when flushing TLB page References: <1422619707-30864-1-git-send-email-asolokha@kb.kras.ru> <1422654781.10544.173.camel@freescale.com> In-Reply-To: <1422654781.10544.173.camel@freescale.com> Content-Type: text/plain; charset=utf-8 Cc: Paul Mackerras , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , > On Fri, 2015-01-30 at 19:08 +0700, Arseny Solokha wrote: >> MMU_NO_CONTEXT is conditionally defined as 0 or (unsigned int)-1. > > For nohash it is specifically -1. >> However, in __flush_tlb_page() a corresponding variable is only tested >> for open coded 0, which can cause NULL pointer dereference if `mm' >> argument was legitimately passed as such. >> >> Bail out early in case the first argument is NULL, thus eliminate confusion >> between different values of MMU_NO_CONTEXT and avoid disabling and then >> re-enabling preemption unnecessarily. > > How did you notice this? Did you see an oops, or was it code > inspection? I'm wondering what codepath gets here with mm == NULL. Just a code inspection. It didn't seemed right at the first glance. Arsény > > -Scott From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760225AbbAaETi (ORCPT ); Fri, 30 Jan 2015 23:19:38 -0500 Received: from ispman.iskranet.ru ([62.213.33.10]:47663 "EHLO ispman.iskranet.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756520AbbAaETg (ORCPT ); Fri, 30 Jan 2015 23:19:36 -0500 Message-ID: <54CC579D.1070709@kb.kras.ru> Date: Sat, 31 Jan 2015 11:18:37 +0700 From: Arseny Solokha User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Scott Wood CC: Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] powerpc/mm: bail out early when flushing TLB page References: <1422619707-30864-1-git-send-email-asolokha@kb.kras.ru> <1422654781.10544.173.camel@freescale.com> In-Reply-To: <1422654781.10544.173.camel@freescale.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On Fri, 2015-01-30 at 19:08 +0700, Arseny Solokha wrote: >> MMU_NO_CONTEXT is conditionally defined as 0 or (unsigned int)-1. > > For nohash it is specifically -1. >> However, in __flush_tlb_page() a corresponding variable is only tested >> for open coded 0, which can cause NULL pointer dereference if `mm' >> argument was legitimately passed as such. >> >> Bail out early in case the first argument is NULL, thus eliminate confusion >> between different values of MMU_NO_CONTEXT and avoid disabling and then >> re-enabling preemption unnecessarily. > > How did you notice this? Did you see an oops, or was it code > inspection? I'm wondering what codepath gets here with mm == NULL. Just a code inspection. It didn't seemed right at the first glance. Arsény > > -Scott