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
| ||
|
Message-ID: <be98552a061f6249de558b210ff25de45e80d690.camel@gmail.com> Date: Fri, 16 Dec 2022 07:34:21 -0800 From: Alexander H Duyck <alexander.duyck@...il.com> To: wei.fang@....com, davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, xiaoning.wang@....com, shenwei.wang@....com, linux-imx@....com Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH net] net: fec: Coverity issue: Dereference null return value On Thu, 2022-12-15 at 17:11 +0800, wei.fang@....com wrote: > From: Wei Fang <wei.fang@....com> > > The build_skb might return a null pointer but there is no check on the > return value in the fec_enet_rx_queue(). So a null pointer dereference > might occur. To avoid this, we check the return value of build_skb. If > the return value is a null pointer, the driver will recycle the page and > update the statistic of ndev. Then jump to rx_processing_done to clear > the status flags of the BD so that the hardware can recycle the BD. > > Signed-off-by: Wei Fang <wei.fang@....com> > Reviewed-by: Shenwei Wang <Shenwei.wang@....com> > --- > drivers/net/ethernet/freescale/fec_main.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c > index 5528b0af82ae..c78aaa780983 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -1674,6 +1674,16 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id) > * bridging applications. > */ > skb = build_skb(page_address(page), PAGE_SIZE); > + if (unlikely(!skb)) { > + page_pool_recycle_direct(rxq->page_pool, page); > + ndev->stats.rx_packets--; > + ndev->stats.rx_bytes -= pkt_len; > + ndev->stats.rx_dropped++; I'm not sure you really need to bother with rewinding the rx_packets and rx_bytes counters. I know that the rx_dropped statistic will get incremented in the network stack in the event of a packet failing to enqueue to the backlog, so it might be better to just leave the rx_packets counter as is and assume the actual packet count is rx_packets - rx_dropped. > + > + netdev_err(ndev, "build_skb failed!\n"); Instead of netdev_err you may want to consider netdev_err_once for this. Generally speaking when we start seeing memory allocation error issues they can get very noisy very quickly as you are likely to fail the allocation for every packet in a given polling session, and sessions to follow. > + goto rx_processing_done; > + } > + > skb_reserve(skb, data_start); > skb_put(skb, pkt_len - sub_len); > skb_mark_for_recycle(skb);
Powered by blists - more mailing lists