| 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
| ||
|
Message-ID: <CAMArcTWDe2cd41=ub=zzvYifaYcYv-N-csxfqxUvejy_L0D6UQ@mail.gmail.com>
Date: Wed, 30 Apr 2025 21:59:59 +0900
From: Taehee Yoo <ap420073@...il.com>
To: Vadim Fedorenko <vadfed@...a.com>
Cc: Vadim Fedorenko <vadim.fedorenko@...ux.dev>, Michael Chan <michael.chan@...adcom.com>,
Pavan Chebbi <pavan.chebbi@...adcom.com>, Jakub Kicinski <kuba@...nel.org>,
Richard Cochran <richardcochran@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net v4] bnxt_en: improve TX timestamping FIFO configuration
On Thu, Apr 24, 2025 at 10:11 PM Vadim Fedorenko <vadfed@...a.com> wrote:
>
Hi Vadim,
Thanks for this work!
> Reconfiguration of netdev may trigger close/open procedure which can
> break FIFO status by adjusting the amount of empty slots for TX
> timestamps. But it is not really needed because timestamps for the
> packets sent over the wire still can be retrieved. On the other side,
> during netdev close procedure any skbs waiting for TX timestamps can be
> leaked because there is no cleaning procedure called. Free skbs waiting
> for TX timestamps when closing netdev.
>
> Fixes: 8aa2a79e9b95 ("bnxt_en: Increase the max total outstanding PTP TX packets to 4")
> Reviewed-by: Michael Chan <michael.chan@...adcom.com>
> Reviewed-by: Pavan Chebbi <pavan.chebbi@...adcom.com>
> Signed-off-by: Vadim Fedorenko <vadfed@...a.com>
> ---
> v3 -> v4:
> * actually remove leftover unused variable in bnxt_ptp_clear()
> (v3 was not committed before preparing unfortunately)
> v2 -> v3:
> * remove leftover unused variable in bnxt_ptp_clear()
> v1 -> v2:
> * move clearing of TS skbs to bnxt_free_tx_skbs
> * remove spinlock as no TX is possible after bnxt_tx_disable()
> * remove extra FIFO clearing in bnxt_ptp_clear()
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 5 ++--
> drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 29 ++++++++++++++-----
> drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h | 1 +
> 3 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index c8e3468eee61..2c8e2c19d854 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -3414,6 +3414,9 @@ static void bnxt_free_tx_skbs(struct bnxt *bp)
>
> bnxt_free_one_tx_ring_skbs(bp, txr, i);
> }
> +
> + if (bp->ptp_cfg && !(bp->fw_cap & BNXT_FW_CAP_TX_TS_CMP))
> + bnxt_ptp_free_txts_skbs(bp->ptp_cfg);
> }
>
> static void bnxt_free_one_rx_ring(struct bnxt *bp, struct bnxt_rx_ring_info *rxr)
> @@ -12797,8 +12800,6 @@ static int __bnxt_open_nic(struct bnxt *bp, bool irq_re_init, bool link_re_init)
> /* VF-reps may need to be re-opened after the PF is re-opened */
> if (BNXT_PF(bp))
> bnxt_vf_reps_open(bp);
> - if (bp->ptp_cfg && !(bp->fw_cap & BNXT_FW_CAP_TX_TS_CMP))
> - WRITE_ONCE(bp->ptp_cfg->tx_avail, BNXT_MAX_TX_TS);
> bnxt_ptp_init_rtc(bp, true);
> bnxt_ptp_cfg_tstamp_filters(bp);
> if (BNXT_SUPPORTS_MULTI_RSS_CTX(bp))
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> index 2d4e19b96ee7..0669d43472f5 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> @@ -794,6 +794,27 @@ static long bnxt_ptp_ts_aux_work(struct ptp_clock_info *ptp_info)
> return HZ;
> }
>
> +void bnxt_ptp_free_txts_skbs(struct bnxt_ptp_cfg *ptp)
> +{
> + struct bnxt_ptp_tx_req *txts_req;
> + u16 cons = ptp->txts_cons;
> +
> + /* make sure ptp aux worker finished with
> + * possible BNXT_STATE_OPEN set
> + */
> + ptp_cancel_worker_sync(ptp->ptp_clock);
> +
> + ptp->tx_avail = BNXT_MAX_TX_TS;
> + while (cons != ptp->txts_prod) {
> + txts_req = &ptp->txts_req[cons];
> + if (!IS_ERR_OR_NULL(txts_req->tx_skb))
> + dev_kfree_skb_any(txts_req->tx_skb);
> + cons = NEXT_TXTS(cons);
> + }
> + ptp->txts_cons = cons;
> + ptp_schedule_worker(ptp->ptp_clock, 0);
> +}
> +
> int bnxt_ptp_get_txts_prod(struct bnxt_ptp_cfg *ptp, u16 *prod)
> {
> spin_lock_bh(&ptp->ptp_tx_lock);
> @@ -1105,7 +1126,6 @@ int bnxt_ptp_init(struct bnxt *bp)
> void bnxt_ptp_clear(struct bnxt *bp)
> {
> struct bnxt_ptp_cfg *ptp = bp->ptp_cfg;
> - int i;
>
> if (!ptp)
> return;
> @@ -1117,12 +1137,5 @@ void bnxt_ptp_clear(struct bnxt *bp)
> kfree(ptp->ptp_info.pin_config);
> ptp->ptp_info.pin_config = NULL;
>
> - for (i = 0; i < BNXT_MAX_TX_TS; i++) {
> - if (ptp->txts_req[i].tx_skb) {
> - dev_kfree_skb_any(ptp->txts_req[i].tx_skb);
> - ptp->txts_req[i].tx_skb = NULL;
> - }
> - }
> -
> bnxt_unmap_ptp_regs(bp);
> }
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h
> index a95f05e9c579..0481161d26ef 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h
> @@ -162,6 +162,7 @@ int bnxt_ptp_cfg_tstamp_filters(struct bnxt *bp);
> void bnxt_ptp_reapply_pps(struct bnxt *bp);
> int bnxt_hwtstamp_set(struct net_device *dev, struct ifreq *ifr);
> int bnxt_hwtstamp_get(struct net_device *dev, struct ifreq *ifr);
> +void bnxt_ptp_free_txts_skbs(struct bnxt_ptp_cfg *ptp);
> int bnxt_ptp_get_txts_prod(struct bnxt_ptp_cfg *ptp, u16 *prod);
> void bnxt_get_tx_ts_p5(struct bnxt *bp, struct sk_buff *skb, u16 prod);
> int bnxt_get_rx_ts_p5(struct bnxt *bp, u64 *ts, u32 pkt_ts);
> --
> 2.47.1
>
>
I’ve encountered a kernel panic that I think is related to this patch.
Could you please investigate it?
Reproducer:
ip link set $interface up
modprobe -rv bnxt_en
Splat looks like:
Oops: general protection fault, probably for non-canonical address
0xdffffc00000000fd:I
KASAN: null-ptr-deref in range [0x00000000000007e8-0x00000000000007ef]
CPU: 2 UID: 0 PID: 1963 Comm: modprobe Not tainted 6.15.0-rc3+ #5
PREEMPT(undef) 78b5b
RIP: 0010:__kthread_cancel_work_sync (/kernel/kthread.c:1476)
Code: 00 48 b8 00 00 00 00 00 fc ff df 41 57 4c 8d 7f 18 41 56 4c 89
fa 41 55 48 c1 ea4
Code starting with the faulting instruction
===========================================
0: 00 48 b8 add %cl,-0x48(%rax)
3: 00 00 add %al,(%rax)
5: 00 00 add %al,(%rax)
7: 00 fc add %bh,%ah
9: ff (bad)
a: df 41 57 filds 0x57(%rcx)
d: 4c 8d 7f 18 lea 0x18(%rdi),%r15
11: 41 56 push %r14
13: 4c 89 fa mov %r15,%rdx
16: 41 55 push %r13
18: 48 rex.W
19: c1 .byte 0xc1
1a: a4 movsb %ds:(%rsi),%es:(%rdi)
RSP: 0018:ffff888111857608 EFLAGS: 00010292
RAX: dffffc0000000000 RBX: 00000000000007d0 RCX: ffff8881330ece8e
RDX: 00000000000000fd RSI: 0000000000000001 RDI: 00000000000007d0
RBP: ffff888198ad8e00 R08: 0000000000000001 R09: ffff888198b800d8
R10: ffff888198b8019f R11: 0000000000000000 R12: ffff888198ad9008
R13: 0000000000000001 R14: ffff888198ad8e88 R15: 00000000000007e8
FS: 00007f831f921080(0000) GS:ffff888888405000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00005655646882e0 CR3: 000000014c52e000 CR4: 00000000007506f0
PKRU: 55555554
Call Trace:
<TASK>
bnxt_ptp_free_txts_skbs
(/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c:807) bnxt_en
__bnxt_close_nic.constprop.0
(/drivers/net/ethernet/broadcom/bnxt/bnxt.c:3513
/drivers/net/ethernet/broadcom/bnxt/bnxt.c:3523
/drivers/net/ethernet/broadcom/bnxt/bnxt.c:12965) bnxt_en
? __lock_acquire (/kernel/locking/lockdep.c:5246)
? __pfx___bnxt_close_nic.constprop.0
(/drivers/net/ethernet/broadcom/bnxt/bnxt.c:12940) bnxt_en
bnxt_close_nic (/drivers/net/ethernet/broadcom/bnxt/bnxt.c:12980) bnxt_en
? do_raw_spin_trylock (/./arch/x86/include/asm/atomic.h:107
/./include/linux/atomic/atomic-arch-fallback.h:2170
/./include/linux/atomic/atomic-instrumented.h:1302
/./include/asm-generic/qspinlock.h:97
/kernel/locking/spinlock_debug.c:123)
? __pfx_bnxt_close_nic
(/drivers/net/ethernet/broadcom/bnxt/bnxt.c:12980) bnxt_en
? __local_bh_enable_ip (/./arch/x86/include/asm/irqflags.h:42
/./arch/x86/include/asm/irqflags.h:119 /kernel/softirq.c:412)
? lockdep_hardirqs_on (/./arch/x86/include/generated/asm/syscalls_64.h:316)
? dev_deactivate_many (/net/sched/sch_generic.c:1325
/net/sched/sch_generic.c:1383)
? __local_bh_enable_ip (/./arch/x86/include/asm/irqflags.h:42
/./arch/x86/include/asm/irqflags.h:119 /kernel/softirq.c:412)
bnxt_close (/drivers/net/ethernet/broadcom/bnxt/bnxt.c:12215
/drivers/net/ethernet/broadcom/bnxt/bnxt.c:13015) bnxt_en
? __pfx_bnxt_close (/drivers/net/ethernet/broadcom/bnxt/bnxt.c:13011) bnxt_en
? notifier_call_chain (/kernel/notifier.c:85)
__dev_close_many (/net/core/dev.c:1702)
? __pfx___dev_close_many (/net/core/dev.c:1663)
? __pfx___mutex_lock (/arch/x86/entry/syscall_32.c:46)
dev_close_many (/net/core/dev.c:1729)
? __pfx_dev_close_many (/net/core/dev.c:1719)
unregister_netdevice_many_notify (/net/core/dev.c:11946)
? rcu_is_watching (/./include/linux/context_tracking.h:128
/kernel/rcu/tree.c:736)
? __mutex_lock (/arch/x86/entry/syscall_32.c:46)
? __pfx_unregister_netdevice_many_notify (/net/core/dev.c:11909)
? rtnl_net_dev_lock (/net/core/dev.c:2093)
? __pfx___mutex_lock (/arch/x86/entry/syscall_32.c:46)
unregister_netdevice_queue (/net/core/dev.c:11891)
? __pfx_unregister_netdevice_queue (/net/core/dev.c:11880)
? rtnl_net_dev_lock (/./include/linux/rcupdate.h:331
/./include/linux/rcupdate.h:841 /net/core/dev.c:2084)
? rtnl_net_dev_lock (/net/core/dev.c:2093)
unregister_netdev (/./include/net/net_namespace.h:409
/./include/linux/netdevice.h:2708 /net/core/dev.c:2104
/net/core/dev.c:12065)
bnxt_remove_one (/drivers/net/ethernet/broadcom/bnxt/bnxt.c:16012) bnxt_en
pci_device_remove (/drivers/pci/pci-driver.c:474)
device_release_driver_internal (/drivers/base/dd.c:1275 /drivers/base/dd.c:1296)
driver_detach (/drivers/base/dd.c:1360)
bus_remove_driver (/drivers/base/bus.c:748)
pci_unregister_driver (/./include/linux/spinlock.h:351
/drivers/pci/pci-driver.c:85 /drivers/pci/pci-driver.c:1465)
bnxt_exit (/drivers/net/ethernet/broadcom/bnxt/bnxt.c:1588) bnxt_en
__do_sys_delete_module.constprop.0 (/kernel/module/main.c:781)
? __pfx___do_sys_delete_module.constprop.0 (/kernel/module/main.c:724)
? __pfx_rseq_syscall (/kernel/rseq.c:458)
? ksys_write (/fs/read_write.c:736)
? __pfx_ksys_write (/fs/read_write.c:726)
? rcu_is_watching (/./include/linux/context_tracking.h:128
/kernel/rcu/tree.c:736)
? do_syscall_64 (/./include/trace/events/initcall.h:10)
do_syscall_64 (/./include/trace/events/initcall.h:10)
entry_SYSCALL_64_after_hwframe (/./include/trace/events/initcall.h:27)
RIP: 0033:0x7f831f12ac9b
Code: 73 01 c3 48 8b 0d 7d 81 0d 00 f7 d8 64 89 01 48 83 c8 ff c3 66
2e 0f 1f 84 00 008
Code starting with the faulting instruction
===========================================
0: 73 01 jae 0x3
2: c3 ret
3: 48 8b 0d 7d 81 0d 00 mov 0xd817d(%rip),%rcx # 0xd8187
a: f7 d8 neg %eax
c: 64 89 01 mov %eax,%fs:(%rcx)
f: 48 83 c8 ff or $0xffffffffffffffff,%rax
13: c3 ret
14: 66 data16
15: 2e cs
16: 0f .byte 0xf
17: 1f (bad)
18: 84 00 test %al,(%rax)
1a: 08 .byte 0x8
RSP: 002b:00007ffdfb651448 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
RAX: ffffffffffffffda RBX: 000055fb93ae5fa0 RCX: 00007f831f12ac9b
RDX: 0000000000000000 RSI: 0000000000000800 RDI: 000055fb93ae6008
RBP: 00007ffdfb651470 R08: 0000000000000073 R09: 0000000000000000
R10: 00000000ffffffff R11: 0000000000000206 R12: 0000000000000000
R13: 00007ffdfb6514a0 R14: 0000000000000000 R15: 0000000000000000
</TASK>
Modules linked in: xt_nat xt_tcpudp veth xt_conntrack nft_chain_nat
xt_MASQUERADE nf_c]
---[ end trace 0000000000000000 ]---
RIP: 0010:__kthread_cancel_work_sync (/kernel/kthread.c:1476)
Code: 00 48 b8 00 00 00 00 00 fc ff df 41 57 4c 8d 7f 18 41 56 4c 89
fa 41 55 48 c1 ea4
Code starting with the faulting instruction
===========================================
0: 00 48 b8 add %cl,-0x48(%rax)
3: 00 00 add %al,(%rax)
5: 00 00 add %al,(%rax)
7: 00 fc add %bh,%ah
9: ff (bad)
a: df 41 57 filds 0x57(%rcx)
d: 4c 8d 7f 18 lea 0x18(%rdi),%r15
11: 41 56 push %r14
13: 4c 89 fa mov %r15,%rdx
16: 41 55 push %r13
18: 48 rex.W
19: c1 .byte 0xc1
1a: a4 movsb %ds:(%rsi),%es:(%rdi)
RSP: 0018:ffff888111857608 EFLAGS: 00010292
RAX: dffffc0000000000 RBX: 00000000000007d0 RCX: ffff8881330ece8e
RDX: 00000000000000fd RSI: 0000000000000001 RDI: 00000000000007d0
RBP: ffff888198ad8e00 R08: 0000000000000001 R09: ffff888198b800d8
R10: ffff888198b8019f R11: 0000000000000000 R12: ffff888198ad9008
R13: 0000000000000001 R14: ffff888198ad8e88 R15: 00000000000007e8
FS: 00007f831f921080(0000) GS:ffff888888405000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00005655646882e0 CR3: 000000014c52e000 CR4: 00000000007506f0
PKRU: 55555554
Kernel panic - not syncing: Fatal exception
Kernel Offset: 0x6000000 from 0xffffffff81000000 (relocation range:
0xffffffff80000000)
Thanks a lot!
Taehee Yoo
Powered by blists - more mailing lists