From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757434AbcAMK6B (ORCPT ); Wed, 13 Jan 2016 05:58:01 -0500 Received: from arrakis.dune.hu ([78.24.191.176]:34365 "EHLO arrakis.dune.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754548AbcAMK57 (ORCPT ); Wed, 13 Jan 2016 05:57:59 -0500 Subject: Re: bgmac: fix a missing check for build_skb To: Weidong Wang , Paolo Abeni References: <5695BF41.2040002@huawei.com> <1452674087.4516.3.camel@redhat.com> <5696262D.3040803@huawei.com> Cc: David Miller , zajec5@gmail.com, hauke@hauke-m.de, Joe Perches , peter.senna@gmail.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org From: Felix Fietkau Message-ID: <56962DAF.9040807@openwrt.org> Date: Wed, 13 Jan 2016 11:57:51 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <5696262D.3040803@huawei.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016-01-13 11:25, Weidong Wang wrote: > On 2016/1/13 16:34, Paolo Abeni wrote: >> On Wed, 2016-01-13 at 11:06 +0800, Weidong Wang wrote: >>> when build_skb failed, it may occure a NULL pointer. >>> So add a 'NULL check' for it. >>> >>> Signed-off-by: Weidong Wang >>> --- >>> drivers/net/ethernet/broadcom/bgmac.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c >>> index 21e3c38..d75180a 100644 >>> --- a/drivers/net/ethernet/broadcom/bgmac.c >>> +++ b/drivers/net/ethernet/broadcom/bgmac.c >>> @@ -466,6 +466,11 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring, >>> len -= ETH_FCS_LEN; >>> >>> skb = build_skb(buf, BGMAC_RX_ALLOC_SIZE); >>> + if (unlikely(skb)) { >> >> Should that be instead: >> >> if (unlikely(!skb)) { >> >> ? >> > > What to instead of it? Your patch has a logic error (missing the !), and it's breaking the ethernet driver completely instead of fixing anything. Did you even test this? Dave, why did you apply this patch so fast without any chance of review? - Felix