netdev - Re: net: wwan: t7xx: fix potential skb->frags overflow in RX path

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] [day] [month] [year] [list]

Message-ID: <20260125223443.3404985-1-kuba@kernel.org>
Date: Sun, 25 Jan 2026 14:34:43 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: qikeyu2017@...il.com
Cc: Jakub Kicinski <kuba@...nel.org>,
	johannes@...solutions.net,
	pabeni@...hat.com,
	chandrashekar.devegowda@...el.com,
	edumazet@...gle.com,
	loic.poulain@....qualcomm.com,
	ryazanov.s.a@...il.com,
	haijun.liu@...iatek.com,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	andrew+netdev@...n.ch,
	ricardo.martinez@...ux.intel.com,
	chiranjeevi.rapolu@...ux.intel.com,
	davem@...emloft.net
Subject: Re: net: wwan: t7xx: fix potential skb->frags overflow in RX path

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: wwan: t7xx: fix potential skb->frags overflow in RX path

This patch adds a bounds check to prevent skb->frags[] array overflow
when the modem firmware sends packets with excessive fragments. The fix
adds a check for nr_frags >= MAX_SKB_FRAGS before calling skb_add_rx_frag().

> diff --git a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c
> index b76bea6ab2d7..b041e6f48732 100644
> --- a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c
> +++ b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c
> @@ -395,6 +395,7 @@ static int t7xx_dpmaif_set_frag_to_skb(const struct dpmaif_rx_queue *rxq,
>  				       struct sk_buff *skb)
>  {
>  	unsigned long long data_bus_addr, data_base_addr;
> +	struct skb_shared_info *shinfo = skb_shinfo(skb);
>  	struct device *dev = rxq->dpmaif_ctrl->dev;
>  	struct dpmaif_bat_page *page_info;
>  	unsigned int data_len;
> @@ -407,13 +408,16 @@ static int t7xx_dpmaif_set_frag_to_skb(const struct dpmaif_rx_queue *rxq,
>  	if (!page_info->page)
>  		return -EINVAL;
>
> +	if (shinfo->nr_frags >= MAX_SKB_FRAGS)
> +		return -EINVAL;

When this new error path is taken, it appears that page_info->page has
already had its DMA mapping removed via dma_unmap_page() earlier in the
function, but the page itself is not freed (put_page is not called) and
page_info->page is not set to NULL.

Could this cause a page leak? On this error path:
- dma_unmap_page() has already been called
- page_info->page still holds a reference to the page
- t7xx_dpmaif_set_bat_mask() is not called in the caller t7xx_dpmaif_get_frag()

When the device is eventually torn down, t7xx_dpmaif_bat_free() will
iterate all entries and call t7xx_unmap_bat_page(), which will call
dma_unmap_page() again on the same (now stale) data_bus_addr. Is there a
concern about calling dma_unmap_page() twice for the same mapping?

It looks like the page cleanup on this error path should either:
- Move the new bounds check before dma_unmap_page(), or
- Add put_page() and set page_info->page = NULL on the error path

> +
>  	data_bus_addr = le32_to_cpu(pkt_info->pd.data_addr_h);

[ ... ]
-- 
pw-bot: cr