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: <20140908140324.GE31377@oracle.com> Date: Mon, 8 Sep 2014 10:03:24 -0400 From: Sowmini Varadhan <sowmini.varadhan@...cle.com> To: David L Stevens <david.stevens@...cle.com> Cc: David Miller <davem@...emloft.net>, netdev@...r.kernel.org, raghuram.kothakota@...cle.com Subject: Re: [PATCH net-next v2 2/2] Re-check for a VIO_DESC_READY data descriptor after short udelay() On (09/08/14 09:45), David L Stevens wrote: First off, please don't drop email ids from the original email. I've re-added Raghuram Kothakota to fix that initial omission. Second, this thread originally came up with your suggestion that we shold move the wmb() down one line. I looked at this, and consulted a few folks, all of whom agree that that is probably incorrect. As Bob Picco pointed out to me: " Placing the wmb() after probably won't have any negative consequences BUT could. Suppose the compiler reorders the stores and the d->hdr.state store occurs before another part of the descriptor. We HV trap, and the trap doesn't flush the store buffer. The consumer could then see VIO_DESC_READY before the cookies arrray is updated. It would depend on how many cache lines are spanned by vio_net_desc and the store buffer organization." thus I would be cautious about doing that. > I'm no mb expert, and I know of no symptoms, but it appears to be saying that > load reordering could result in a race where the READY flag could be set with > old data in other descriptor fields due to loading them in a different order -- > something it says wmb() on another CPU explicitly does not prevent. Please see the explanation that I, and Raghuram offered. The wmb() assures that the stores of the cookie state are ordered correctly by the consumer. The next store is for VIO_DESC_READY and the consumer will not proceed to read cookie state until this is visible. Thus no rmb() is needed. On its side, the consumer resets the hdr.state to DONE after it consumes the cookies, and the DRING_STOPPLED announcement further informs the producer that the descriptor is available. Hope that helps. If you think there is some sequence where this is insufficient, please explain with details. > > The particular case would be adding to the ring at the same time the > other side > is removing from the ring, so no locks or LDC traffic would affect it. > > So, it appears to me we have a missing rmb() that is needed and I > don't know what > leads you to believe it isn't. > > +-DLS -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists