[PATCH net v2] eth: fbnic: fix double-free of PCS on phylink creation failure

[PATCH net v2] eth: fbnic: fix double-free of PCS on phylink creation failure

[Bobby Eshleman]

From: Bobby Eshleman <bobbyeshleman@meta.com>

fbnic_phylink_create() stores the newly allocated PCS in fbn->pcs and
then calls phylink_create(). When phylink_create() fails, the error path
correctly destroys the PCS via xpcs_destroy_pcs(), but the caller,
fbnic_netdev_alloc(), responds by invoking fbnic_netdev_free() which
calls fbnic_phylink_destroy(). That function finds fbn->pcs non-NULL and
calls xpcs_destroy_pcs() a second time on the already-freed object,
triggering a refcount underflow use-after-free:

[   1.934973] fbnic 0000:01:00.0: Failed to create Phylink interface, err: -22
[   1.935103] ------------[ cut here ]------------
[   1.935179] refcount_t: underflow; use-after-free.
[   1.935252] WARNING: lib/refcount.c:28 at refcount_warn_saturate+0x59/0x90, CPU#0: swapper/0/1
[   1.935389] Modules linked in:
[   1.935484] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 7.0.0-virtme-04244-g1f5ffc672165-dirty #1 PREEMPT(lazy)
[   1.935661] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
[   1.935826] RIP: 0010:refcount_warn_saturate+0x59/0x90
[   1.935931] Code: 44 48 8d 3d 49 f9 a7 01 67 48 0f b9 3a e9 bf 1e 96 00 48 8d 3d 48 f9 a7 01 67 48 0f b9 3a c3 cc cc cc cc 48 8d 3d 47 f9 a7 01 <67> 48 0f b9 3a c3 cc cc cc cc 48 8d 3d 46 f9 a7 01 67 48 0f b9 3a
[   1.936274] RSP: 0000:ffffd0d440013c58 EFLAGS: 00010246
[   1.936376] RAX: 0000000000000000 RBX: ffff8f39c188c278 RCX: 000000000000002b
[   1.936524] RDX: ffff8f39c004f000 RSI: 0000000000000003 RDI: ffffffff96abab00
[   1.936692] RBP: ffff8f39c188c240 R08: ffffffff96988e88 R09: 00000000ffffdfff
[   1.936835] R10: ffffffff96878ea0 R11: 0000000000000187 R12: 0000000000000000
[   1.936970] R13: ffff8f39c0cef0c8 R14: ffff8f39c1ac01c0 R15: 0000000000000000
[   1.937114] FS:  0000000000000000(0000) GS:ffff8f3ba08b4000(0000) knlGS:0000000000000000
[   1.937273] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   1.937382] CR2: ffff8f3b3ffff000 CR3: 0000000172642001 CR4: 0000000000372ef0
[   1.937540] Call Trace:
[   1.937619]  <TASK>
[   1.937698]  xpcs_destroy_pcs+0x25/0x40
[   1.937783]  fbnic_netdev_alloc+0x1e5/0x200
[   1.937859]  fbnic_probe+0x230/0x370
[   1.937939]  local_pci_probe+0x3e/0x90
[   1.938013]  pci_device_probe+0xbb/0x1e0
[   1.938091]  ? sysfs_do_create_link_sd+0x6d/0xe0
[   1.938188]  really_probe+0xc1/0x2b0
[   1.938282]  __driver_probe_device+0x73/0x120
[   1.938371]  driver_probe_device+0x1e/0xe0
[   1.938466]  __driver_attach+0x8d/0x190
[   1.938560]  ? __pfx___driver_attach+0x10/0x10
[   1.938663]  bus_for_each_dev+0x7b/0xd0
[   1.938758]  bus_add_driver+0xe8/0x210
[   1.938854]  driver_register+0x60/0x120
[   1.938929]  ? __pfx_fbnic_init_module+0x10/0x10
[   1.939026]  fbnic_init_module+0x25/0x60
[   1.939109]  do_one_initcall+0x49/0x220
[   1.939202]  ? rdinit_setup+0x20/0x40
[   1.939304]  kernel_init_freeable+0x1b0/0x310
[   1.939449]  ? __pfx_kernel_init+0x10/0x10
[   1.939560]  kernel_init+0x1a/0x1c0
[   1.939640]  ret_from_fork+0x1ed/0x240
[   1.939730]  ? __pfx_kernel_init+0x10/0x10
[   1.939805]  ret_from_fork_asm+0x1a/0x30
[   1.939886]  </TASK>
[   1.939927] ---[ end trace 0000000000000000 ]---
[   1.940184] fbnic 0000:01:00.0: Netdev allocation failed

