On Wed, Jan 19, 2022 at 03:28:32PM -0800, Peter Collingbourne wrote:
On Wed, Jan 19, 2022 at 2:03 AM Peter Zijlstra peterz@infradead.org wrote:
On Tue, Jan 18, 2022 at 03:05:39PM -0800, Peter Collingbourne wrote:
After submitting a patch with a compare-exchange loop similar to this one to set the KASAN tag in the page flags, Andrey Konovalov pointed out that we should be using READ_ONCE() to read the page flags. Fix it here.
What does it actually fix? If it manages to split the read and read garbage the cmpxchg will fail and we go another round, no harm done.
What I wasn't sure about was whether the compiler would be allowed to break this code by hoisting the read of page->flags out of the loop (because nothing in the loop actually writes to page->flags aside from the compare-exchange, and if that succeeds we're *leaving* the loop).
The cmpxchg is a barrier() and as such I don't think it's allowed to hoist anything out of the loop. Except perhaps since it's do-while, it could try and unroll the first iteration and wreck that something fierce.
The bigger problem is I think that page_cpuid_last() usage which does a second load of page->flags, and given sufficient races that could actually load a different value and then things would be screwy. But that's not actually fixed.
Signed-off-by: Peter Collingbourne pcc@google.com Link: https://linux-review.googlesource.com/id/I2e1f5b5b080ac9c4e0eb7f98768dba6fd7...
That's that doing here?
I upload my changes to Gerrit and link to them here so that I (and others) can see the progression of the patch via the web UI.
What's the life-time guarantee for that URL existing? Because if it becomes part of the git commit, it had better stay around 'forever' etc..