[PATCH] riscv: mm: Fix concurrency in mark_new_valid_map()

[Vivian Wang]

On 7/10/26 08:59, Paul Walmsley wrote:
> Hi Vivian,
>
> On Mon, 29 Jun 2026, Vivian Wang wrote:
>
>> Turns out, the concurrency concerns [1] were justified - BOSC reported a
>> spurious fault in KFENCE that still triggers despite previous fixes,
>> which KFENCE reports as a false-positive.
>>
>> Fix the concurrency problems in mark_new_valid_map():
>>
>> - Add smp_wmb() before filling the bitmap, to make sure page table
>>   writes are "received".
> Probably this expression should be clarified.  I guess the intent here is 
> to ensure that the page table write is visible by other cores either 
> before or simultaneously with the bitmap fill?  And the purpose of that is 
> to avoid a situation where another core could execute an sfence.vma before 
> the PTE write is observable by the other core?  If so, we should probably 
> write something like this out in the commit message.

Apologies for the mega delay. I had been focused on the other stuff.

Yes, This is the intention. I agree that this would probably need a
better wording.

> I wonder if there's still a window where spurious faults could be observed 
> by kfence.  If another core "B" faults (due to its local TLB caching an 
> invalid PTE) after the PTE write is observable by B, but before the bitmap 
> write is observable by B, core B won't sfence.vma.  If this possibility 
> still exists, we should probably document it.  It might be possible to 
> narrow that window further with a smp_mb() after the bitmap fill, but 
> unclear that it's worth trying unless more spurious faults are seen.
>   

Having thought about this more, I'm not sure how this should be handled.
The expected use case of this is some "allocation" function, at which
point the allocated pointer would be passed on to some other thread.

Here's the iffy part, *in general*, I *expect* that before the allocated
pointer or whatever becomes accessible in another thread, there's a
smp_wmb(), or an unlock-then-lock, or something like that. Otherwise,
none of the data being "sent through" this pointer wouldn't make much
sense, right. But this would have to be handled by every allocating
function, or similar, right? And I don't know if I'm able to practically
understand all the mm stuff to see if it is *actually* how it works.

> ...
>
> Also, now that we're adding an smp_wmb(), we should probably expand the 
> preemption-disabled section in kfence_protect_page() to cover the entire 
> function, to eliminate any risk that the PTE writes execute on a separate 
> CPU from the smp_wmb().

I'm not sure that this is a problem if the previous point wasn't. If the
thread is preempted, then migrated to a different CPU, then the thread
migration should have done the equivalent of a smp_mb(), right?
Something like:

        CPU a                    |      CPU b
                                 |
... thread 1 ...                 |
                                 |
[ Preemption starts here         |
                                 |
smp_mb()                         |
                                 |
Some store release in sched      |
to send the task to CPU b        |
                                 |      Some load acquire in sched to
                                 |      grab the task
                                 |
                                 |      smp_mb()
                                 |
                                 |      preemption finishes ]
                                 |
                                 |      ... thread 1 continues ...

Otherwise, even single-threaded program-order behavior wouldn't be
preserved. Therefore I believe it would still be safe even if the thread
was migrated after the smp_wmb().

As such, I do think preemption shouldn't be a problem, or should be
rolled into the other problem, namely, "wmb before sending a pointer to
another thread".

At my wit's end, again,
Vivian "dramforever" Wang