Instead of calling fbnic_phylink_destroy(), the prior initialization of
netdev should just be unrolled with free_netdev() and clearing
fbd->netdev.

Clearing fbd->netdev to NULL avoids UAF in init_failure_mode where
callers guard by checking !fbd->netdev, such as fbnic_mdio_read_pmd().
These callers remain active even after a failed probe, so fdb->netdev
still needs to be cleared.

Fixes: d0fe7104c795 ("fbnic: Replace use of internal PCS w/ Designware XPCS")
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v2:
- instead of just clearing fbn->pcs, avoid the UAF by only doing
  teardown for netdev when phylink creation fails, avoid
  fbnic_phylink_destroy
- clear fbn->netdev to avoid failures in post-probe init_failure_mode
- Link to v1: https://lore.kernel.org/r/20260416-fbnic-pcs-fix-v1-1-ac4b6badeac0@meta.com
---
 drivers/net/ethernet/meta/fbnic/fbnic_netdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
index c406a3b56b37..4dea2bb58d2f 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
@@ -826,7 +826,8 @@ struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
 	netif_tx_stop_all_queues(netdev);
 
 	if (fbnic_phylink_create(netdev)) {
-		fbnic_netdev_free(fbd);
+		free_netdev(netdev);
+		fbd->netdev = NULL;
 		return NULL;
 	}
 

---
base-commit: bd3a4795d5744f59a1f485379f1303e5e606f377
change-id: 20260416-fbnic-pcs-fix-26dc23c7deae

Best regards,
-- 
Bobby Eshleman <bobbyeshleman@meta.com>

Re: [PATCH net v2] eth: fbnic: fix double-free of PCS on phylink creation failure

[Paolo Abeni]

