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
| ||
|
Date: Thu, 18 Oct 2018 15:56:47 -0700 (PDT) From: David Miller <davem@...emloft.net> To: sam@...dozajonas.com Cc: netdev@...r.kernel.org, Justin.Lee1@...l.com, linux-kernel@...r.kernel.org, openbmc@...ts.ozlabs.org Subject: Re: [PATCH net-next 0/6] net/ncsi: Allow enabling multiple packages & channels From: Samuel Mendoza-Jonas <sam@...dozajonas.com> Date: Thu, 18 Oct 2018 14:59:11 +1100 > This series extends the NCSI driver to configure multiple packages > and/or channels simultaneously. Since the RFC series this includes a few > extra changes to fix areas in the driver that either made this harder or > were roadblocks due to deviations from the NCSI specification. > > Patches 1 & 2 fix two issues where the driver made assumptions about the > capabilities of the NCSI topology. > Patches 3 & 4 change some internal semantics slightly to make multi-mode > easier. > Patch 5 introduces a cleaner way of reconfiguring the NCSI configuration > and keeping track of channel states. > Patch 6 implements the main multi-package/multi-channel configuration, > configured via the Netlink interface. > > Readers who have an interesting NCSI setup - especially multi-package > with HWA - please test! I think I've covered all permutations but I > don't have infinite hardware to test on. This doesn't apply cleanly to net-next. Does it depend upon changes applied elsewhere? You must always make that explicit. Also, please explain this locking in ncsi_reset_dev(): + NCSI_FOR_EACH_PACKAGE(ndp, np) { + NCSI_FOR_EACH_CHANNEL(np, nc) { + spin_lock_irqsave(&nc->lock, flags); + enabled = nc->monitor.enabled; + state = nc->state; + spin_unlock_irqrestore(&nc->lock, flags); + + if (enabled) + ncsi_stop_channel_monitor(nc); + if (state == NCSI_CHANNEL_ACTIVE) { + active = nc; + break; + } Is that really protecting anything? Right after you drop np->lock those two values can change, the state of the 'nc' can change such that it isn't NCSI_CHANNEL_ACTIVE anymore etc. At best this locking makes sure thatn enabled and state are consistent with respect to eachother, only. It doesn't guarantee anything about the stability of the state of the object at all, and it can change right from under you.
Powered by blists - more mailing lists