netdev - Re: bgmac: fix a missing check for build

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56962DAF.9040807@openwrt.org>
Date:	Wed, 13 Jan 2016 11:57:51 +0100
From:	Felix Fietkau <nbd@...nwrt.org>
To:	Weidong Wang <wangweidong1@...wei.com>,
	Paolo Abeni <pabeni@...hat.com>
Cc:	David Miller <davem@...emloft.net>, zajec5@...il.com,
	hauke@...ke-m.de, Joe Perches <joe@...ches.com>,
	peter.senna@...il.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: bgmac: fix a missing check for build_skb

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 <wangweidong1@...wei.com>
>>> ---
>>>  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