On 5/5/26 3:42 AM, Bobby Eshleman wrote:
> From: Bobby Eshleman <bobbyeshleman@meta.com>
> 
> fbnic_phylink_create() stores the newly allocated PCS in fbn->pcs and
> then calls phylink_create(). When phylink_create() fails, the error path
> correctly destroys the PCS via xpcs_destroy_pcs(), but the caller,
> fbnic_netdev_alloc(), responds by invoking fbnic_netdev_free() which
> calls fbnic_phylink_destroy(). That function finds fbn->pcs non-NULL and
> calls xpcs_destroy_pcs() a second time on the already-freed object,
> triggering a refcount underflow use-after-free:
> 
> [   1.934973] fbnic 0000:01:00.0: Failed to create Phylink interface, err: -22
> [   1.935103] ------------[ cut here ]------------
> [   1.935179] refcount_t: underflow; use-after-free.
> [   1.935252] WARNING: lib/refcount.c:28 at refcount_warn_saturate+0x59/0x90, CPU#0: swapper/0/1
> [   1.935389] Modules linked in:
> [   1.935484] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 7.0.0-virtme-04244-g1f5ffc672165-dirty #1 PREEMPT(lazy)
> [   1.935661] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> [   1.935826] RIP: 0010:refcount_warn_saturate+0x59/0x90
> [   1.935931] Code: 44 48 8d 3d 49 f9 a7 01 67 48 0f b9 3a e9 bf 1e 96 00 48 8d 3d 48 f9 a7 01 67 48 0f b9 3a c3 cc cc cc cc 48 8d 3d 47 f9 a7 01 <67> 48 0f b9 3a c3 cc cc cc cc 48 8d 3d 46 f9 a7 01 67 48 0f b9 3a
> [   1.936274] RSP: 0000:ffffd0d440013c58 EFLAGS: 00010246
> [   1.936376] RAX: 0000000000000000 RBX: ffff8f39c188c278 RCX: 000000000000002b
> [   1.936524] RDX: ffff8f39c004f000 RSI: 0000000000000003 RDI: ffffffff96abab00
> [   1.936692] RBP: ffff8f39c188c240 R08: ffffffff96988e88 R09: 00000000ffffdfff
> [   1.936835] R10: ffffffff96878ea0 R11: 0000000000000187 R12: 0000000000000000
> [   1.936970] R13: ffff8f39c0cef0c8 R14: ffff8f39c1ac01c0 R15: 0000000000000000
> [   1.937114] FS:  0000000000000000(0000) GS:ffff8f3ba08b4000(0000) knlGS:0000000000000000
> [   1.937273] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   1.937382] CR2: ffff8f3b3ffff000 CR3: 0000000172642001 CR4: 0000000000372ef0
> [   1.937540] Call Trace:
> [   1.937619]  <TASK>
> [   1.937698]  xpcs_destroy_pcs+0x25/0x40
> [   1.937783]  fbnic_netdev_alloc+0x1e5/0x200
> [   1.937859]  fbnic_probe+0x230/0x370
> [   1.937939]  local_pci_probe+0x3e/0x90
> [   1.938013]  pci_device_probe+0xbb/0x1e0
> [   1.938091]  ? sysfs_do_create_link_sd+0x6d/0xe0
> [   1.938188]  really_probe+0xc1/0x2b0
> [   1.938282]  __driver_probe_device+0x73/0x120
> [   1.938371]  driver_probe_device+0x1e/0xe0
> [   1.938466]  __driver_attach+0x8d/0x190
> [   1.938560]  ? __pfx___driver_attach+0x10/0x10
> [   1.938663]  bus_for_each_dev+0x7b/0xd0
> [   1.938758]  bus_add_driver+0xe8/0x210
> [   1.938854]  driver_register+0x60/0x120
> [   1.938929]  ? __pfx_fbnic_init_module+0x10/0x10
> [   1.939026]  fbnic_init_module+0x25/0x60
> [   1.939109]  do_one_initcall+0x49/0x220
> [   1.939202]  ? rdinit_setup+0x20/0x40
> [   1.939304]  kernel_init_freeable+0x1b0/0x310
> [   1.939449]  ? __pfx_kernel_init+0x10/0x10
> [   1.939560]  kernel_init+0x1a/0x1c0
> [   1.939640]  ret_from_fork+0x1ed/0x240
> [   1.939730]  ? __pfx_kernel_init+0x10/0x10
> [   1.939805]  ret_from_fork_asm+0x1a/0x30
> [   1.939886]  </TASK>
> [   1.939927] ---[ end trace 0000000000000000 ]---
> [   1.940184] fbnic 0000:01:00.0: Netdev allocation failed
> 
> Instead of calling fbnic_phylink_destroy(), the prior initialization of
> netdev should just be unrolled with free_netdev() and clearing
> fbd->netdev.
> 
> Clearing fbd->netdev to NULL avoids UAF in init_failure_mode where
> callers guard by checking !fbd->netdev, such as fbnic_mdio_read_pmd().
> These callers remain active even after a failed probe, so fdb->netdev
> still needs to be cleared.
> 
> Fixes: d0fe7104c795 ("fbnic: Replace use of internal PCS w/ Designware XPCS")
> Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>

Note that sashiko-gemini spotted a pre-existing issue:

https://sashiko.dev/#/patchset/20260504-fbnic-pcs-fix-v2-1-de45192821d9%40meta.com

does not block this patch but could deserve a follow-up.

Thanks,

Paolo

