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
| ||
|
Date: Fri, 18 Nov 2022 19:53:51 +0800 From: Zhang Changzhong <zhangchangzhong@...wei.com> To: Leon Romanovsky <leon@...nel.org>, Edward Cree <ecree.xilinx@...il.com>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org> Subject: Re: [PATCH net] sfc: fix potential memleak in __ef100_hard_start_xmit() On 2022/11/18 17:15, Martin Habets wrote: > On Thu, Nov 17, 2022 at 03:05:27PM +0200, Leon Romanovsky wrote: >> On Thu, Nov 17, 2022 at 08:41:52PM +0800, Zhang Changzhong wrote: >>> >>> >>> On 2022/11/17 19:36, Leon Romanovsky wrote: >>>> On Thu, Nov 17, 2022 at 03:50:09PM +0800, Zhang Changzhong wrote: >>>>> The __ef100_hard_start_xmit() returns NETDEV_TX_OK without freeing skb >>>>> in error handling case, add dev_kfree_skb_any() to fix it. >>>>> >>>>> Fixes: 51b35a454efd ("sfc: skeleton EF100 PF driver") >>>>> Signed-off-by: Zhang Changzhong <zhangchangzhong@...wei.com> >>>>> --- >>>>> drivers/net/ethernet/sfc/ef100_netdev.c | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c >>>>> index 88fa295..ddcc325 100644 >>>>> --- a/drivers/net/ethernet/sfc/ef100_netdev.c >>>>> +++ b/drivers/net/ethernet/sfc/ef100_netdev.c >>>>> @@ -218,6 +218,7 @@ netdev_tx_t __ef100_hard_start_xmit(struct sk_buff *skb, >>>>> skb->len, skb->data_len, channel->channel); >>>>> if (!efx->n_channels || !efx->n_tx_channels || !channel) { >>>>> netif_stop_queue(net_dev); >>>>> + dev_kfree_skb_any(skb); >>>>> goto err; >>>>> } >>>> >>>> ef100 doesn't release in __ef100_enqueue_skb() either. SKB shouldn't be >>>> NULL or ERR at this stage. >>> >>> SKB shouldn't be NULL or ERR, so it can be freed. But this code looks weird. >> >> Please take a look __ef100_enqueue_skb() and see if it frees SKB on >> error or not. If not, please fix it. > > That function looks ok to me, but I appreciate the extra eyes on it. > I've looked at this function and it seems every error handling path frees the skb. Thanks, Changzhong > Martin > >> Thanks >> >>> >>>> >>>> diff --git a/drivers/net/ethernet/sfc/ef100_tx.c b/drivers/net/ethernet/sfc/ef100_tx.c >>>> index 29ffaf35559d..426706b91d02 100644 >>>> --- a/drivers/net/ethernet/sfc/ef100_tx.c >>>> +++ b/drivers/net/ethernet/sfc/ef100_tx.c >>>> @@ -497,7 +497,7 @@ int __ef100_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb, >>>> >>>> err: >>>> efx_enqueue_unwind(tx_queue, old_insert_count); >>>> - if (!IS_ERR_OR_NULL(skb)) >>>> + if (rc) >>>> dev_kfree_skb_any(skb); >>>> >>>> /* If we're not expecting another transmit and we had something to push >>>> >>>> >>>>> >>>>> -- >>>>> 2.9.5 >>>>> >>>> . >>>> > . >
Powered by blists - more mailing lists