Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752288AbdFUXE7 (ORCPT ); Wed, 21 Jun 2017 19:04:59 -0400 Received: from mail-wr0-f196.google.com ([209.85.128.196]:35508 "EHLO mail-wr0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752066AbdFUXE5 (ORCPT ); Wed, 21 Jun 2017 19:04:57 -0400 Subject: Re: [PATCH] Convert BUG_ON to WARN_ON in bond_options.c To: Jay Vosburgh Cc: David Miller , vfalico@gmail.com, andy@greyhouse.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, joe@perches.com References: <20170621.173655.1945994342723484710.davem@davemloft.net> <20170621.175651.854625612625047729.davem@davemloft.net> <125b4ae9-2cb7-3532-5391-24404cf6eaec@gmail.com> <24691.1498084768@famine> From: Michael J Dilmore Message-ID: <6dd7cfa3-9d3f-ed54-3278-af37e9d0f266@gmail.com> Date: Thu, 22 Jun 2017 00:04:54 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <24691.1498084768@famine> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1726 Lines: 45 On 21/06/17 23:39, Jay Vosburgh wrote: > Michael J Dilmore wrote: > >> On 21/06/17 22:56, David Miller wrote: >> >>> From: Michael D >>> Date: Wed, 21 Jun 2017 22:41:07 +0100 >>> >>>> I don't think you can stop it being dereferenced... you just need to >>>> prevent an attacker from exploiting the null pointer dereference >>>> vulnerability right? And this is done by returning the function right >>>> away? >>> What's all of this about an "attacker"? >>> >>> If there is a bug, we dererence a NULL pointer, and we should >>> fix that bug. >>> >>> The BUG_ON() helps us see where the problem is while at the >>> same time stopping the kernel before the NULL deref happens. >> Ok this is starting to make sense now - went a bit off track but think my >> general thinking is ok - i.e. if we return the function with an error code >> before the dereference then this basically does the same thing as BUG_ON >> but without crashing the kernel. >> >> Something like: >> >> if (WARN_ON(!new_active_slave) { >> netdev_dbg("Can't add new active slave - pointer null"); >> return ERROR_CODE >> } > In general, yes, but in this case, the condition should be > impossible to hit, so BUG_ON seems appropriate. > > If bond_slave_get_rtnl/rcu() returns NULL for an actual bonding > slave, other code paths (bond_fill_slave_info, bond_handle_frame) will > likely crash before getting to this one. > > -J > > --- > -Jay Vosburgh, jay.vosburgh@canonical.com That did cross my mind but I read that Linus that was quite averse to BUG_ON anywhere in the kernel so thought it might be have been worth doing. Is it worth at least wrapping BUG_ON in an unlikely macro then?