Re: [PATCH net v2] eth: fbnic: fix double-free of PCS on phylink creation failure

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 04 May 2026 18:42:11 -0700 you wrote:
> From: Bobby Eshleman <bobbyeshleman@meta.com>
> 
> fbnic_phylink_create() stores the newly allocated PCS in fbn->pcs and
> then calls phylink_create(). When phylink_create() fails, the error path
> correctly destroys the PCS via xpcs_destroy_pcs(), but the caller,
> fbnic_netdev_alloc(), responds by invoking fbnic_netdev_free() which
> calls fbnic_phylink_destroy(). That function finds fbn->pcs non-NULL and
> calls xpcs_destroy_pcs() a second time on the already-freed object,
> triggering a refcount underflow use-after-free:
> 
> [...]

Here is the summary with links:
  - [net,v2] eth: fbnic: fix double-free of PCS on phylink creation failure
    https://git.kernel.org/netdev/net/c/593dfd40a94c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net v2] eth: fbnic: fix double-free of PCS on phylink creation failure

[Jakub Kicinski]

On Thu, 7 May 2026 12:34:24 +0200 Paolo Abeni wrote:
> > Clearing fbd->netdev to NULL avoids UAF in init_failure_mode where
> > callers guard by checking !fbd->netdev, such as fbnic_mdio_read_pmd().
> > These callers remain active even after a failed probe, so fdb->netdev
> > still needs to be cleared.
> > 
> > Fixes: d0fe7104c795 ("fbnic: Replace use of internal PCS w/ Designware XPCS")
> > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>  
> 
> Note that sashiko-gemini spotted a pre-existing issue:
> 
> https://sashiko.dev/#/patchset/20260504-fbnic-pcs-fix-v2-1-de45192821d9%40meta.com
> 
> does not block this patch but could deserve a follow-up.

fbd is a devlink priv, not netdev priv, touching it after free_netdev()
is perfectly fine. I wish Gemini tried a *little* harder instead of
guessing :| Sorry for not commenting earlier.

Re: [PATCH net v2] eth: fbnic: fix double-free of PCS on phylink creation failure

[Jakub Kicinski]

On Thu, 7 May 2026 07:24:53 -0700 Jakub Kicinski wrote:
> On Thu, 7 May 2026 12:34:24 +0200 Paolo Abeni wrote:
> > > Clearing fbd->netdev to NULL avoids UAF in init_failure_mode where
> > > callers guard by checking !fbd->netdev, such as fbnic_mdio_read_pmd().
> > > These callers remain active even after a failed probe, so fdb->netdev
> > > still needs to be cleared.
> > > 
> > > Fixes: d0fe7104c795 ("fbnic: Replace use of internal PCS w/ Designware XPCS")
> > > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>    
> > 
> > Note that sashiko-gemini spotted a pre-existing issue:
> > 
> > https://sashiko.dev/#/patchset/20260504-fbnic-pcs-fix-v2-1-de45192821d9%40meta.com
> > 
> > does not block this patch but could deserve a follow-up.  
> 
> fbd is a devlink priv, not netdev priv, touching it after free_netdev()
> is perfectly fine. I wish Gemini tried a *little* harder instead of
> guessing :| Sorry for not commenting earlier.

Ugh, not enough coffee. It's complaining about MDIO reads, I think
that's valid.

Re: [PATCH net v2] eth: fbnic: fix double-free of PCS on phylink creation failure

[Bobby Eshleman]

On Thu, May 07, 2026 at 07:29:54AM -0700, Jakub Kicinski wrote:
> On Thu, 7 May 2026 07:24:53 -0700 Jakub Kicinski wrote:
> > On Thu, 7 May 2026 12:34:24 +0200 Paolo Abeni wrote:
> > > > Clearing fbd->netdev to NULL avoids UAF in init_failure_mode where
> > > > callers guard by checking !fbd->netdev, such as fbnic_mdio_read_pmd().
> > > > These callers remain active even after a failed probe, so fdb->netdev
> > > > still needs to be cleared.
> > > > 
> > > > Fixes: d0fe7104c795 ("fbnic: Replace use of internal PCS w/ Designware XPCS")
> > > > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>    
> > > 
> > > Note that sashiko-gemini spotted a pre-existing issue:
> > > 
> > > https://sashiko.dev/#/patchset/20260504-fbnic-pcs-fix-v2-1-de45192821d9%40meta.com
> > > 
> > > does not block this patch but could deserve a follow-up.  
> > 
> > fbd is a devlink priv, not netdev priv, touching it after free_netdev()
> > is perfectly fine. I wish Gemini tried a *little* harder instead of
> > guessing :| Sorry for not commenting earlier.
> 
> Ugh, not enough coffee. It's complaining about MDIO reads, I think
> that's valid.

