[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260105173954.7d6ade2d@kernel.org>
Date: Mon, 5 Jan 2026 17:39:54 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Fan Gong <gongfan1@...wei.com>
Cc: Zhu Yikai <zhuyikai1@...artners.com>, <netdev@...r.kernel.org>, "David
S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo
Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Andrew Lunn
<andrew+netdev@...n.ch>, Markus Elfring <Markus.Elfring@....de>, Pavan
Chebbi <pavan.chebbi@...adcom.com>, ALOK TIWARI <alok.a.tiwari@...cle.com>,
<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>, luosifu
<luosifu@...wei.com>, Xin Guo <guoxin09@...wei.com>, Shen Chenyang
<shenchenyang1@...ilicon.com>, Zhou Shuai <zhoushuai28@...wei.com>, Wu Like
<wulike1@...wei.com>, Shi Jing <shijing34@...wei.com>, Luo Yang
<luoyang82@...artners.com>
Subject: Re: [PATCH net-next v08 3/9] hinic3: Add .ndo_tx_timeout and
.ndo_get_stats64
AI code review points out:
> @@ -329,6 +359,7 @@ static int hinic3_nic_probe(struct auxiliary_device *adev,
> if (err)
> goto err_uninit_sw;
>
> + queue_delayed_work(nic_dev->workq, &nic_dev->periodic_work, HZ);
> netif_carrier_off(netdev);
>
> err = register_netdev(netdev);
> @@ -346,7 +377,8 @@ static int hinic3_nic_probe(struct auxiliary_device *adev,
>
> err_free_nic_io:
> hinic3_free_nic_io(nic_dev);
> -
> +err_free_nic_dev:
> + hinic3_free_nic_dev(nic_dev);
If register_netdev() fails after the delayed work has been queued, the error
path reaches hinic3_free_nic_dev() which calls destroy_workqueue() without
first cancelling the pending delayed work. The destroy_workqueue()
documentation states that all delayed_work must be cancelled before calling
it.
Should there be a disable_delayed_work_sync() or cancel_delayed_work_sync()
call in the error path before hinic3_free_nic_dev()? The remove path handles
this correctly:
> @@ -368,6 +400,9 @@ static void hinic3_nic_remove(struct auxiliary_device *adev)
> netdev = nic_dev->netdev;
> unregister_netdev(netdev);
>
> + disable_delayed_work_sync(&nic_dev->periodic_work);
> + hinic3_free_nic_dev(nic_dev);
Here in hinic3_nic_remove() the work is properly cancelled before destroying
the workqueue, but the probe error path after queue_delayed_work() does not
have similar protection.
Powered by blists - more mailing lists