[PATCH net] net: wan: fsl_ucc_hdlc: free tx_skbuff in uhdlc_memclean

[PATCH net] net: wan: fsl_ucc_hdlc: free tx_skbuff in uhdlc_memclean

[Holger Brunck]

When cleaning up the resources we need to iterate over the
tx_skbuf array to free pending TX messages.

Fixes: c19b6d246a35 ("drivers/net: support hdlc function for QE-UCC")
Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index adf3863463f5..68f78aeabdc3 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -739,6 +739,8 @@ static int uhdlc_open(struct net_device *dev)
 
 static void uhdlc_memclean(struct ucc_hdlc_private *priv)
 {
+	int i;
+
 	qe_muram_free(ioread16be(&priv->ucc_pram->riptr));
 	qe_muram_free(ioread16be(&priv->ucc_pram->tiptr));
 
@@ -769,6 +771,11 @@ static void uhdlc_memclean(struct ucc_hdlc_private *priv)
 	kfree(priv->rx_skbuff);
 	priv->rx_skbuff = NULL;
 
+	for (i = 0; i < TX_BD_RING_LEN) {
+		kfree(priv->tx_skbuff[i]);
+		priv->tx_skbuff[i] = NULL;
+	}
+
 	kfree(priv->tx_skbuff);
 	priv->tx_skbuff = NULL;
 
-- 
2.47.3

Re: [PATCH net] net: wan: fsl_ucc_hdlc: free tx_skbuff in uhdlc_memclean

[Christophe Leroy (CS GROUP)]

Le 04/05/2026 à 18:11, Holger Brunck a écrit :
> When cleaning up the resources we need to iterate over the
> tx_skbuf array to free pending TX messages.
> 
> Fixes: c19b6d246a35 ("drivers/net: support hdlc function for QE-UCC")
> Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>
> ---
>   drivers/net/wan/fsl_ucc_hdlc.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
> index adf3863463f5..68f78aeabdc3 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.c
> +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> @@ -739,6 +739,8 @@ static int uhdlc_open(struct net_device *dev)
>   
>   static void uhdlc_memclean(struct ucc_hdlc_private *priv)
>   {
> +	int i;
> +
>   	qe_muram_free(ioread16be(&priv->ucc_pram->riptr));
>   	qe_muram_free(ioread16be(&priv->ucc_pram->tiptr));
>   
> @@ -769,6 +771,11 @@ static void uhdlc_memclean(struct ucc_hdlc_private *priv)
>   	kfree(priv->rx_skbuff);
>   	priv->rx_skbuff = NULL;
>   
> +	for (i = 0; i < TX_BD_RING_LEN) {
> +		kfree(priv->tx_skbuff[i]);

I don't think you can just kfree() an skb like this.

I think you have to call dev_kfree_skb_any() instead.

Christophe


> +		priv->tx_skbuff[i] = NULL;
> +	}
> +
>   	kfree(priv->tx_skbuff);
>   	priv->tx_skbuff = NULL;
>   

RE: [PATCH net] net: wan: fsl_ucc_hdlc: free tx_skbuff in uhdlc_memclean

[Holger Brunck]

> 
> Le 04/05/2026 à 18:11, Holger Brunck a écrit :
> > When cleaning up the resources we need to iterate over the tx_skbuf
> > array to free pending TX messages.
> >
> > Fixes: c19b6d246a35 ("drivers/net: support hdlc function for QE-UCC")
> > Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>
> > ---
> >   drivers/net/wan/fsl_ucc_hdlc.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/net/wan/fsl_ucc_hdlc.c
> > b/drivers/net/wan/fsl_ucc_hdlc.c index adf3863463f5..68f78aeabdc3
> > 100644
> > --- a/drivers/net/wan/fsl_ucc_hdlc.c
> > +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> > @@ -739,6 +739,8 @@ static int uhdlc_open(struct net_device *dev)
> >
> >   static void uhdlc_memclean(struct ucc_hdlc_private *priv)
> >   {
> > +     int i;
> > +
> >       qe_muram_free(ioread16be(&priv->ucc_pram->riptr));
> >       qe_muram_free(ioread16be(&priv->ucc_pram->tiptr));
> >
> > @@ -769,6 +771,11 @@ static void uhdlc_memclean(struct ucc_hdlc_private
> *priv)
> >       kfree(priv->rx_skbuff);
> >       priv->rx_skbuff = NULL;
> >
> > +     for (i = 0; i < TX_BD_RING_LEN) {
> > +             kfree(priv->tx_skbuff[i]);
> 
> I don't think you can just kfree() an skb like this.
> 
> I think you have to call dev_kfree_skb_any() instead.
> 

yes you are right or at least dev_kfree_skb() as the error handling code in
ucc_hdlc_tx does.

Thanks
Holger

Re: [PATCH net] net: wan: fsl_ucc_hdlc: free tx_skbuff in uhdlc_memclean

[Jakub Kicinski]

On Tue, 5 May 2026 08:33:34 +0000 Holger Brunck wrote:
> > I don't think you can just kfree() an skb like this.
> > 
> > I think you have to call dev_kfree_skb_any() instead.
> 
> yes you are right or at least dev_kfree_skb() as the error handling code in
> ucc_hdlc_tx does.

Please make it clear in the commit message how you discovered
the issue and how you tested your patches.

RE: [PATCH net] net: wan: fsl_ucc_hdlc: free tx_skbuff in uhdlc_memclean

[Holger Brunck]

 > On Tue, 5 May 2026 08:33:34 +0000 Holger Brunck wrote:
> > > I don't think you can just kfree() an skb like this.
> > >
> > > I think you have to call dev_kfree_skb_any() instead.
> >
> > yes you are right or at least dev_kfree_skb() as the error handling
> > code in ucc_hdlc_tx does.
> 
> Please make it clear in the commit message how you discovered the issue and
> how you tested your patches.

currently the driver is broken if you compile it as a module and try to reload it.
The issue was discovered by a sashiko review for a previous patch of me:
 https://sashiko.dev/#/patchset/20260429114208.941011-1-holger.brunck%40hitachienergy.com

Still there are further issues remaining in the driver. I will update the patch and
the commit message.

Best regards
Holger

Re: [PATCH net] net: wan: fsl_ucc_hdlc: free tx_skbuff in uhdlc_memclean

[kernel test robot]

Hi Holger,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]
[also build test ERROR on v7.1-rc3 next-20260508]
[cannot apply to net/main linus/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Holger-Brunck/net-wan-fsl_ucc_hdlc-free-tx_skbuff-in-uhdlc_memclean/20260514-055007
base:   net-next/main
patch link:    https://lore.kernel.org/r/20260504161145.2217950-1-holger.brunck%40hitachienergy.com
patch subject: [PATCH net] net: wan: fsl_ucc_hdlc: free tx_skbuff in uhdlc_memclean
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20260515/202605151029.MIApM8zq-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260515/202605151029.MIApM8zq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605151029.MIApM8zq-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/net/wan/fsl_ucc_hdlc.c:775:32: error: expected ';' in 'for' statement specifier
     775 |         for (i = 0; i < TX_BD_RING_LEN) {
         |                                       ^
>> drivers/net/wan/fsl_ucc_hdlc.c:775:14: warning: variable 'i' used in loop condition not modified in loop body [-Wfor-loop-analysis]
     775 |         for (i = 0; i < TX_BD_RING_LEN) {
         |                     ^
   1 warning and 1 error generated.


vim +775 drivers/net/wan/fsl_ucc_hdlc.c

   740	
   741	static void uhdlc_memclean(struct ucc_hdlc_private *priv)
   742	{
   743		int i;
   744	
   745		qe_muram_free(ioread16be(&priv->ucc_pram->riptr));
   746		qe_muram_free(ioread16be(&priv->ucc_pram->tiptr));
   747	
   748		if (priv->rx_bd_base) {
   749			dma_free_coherent(priv->dev,
   750					  RX_BD_RING_LEN * sizeof(struct qe_bd),
   751					  priv->rx_bd_base, priv->dma_rx_bd);
   752	
   753			priv->rx_bd_base = NULL;
   754			priv->dma_rx_bd = 0;
   755		}
   756	
   757		if (priv->tx_bd_base) {
   758			dma_free_coherent(priv->dev,
   759					  TX_BD_RING_LEN * sizeof(struct qe_bd),
   760					  priv->tx_bd_base, priv->dma_tx_bd);
   761	
   762			priv->tx_bd_base = NULL;
   763			priv->dma_tx_bd = 0;
   764		}
   765	
   766		if (priv->ucc_pram) {
   767			qe_muram_free(priv->ucc_pram_offset);
   768			priv->ucc_pram = NULL;
   769			priv->ucc_pram_offset = 0;
   770		 }
   771	
   772		kfree(priv->rx_skbuff);
   773		priv->rx_skbuff = NULL;
   774	
 > 775		for (i = 0; i < TX_BD_RING_LEN) {
   776			kfree(priv->tx_skbuff[i]);
   777			priv->tx_skbuff[i] = NULL;
   778		}
   779	
   780		kfree(priv->tx_skbuff);
   781		priv->tx_skbuff = NULL;
   782	
   783		if (priv->uccf) {
   784			ucc_fast_free(priv->uccf);
   785			priv->uccf = NULL;
   786		}
   787	
   788		if (priv->rx_buffer) {
   789			dma_free_coherent(priv->dev,
   790					  (RX_BD_RING_LEN + TX_BD_RING_LEN) * MAX_RX_BUF_LENGTH,
   791					  priv->rx_buffer, priv->dma_rx_addr);
   792			priv->rx_buffer = NULL;
   793			priv->dma_rx_addr = 0;
   794	
   795			priv->tx_buffer = NULL;
   796			priv->dma_tx_addr = 0;
   797	
   798		}
   799	}
   800	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki