From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751641AbdF0Otf (ORCPT <rfc822;w@1wt.eu>);
        Tue, 27 Jun 2017 10:49:35 -0400
Received: from mga09.intel.com ([134.134.136.24]:37045 "EHLO mga09.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1750934AbdF0Ot2 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 27 Jun 2017 10:49:28 -0400
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.39,401,1493708400"; 
   d="scan'208";a="1145276638"
Date: Tue, 27 Jun 2017 07:49:18 -0700
From: Andi Kleen <ak@linux.intel.com>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Kees Cook <keescook@chromium.org>, Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Hellstrom <thellstrom@vmware.com>,
        Daniel Micay <danielmicay@gmail.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kref: Avoid null pointer dereference after WARN
Message-ID: <20170627144918.GG23705@tassilo.jf.intel.com>
References: <20170627035215.GA132342@beast>
 <20170627070626.GH29909@kroah.com>
 <20170627110013.GA3026@zx2c4.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170627110013.GA3026@zx2c4.com>
User-Agent: Mutt/1.8.0 (2017-02-23)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Jun 27, 2017 at 01:00:16PM +0200, Jason A. Donenfeld wrote:
> On Tue, Jun 27, 2017 at 09:06:26AM +0200, Greg Kroah-Hartman wrote:
> > I remember one complaint was that WARN_ON was "huge" and this bloated
> > the kernel code a lot.  But then that got fixed up.  Is BUG_ON going to
> > cause the same complaint again?
> 
> Complaint or not, I'm pretty sure using BUG_ON here is the right
> behavior. If removing it will result in a null pointer dereference (and
> subsequent function call), that's bad news bears, especially on systems
> with a zero mmap_min_addr or combined with other bugs.
> 
> If somehow a driver manages to pass a NULL as the release function,
> something is really messed up and the kernel should "safely" panic
> instead.

Please post data on the kernel text change from this patch.

In general I'm sceptical on the value of changes like this. Why
is release so important to check compared to other places?

Is there any data how many security holes this would have
caught? Please no hand waving. A lot of the recent
security patches seem to have gone in with just a lot of
hand waving and security theater 

("It's in gr-security, so it must be great")

In general accountability is important, especially in security

-Andi