[PATCH] net: wwan: t7xx: check skb_clone in control TX

[PATCH] net: wwan: t7xx: check skb_clone in control TX

[Ruoyu Wang]

t7xx_port_ctrl_tx() clones each skb fragment before passing it to the
port transmit path. The clone is used immediately to set cloned->len, so
an skb_clone() failure results in a NULL pointer dereference.

Check the clone before using it. If previous fragments were already
queued, preserve the driver's existing partial-write behavior by
returning the number of bytes submitted so far. Keep the existing byte
accounting unchanged so this patch only handles the allocation failure.

Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/net/wwan/t7xx/t7xx_port_wwan.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wwan/t7xx/t7xx_port_wwan.c b/drivers/net/wwan/t7xx/t7xx_port_wwan.c
index 7fc569565..d2529df75 100644
--- a/drivers/net/wwan/t7xx/t7xx_port_wwan.c
+++ b/drivers/net/wwan/t7xx/t7xx_port_wwan.c
@@ -106,6 +106,8 @@ static int t7xx_port_ctrl_tx(struct t7xx_port *port, struct sk_buff *skb)
 
 	while (cur) {
 		cloned = skb_clone(cur, GFP_KERNEL);
+		if (!cloned)
+			return cnt ? cnt : -ENOMEM;
 		cloned->len = skb_headlen(cur);
 		ret = t7xx_port_send_skb(port, cloned, 0, 0);
 		if (ret) {
-- 
2.51.0

Re: [PATCH] net: wwan: t7xx: check skb_clone in control TX

[Loic Poulain]

On Sun, Jun 7, 2026 at 11:35 AM Ruoyu Wang <ruoyuw560@gmail.com> wrote:
>
> t7xx_port_ctrl_tx() clones each skb fragment before passing it to the
> port transmit path. The clone is used immediately to set cloned->len, so
> an skb_clone() failure results in a NULL pointer dereference.
>
> Check the clone before using it. If previous fragments were already
> queued, preserve the driver's existing partial-write behavior by
> returning the number of bytes submitted so far. Keep the existing byte
> accounting unchanged so this patch only handles the allocation failure.
>

Looks like it deserves a 'Fixes' tag.
https://www.kernel.org/doc/html/latest/process/submitting-patches.html

> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
>  drivers/net/wwan/t7xx/t7xx_port_wwan.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wwan/t7xx/t7xx_port_wwan.c b/drivers/net/wwan/t7xx/t7xx_port_wwan.c
> index 7fc569565..d2529df75 100644
> --- a/drivers/net/wwan/t7xx/t7xx_port_wwan.c
> +++ b/drivers/net/wwan/t7xx/t7xx_port_wwan.c
> @@ -106,6 +106,8 @@ static int t7xx_port_ctrl_tx(struct t7xx_port *port, struct sk_buff *skb)
>
>         while (cur) {
>                 cloned = skb_clone(cur, GFP_KERNEL);
> +               if (!cloned)
> +                       return cnt ? cnt : -ENOMEM;

This feels a bit inconsistent as the return value depends on where the
clone fails in the loop.
Wouldn’t it be clearer to either return 0 (indicating no submission)
or unconditionally return -ENOMEM on failure?

>                 cloned->len = skb_headlen(cur);
>                 ret = t7xx_port_send_skb(port, cloned, 0, 0);
>                 if (ret) {
> --
> 2.51.0
>