[PATCH] mm: bypass datarace check

[PATCH] mm: bypass datarace check

[Guilherme Giacomo Simoes]

Despiste kcsan point to a possible race condition problem, this is a
safe race condition due the access memory ordering, since
spin_lock(&mm->page_table_lock) have ACQUIRE semantics and ensure the
ordering mapping.

Create a new function called vma_is_faulted(), that return a
data_race(vma->anon_vma) to bypass kcsan

Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
---
 mm/memory.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 6b8280cfc1db..d85d67927400 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3800,6 +3800,25 @@ static inline vm_fault_t vmf_can_call_fault(const struct vm_fault *vmf)
 	return VM_FAULT_RETRY;
 }
 
+/**
+ * vma_is_faulted - check if a vma has been faulted
+ * @vma: the vma to check
+ *
+ * This is a lockless access that may race with __anon_vma_prepare().
+ * The race is safe because:
+ * - The fault handler ensures that the mapping of memory is ordered.
+ * - If we read NULL, the caller will re-check
+ * - The page_table_lock provides ACQUIRE semantics for memory ordering
+ *
+ * Return: true if vma->anon_vma is non-NULL, false otherwise
+ */
+static inline bool vma_is_faulted(const struct vm_area_struct *vma)
+{
+	/* Lockless check - safe because we re-validate under page_table_lock */
+	return data_race(vma->anon_vma);
+}
+
+
 /**
  * __vmf_anon_prepare - Prepare to handle an anonymous fault.
  * @vmf: The vm_fault descriptor passed from the fault handler.
@@ -3819,8 +3838,7 @@ vm_fault_t __vmf_anon_prepare(struct vm_fault *vmf)
 {
 	struct vm_area_struct *vma = vmf->vma;
 	vm_fault_t ret = 0;
-
-	if (likely(vma->anon_vma))
+	if (likely(vma_is_faulted(vma)))
 		return 0;
 	if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
 		if (!mmap_read_trylock(vma->vm_mm))
-- 
2.52.0

Re: [PATCH] mm: bypass datarace check

[Lorenzo Stoakes (ARM)]

I started review below but honestly this patch is confused in multiple ways
and it's not entirely clear you really understand what's going on here.

It's also basically implementing what we suggested.

So at this point I think it's easier if I send the patch with a:

Reported-by:
Closes:

tag -> you, this patch.

Thanks!

On Wed, Sep 09, 2026 at 08:57:23AM -0300, Guilherme Giacomo Simoes wrote:
> Despiste kcsan point to a possible race condition problem, this is a

Typos  -> Despite, point -> points

> safe race condition due the access memory ordering, since
> spin_lock(&mm->page_table_lock) have ACQUIRE semantics and ensure the
> ordering mapping.

This sentence is a bit confused. Acquire semantics mean absolutely nothing
unless paired with another operation and etc. etc.

>
> Create a new function called vma_is_faulted(), that return a
> data_race(vma->anon_vma) to bypass kcsan

This sentence reads incomplete? Or missing full stop?

>

Needs a:

Suggested-by: Pedro Falcato <pfalcato@suse.de>

Also:

Assisted-by: LLM?

The list below reads very LLM-ish so I have to ask did you use one etc. etc.

https://docs.kernel.org/process/coding-assistants.html

Perhaps given I am suggesting a lot here a:


> Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
> ---
>  mm/memory.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 6b8280cfc1db..d85d67927400 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3800,6 +3800,25 @@ static inline vm_fault_t vmf_can_call_fault(const struct vm_fault *vmf)
>  	return VM_FAULT_RETRY;
>  }
>
> +/**
> + * vma_is_faulted - check if a vma has been faulted

Would rather 'has @vma been faulted in?'

> + * @vma: the vma to check
> + *
> + * This is a lockless access that may race with __anon_vma_prepare().

This doesn't belong here. In fact it doesn't belong at all IMO.

> + * The race is safe because:

There isn't a race at all it's KCSAN getting confused because it can't prove
that there isn't.

> + * - The fault handler ensures that the mapping of memory is ordered.

What has that got to do with this function?

> + * - If we read NULL, the caller will re-check

Umm, no? This function doesn't force callers to re-check?

> + * - The page_table_lock provides ACQUIRE semantics for memory ordering

I'm not sure how this relates to anything but again this doesn't really belong
here.

Anyway let's drop all of this please.


> + *
> + * Return: true if vma->anon_vma is non-NULL, false otherwise

This is completely breaking the abstraction.

'true if the VMA is faulted in, otherwise false.'

> + */
> +static inline bool vma_is_faulted(const struct vm_area_struct *vma)