It is, but I think the race pre-exists.

static int
fbnic_mdio_read_pmd(struct fbnic_dev *fbd, int addr, int regnum)
[...]
	if (fbd->netdev) {
		fbn = netdev_priv(fbd->netdev);
		if (fbn->aui < FBNIC_AUI_UNKNOWN)
			aui = fbn->aui;
	}


Definitely possible that ->netdev gets freed concurrently with
fbd->netdev evaluating to true... but fbnic_netdev_free() faces the same
race.

I'm open to fixing this all at once, if preferred. Probably need to look
at some of the other fbnic_net ptr guards too.

Best,
Bobby

Re: [PATCH net v2] eth: fbnic: fix double-free of PCS on phylink creation failure

[Jakub Kicinski]

On Thu, 7 May 2026 08:35:21 -0700 Bobby Eshleman wrote:
> > > fbd is a devlink priv, not netdev priv, touching it after free_netdev()
> > > is perfectly fine. I wish Gemini tried a *little* harder instead of
> > > guessing :| Sorry for not commenting earlier.  
> > 
> > Ugh, not enough coffee. It's complaining about MDIO reads, I think
> > that's valid.  
> 
> It is, but I think the race pre-exists.
> 
> static int
> fbnic_mdio_read_pmd(struct fbnic_dev *fbd, int addr, int regnum)
> [...]
> 	if (fbd->netdev) {
> 		fbn = netdev_priv(fbd->netdev);
> 		if (fbn->aui < FBNIC_AUI_UNKNOWN)
> 			aui = fbn->aui;
> 	}
> 
> 
> Definitely possible that ->netdev gets freed concurrently with
> fbd->netdev evaluating to true... but fbnic_netdev_free() faces the same
> race.
> 
> I'm open to fixing this all at once, if preferred. Probably need to look
> at some of the other fbnic_net ptr guards too.

I agree with Paolo, seems separate.

FWIW I think the fix may be to move the single aui field that mdio
cares about to fbd instead of fbn ? Feels like the problem is due
to a layering violation, mdio should not be touching fbn fields.

Re: [PATCH net v2] eth: fbnic: fix double-free of PCS on phylink creation failure

[Bobby Eshleman]

On Thu, May 07, 2026 at 09:01:27AM -0700, Jakub Kicinski wrote:
> On Thu, 7 May 2026 08:35:21 -0700 Bobby Eshleman wrote:
> > > > fbd is a devlink priv, not netdev priv, touching it after free_netdev()
> > > > is perfectly fine. I wish Gemini tried a *little* harder instead of
> > > > guessing :| Sorry for not commenting earlier.  
> > > 
> > > Ugh, not enough coffee. It's complaining about MDIO reads, I think
> > > that's valid.  
> > 
> > It is, but I think the race pre-exists.
> > 
> > static int
> > fbnic_mdio_read_pmd(struct fbnic_dev *fbd, int addr, int regnum)
> > [...]
> > 	if (fbd->netdev) {
> > 		fbn = netdev_priv(fbd->netdev);
> > 		if (fbn->aui < FBNIC_AUI_UNKNOWN)
> > 			aui = fbn->aui;
> > 	}
> > 
> > 
> > Definitely possible that ->netdev gets freed concurrently with
> > fbd->netdev evaluating to true... but fbnic_netdev_free() faces the same
> > race.
> > 
> > I'm open to fixing this all at once, if preferred. Probably need to look
> > at some of the other fbnic_net ptr guards too.
> 
> I agree with Paolo, seems separate.
> 
> FWIW I think the fix may be to move the single aui field that mdio
> cares about to fbd instead of fbn ? Feels like the problem is due
> to a layering violation, mdio should not be touching fbn fields.

SGTM. And I'll take a look at moving aui.

Thanks,
Bobby