linux-kernel - Re: [PATCH net] net/mlx5: Fix double unregister of HCA

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: <1bef8fd9-e9b8-4184-98be-98d016df20d0@nvidia.com>
Date: Thu, 4 Dec 2025 19:07:20 +0200
From: Moshe Shemesh <moshe@...dia.com>
To: Gerd Bayer <gbayer@...ux.ibm.com>, Saeed Mahameed <saeedm@...dia.com>,
	Leon Romanovsky <leon@...nel.org>, Tariq Toukan <tariqt@...dia.com>, "Mark
 Bloch" <mbloch@...dia.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David S.
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, "Jakub
 Kicinski" <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Shay Drory
	<shayd@...dia.com>, Simon Horman <horms@...nel.org>
CC: Lukas Wunner <lukas@...ner.de>, Bjorn Helgaas <helgaas@...nel.org>,
	"Niklas Schnelle" <schnelle@...ux.ibm.com>, Farhan Ali <alifm@...ux.ibm.com>,
	<netdev@...r.kernel.org>, <linux-rdma@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-s390@...r.kernel.org>,
	<linux-pci@...r.kernel.org>
Subject: Re: [PATCH net] net/mlx5: Fix double unregister of HCA_PORTS
 component



On 12/4/2025 11:48 AM, Gerd Bayer wrote:
> 
> On Wed, 2025-12-03 at 17:14 +0200, Moshe Shemesh wrote:
>>
>> On 12/2/2025 1:12 PM, Gerd Bayer wrote:
>>>
> 
>    [ ... snip ... ]
> 
>>>
>>> Fixes: 5a977b5833b7 ("net/mlx5: Lag, move devcom registration to LAG layer")
>>> Signed-off-by: Gerd Bayer <gbayer@...ux.ibm.com>
>>
>> Reviewed-by: Moshe Shemesh <moshe@...dia.com>> ---
>>> Hi Shay et al,
>>>
>>
>> Hi Gerd,
>>    I stepped on this bug recently too, without s390 and was about to
>> submit same fix :) So as you wrote it is unrelated to Lukas' patches and
>> this fix is correct.
> 
> Good to hear. I wonder if you could share how you got to run into this?
> 

mlx5_unload_one() can be called from few flows.
Even that it is always called with devlink lock, serial of 
mlx5_unload_one() twice caused it. I got it on fw_reset and shutdown. I 
I will submit also a patch for calling mlx5_drain_fw_reset() on shutdown 
soon.

>>
>>>
>>> I've spotted two additional places where the devcom reference is not
>>> cleared after calling mlx5_devcom_unregister_component() in
>>> drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c that I have not
>>> addressed with a patch, since I'm unclear about how to test these
>>> paths.
>>
>> As for the other cases, we had the patch 664f76be38a1 ("net/mlx5: Fix
>> IPsec cleanup over MPV device") and two other cases on shared clock and
>> SD but I don't see any flow the shared clock or SD can fail,
>> specifically mlx5_sd_cleanup() checks sd pointer at beginning of the
>> function and nullify it right after sd_unregister() that free devcom.
> 
> I didn't locate any calls to mxl5_devcom_unregister_component() in
> "shared clock" - is that not yet upstream?

mlx5_shared_clock_unregister() in 
drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c

> 
> Regarding SD, I follow that sd_cleanup() is followed immediately after
> sd_unregister() and does the clean-up. One path remains uncovered
> though: The error exit at
> https://elixir.bootlin.com/linux/v6.18/source/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c#L265
> 
> Not sure, how likely that is...

It comes on error flow but after successful 
mlx5_devcom_register_component() in sd_register(), and that error leads 
to error flow in mlx5_sd_init(), which calls sd_cleanup() too.

> 
> Thanks,
> Gerd