netdev - Re: [PATCH net 0/4][pull request] igb: fix igb_msix_other() handling for PREEMPT

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250205094818.I-Jl44AK@linutronix.de>
Date: Wed, 5 Feb 2025 10:48:18 +0100
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: Tony Nguyen <anthony.l.nguyen@...el.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
	edumazet@...gle.com, andrew+netdev@...n.ch, netdev@...r.kernel.org,
	wander@...hat.com, rostedt@...dmis.org, clrkwllms@...nel.org,
	jgarzik@...hat.com, yuma@...hat.com, linux-rt-devel@...ts.linux.dev
Subject: Re: [PATCH net 0/4][pull request] igb: fix igb_msix_other() handling
 for PREEMPT_RT

On 2025-02-04 09:52:36 [-0800], Tony Nguyen wrote:
> Wander Lairson Costa says:
> 
> This is the second attempt at fixing the behavior of igb_msix_other()
> for PREEMPT_RT. The previous attempt [1] was reverted [2] following
> concerns raised by Sebastian [3].

I still prefer a solution where we don't have the ifdef in the driver. I
was presented two traces but I didn't get why it works in once case but
not in the other. Maybe it was too obvious.
In the mean time:

igb_msg_task_irq_safe()
-> vfs_raw_spin_lock_irqsave() // raw_spinlock_t
-> igb_vf_reset_event()
  -> igb_vf_reset()
    -> igb_set_rx_mode()
      -> igb_write_mc_addr_list()
         -> mta_list = kcalloc(netdev_mc_count(netdev), 6, GFP_ATOMIC); // kaboom?

By explicitly disabling preemption or using a raw_spinlock_t you need to
pay attention not to do anything that might lead to unbounded loops
(like iterating over many lists, polling on a bit for ages, …) and
paying attention that the whole API underneath that it is not doing that
is allowed to.

Sebastian