On Fri, Feb 16, 2024, Paul Durrant wrote:
On 16/02/2024 13:04, David Woodhouse wrote:
On Thu, 2024-02-15 at 15:29 +0000, Paul Durrant wrote:
From: David Woodhouse dwmw@amazon.co.uk
This function can race with kvm_gpc_deactivate(), which does not take the ->refresh_lock. This means kvm_gpc_deactivate() can wipe the ->pfn and ->khva fields, and unmap the latter, while hva_to_pfn_retry() has temporarily dropped its write lock on gpc->lock.
Let's drop this from your series for now, as it's contentious.
Sean didn't like calling it a 'fix', which I had conceded and reworked the commit message. It was on the list somewhere, and also in https://git.infradead.org/users/dwmw2/linux.git/commitdiff/f19755000a7
I *also* think we should do this simpler one: https://git.infradead.org/users/dwmw2/linux.git/commitdiff/cc69506d19a ... which almost makes the first one unnecessary, but I think we should do it *anyway* because the rwlock abuse it fixes is kind of awful.
And while we still can't actually *identify* the race condition that led to a dereference of a NULL gpc->khva while holding the read lock and gpc->valid and gpc->active both being true... I'll eat my hat if cleaning up and simplifying the locking (and making it self-contained) *doesn't* fix it.
Heh, I'm not taking that bet.
But either way, it isn't really part of your series. The only reason it was tacked on the end was because it would have merge conflicts with your series, which had been outstanding for months already.
So drop this one, and I'll work this bit out with Sean afterwards.
FWIW, I'm not opposed to overhauling the gpc locking, I agree it's a mess. I just want to proceed slower than I would for a fix, it's a lot to digest.
Ok. Sean, I assume that since this is the last patch in the series it's superfluous for me to post a v14 just for this?
Correct, definitely no need for a new version.