Why in memory.c and why inline if it's in a .c file? This belongs in mm.h with other such helpers.

> +{
> +	/* Lockless check - safe because we re-validate under page_table_lock */

You don't need a comment saying accessing a field esp. with data_race() is
lockless, that's implied...

Maybe:

	/* Benign, see __anon_vma_prepare(). */

> +	return data_race(vma->anon_vma);
> +}
> +
> +
>  /**
>   * __vmf_anon_prepare - Prepare to handle an anonymous fault.
>   * @vmf: The vm_fault descriptor passed from the fault handler.
> @@ -3819,8 +3838,7 @@ vm_fault_t __vmf_anon_prepare(struct vm_fault *vmf)
>  {
>  	struct vm_area_struct *vma = vmf->vma;
>  	vm_fault_t ret = 0;
> -
> -	if (likely(vma->anon_vma))
> +	if (likely(vma_is_faulted(vma)))
>  		return 0;

There are other places where this check is done and etc.

>  	if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
>  		if (!mmap_read_trylock(vma->vm_mm))
> --
> 2.52.0
>

--
Cheers, Lorenzo

Re: [PATCH] mm: bypass datarace check

[Guilherme Giacomo Simoes]

"Lorenzo Stoakes (ARM)" <ljs@kernel.org> wrote:
> I started review below but honestly this patch is confused in multiple ways
> and it's not entirely clear you really understand what's going on here.
I can be wrong, but was understand that due the order that the code was write
probably the data race problem will not happen.

The reader (__vmf_anon_prepare()):
```
if (likely(vma->anon_vma)) // lockless check
	return 0;              // OK

// if the check above fail

if (!__anon_vma_prepare(vma)) // called the __anon_vma_prepare
    return 0;

```

inside __anon_vma_prepare()
```
spin_lock(&mm->page_table_lock); //ACQUIRE semantics

if (likely(!vma->anon_vma)) // re-check under lock
    // ... alloc all

spin_unlock(&mm->page_table_lock);
```

This is safe because, if `if (likely(vma->anon_vma))` return NULL, we will got
the mmap_lock and then page_table_lock.

The critical re-check inside __anon_vma_prepare() happens under spin_lock(...)
with has ACQUIRE semantics.
With ACQUIRE semantics , the cpu (or compiler, I don't know) cannot reorder the
memory access acress the lock boundary.

I'm right?

> 
> It's also basically implementing what we suggested.
> 
> So at this point I think it's easier if I send the patch with a:
> 
> Reported-by:
> Closes:
> 
> tag -> you, this patch.
> 
> Thanks!
ok, no problem

> On Wed, Sep 09, 2026 at 08:57:23AM -0300, Guilherme Giacomo Simoes wrote:
> > Despiste kcsan point to a possible race condition problem, this is a
> 
> Typos  -> Despite, point -> points
Hmm, is not the first time that any person points my english mistakes... I will
improve this point, thank you for yout jints

> > safe race condition due the access memory ordering, since
> > spin_lock(&mm->page_table_lock) have ACQUIRE semantics and ensure the
> > ordering mapping.
> 
> This sentence is a bit confused. Acquire semantics mean absolutely nothing
> unless paired with another operation and etc. etc.
missing full stop, my bad.

> Needs a:
> 
> Suggested-by: Pedro Falcato <pfalcato@suse.de>
Yeah, I forget

> 
> Also:
> 
> Assisted-by: LLM?
> The list below reads very LLM-ish so I have to ask did you use one etc. etc.
> 
> https://docs.kernel.org/process/coding-assistants.html
> 
> Perhaps given I am suggesting a lot here a:
I don't have installed any llm (not even cursor), I just use a deepseek,
chatgpt, etc.. to clear up a few questions. (maybe I should start use this to
help me with english too)

> There are other places where this check is done and etc.
I would should checked this, sorry. Anxiety.

Thanks Lorenzo for your review, help and patience 

Re: [PATCH] mm: bypass datarace check

[Lorenzo Stoakes (ARM)]

On Wed, Sep 09, 2026 at 06:29:43PM -0300, Guilherme Giacomo Simoes wrote:
> "Lorenzo Stoakes (ARM)" <ljs@kernel.org> wrote:
> > I started review below but honestly this patch is confused in multiple ways
> > and it's not entirely clear you really understand what's going on here.
> I can be wrong, but was understand that due the order that the code was write
> probably the data race problem will not happen.
>
> The reader (__vmf_anon_prepare()):
> ```
> if (likely(vma->anon_vma)) // lockless check
> 	return 0;              // OK
>
> // if the check above fail
>
> if (!__anon_vma_prepare(vma)) // called the __anon_vma_prepare
>     return 0;
>
> ```
>
> inside __anon_vma_prepare()
> ```
> spin_lock(&mm->page_table_lock); //ACQUIRE semantics
>
> if (likely(!vma->anon_vma)) // re-check under lock
>     // ... alloc all
>
> spin_unlock(&mm->page_table_lock);
> ```
>
> This is safe because, if `if (likely(vma->anon_vma))` return NULL, we will got
> the mmap_lock and then page_table_lock.

I mean yes but it's complicated (the anon rmap is like this all over, it's
complicated for _everybody_ which is part of why I am working to change it).

There are 2 cases basically for _attached_ VMAS - mmap/vma write lock held
(you are the only thread that has access to the vma by definition) or mmap read
lock held in which case it's an optimistic check that must be re-checked with
mm->page_table_lock held to get exclusivity.

And it turns out that _all of mm_ screwed up some aspect of this also see:

https://lore.kernel.org/all/20260908122924.554373-1-tujinjiang@huawei.com/

>
> The critical re-check inside __anon_vma_prepare() happens under spin_lock(...)
> with has ACQUIRE semantics.
> With ACQUIRE semantics , the cpu (or compiler, I don't know) cannot reorder the
> memory access acress the lock boundary.
>
> I'm right?

Well the issue with acquire/release semantics is that instructions that are
outside of a critical section can be re-ordered within the critical section.

See my analysis here:

https://lore.kernel.org/all/ap6ybQeSg_rrmC95@gremlin/

(Again we were all confused about it! Memory barriers are very counterintuitive)

>
> >
> > It's also basically implementing what we suggested.
> >
> > So at this point I think it's easier if I send the patch with a:
> >
> > Reported-by:
> > Closes:
> >
> > tag -> you, this patch.
> >
> > Thanks!
> ok, no problem

Thanks, sorry about that but I feel in general, it's super sensitive and
confusing this and it's the best way in this case.

>
> > On Wed, Sep 09, 2026 at 08:57:23AM -0300, Guilherme Giacomo Simoes wrote:
> > > Despiste kcsan point to a possible race condition problem, this is a
> >
> > Typos  -> Despite, point -> points
> Hmm, is not the first time that any person points my english mistakes... I will
> improve this point, thank you for yout jints

No worries, I make typos all the time and have no excuses for it :)

>
> > > safe race condition due the access memory ordering, since
> > > spin_lock(&mm->page_table_lock) have ACQUIRE semantics and ensure the
> > > ordering mapping.
> >
> > This sentence is a bit confused. Acquire semantics mean absolutely nothing
> > unless paired with another operation and etc. etc.
> missing full stop, my bad.
>
> > Needs a:
> >
> > Suggested-by: Pedro Falcato <pfalcato@suse.de>
> Yeah, I forget
>
> >
> > Also:
> >
> > Assisted-by: LLM?
> > The list below reads very LLM-ish so I have to ask did you use one etc. etc.
> >
> > https://docs.kernel.org/process/coding-assistants.html
> >
> > Perhaps given I am suggesting a lot here a:
> I don't have installed any llm (not even cursor), I just use a deepseek,
> chatgpt, etc.. to clear up a few questions. (maybe I should start use this to
> help me with english too)

Ah sorry, there's such a wave of it and the list seemed that way, I guess
because you had it help on the language it flagged it up :)

>
> > There are other places where this check is done and etc.
> I would should checked this, sorry. Anxiety.

Understandable, this is delicate stuff!

>
> Thanks Lorenzo for your review, help and patience

No worries! :)


--
Cheers, Lorenzo