[PATCH] x86/sgx: Fix SRCU list traversal

[PATCH] x86/sgx: Fix SRCU list traversal

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

Replace list_for_each_entry_rcu() with list_for_each_entry_srcu()
when traversing the encl->mm_list protected by SRCU. This ensures
proper synchronization annotation and avoids potential lockdep
warnings about incorrect RCU usage.

The list is protected by encl->srcu, not RCU, so the SRCU-specific
iterator with srcu_read_lock_held() annotation is required.

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 arch/x86/kernel/cpu/sgx/encl.c | 12 ++++++++----
 arch/x86/kernel/cpu/sgx/main.c |  3 ++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index cf149b9..3c488a0 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -822,7 +822,8 @@ static struct sgx_encl_mm *sgx_encl_find_mm(struct sgx_encl *encl,
 
 	idx = srcu_read_lock(&encl->srcu);
 
-	list_for_each_entry_rcu(tmp, &encl->mm_list, list) {
+	list_for_each_entry_srcu(tmp, &encl->mm_list, list,
+			srcu_read_lock_held(&encl->srcu)) {
 		if (tmp->mm == mm) {
 			encl_mm = tmp;
 			break;
@@ -933,7 +934,8 @@ const cpumask_t *sgx_encl_cpumask(struct sgx_encl *encl)
 
 	idx = srcu_read_lock(&encl->srcu);
 
-	list_for_each_entry_rcu(encl_mm, &encl->mm_list, list) {
+	list_for_each_entry_srcu(encl_mm, &encl->mm_list, list,
+			srcu_read_lock_held(&encl->srcu)) {
 		if (!mmget_not_zero(encl_mm->mm))
 			continue;
 
@@ -1018,7 +1020,8 @@ static struct mem_cgroup *sgx_encl_get_mem_cgroup(struct sgx_encl *encl)
 	 */
 	idx = srcu_read_lock(&encl->srcu);
 
-	list_for_each_entry_rcu(encl_mm, &encl->mm_list, list) {
+	list_for_each_entry_srcu(encl_mm, &encl->mm_list, list,
+			srcu_read_lock_held(&encl->srcu)) {
 		if (!mmget_not_zero(encl_mm->mm))
 			continue;
 
@@ -1212,7 +1215,8 @@ void sgx_zap_enclave_ptes(struct sgx_encl *encl, unsigned long addr)
 
 		idx = srcu_read_lock(&encl->srcu);
 
-		list_for_each_entry_rcu(encl_mm, &encl->mm_list, list) {
+		list_for_each_entry_srcu(encl_mm, &encl->mm_list, list,
+				srcu_read_lock_held(&encl->srcu)) {
 			if (!mmget_not_zero(encl_mm->mm))
 				continue;
 
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index dc73194..ead0405 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -120,7 +120,8 @@ static bool sgx_reclaimer_age(struct sgx_epc_page *epc_page)
 
 	idx = srcu_read_lock(&encl->srcu);
 
-	list_for_each_entry_rcu(encl_mm, &encl->mm_list, list) {
+	list_for_each_entry_srcu(encl_mm, &encl->mm_list, list,
+			srcu_read_lock_held(&encl->srcu)) {
 		if (!mmget_not_zero(encl_mm->mm))
 			continue;
 
-- 
2.9.4

Re: [PATCH] x86/sgx: Fix SRCU list traversal

[Dave Hansen]

On 2/4/26 17:53, lirongqing wrote:
> Replace list_for_each_entry_rcu() with list_for_each_entry_srcu()
> when traversing the encl->mm_list protected by SRCU. This ensures
> proper synchronization annotation and avoids potential lockdep
> warnings about incorrect RCU usage.

Does lockdep trip on this today?

> The list is protected by encl->srcu, not RCU, so the SRCU-specific
> iterator with srcu_read_lock_held() annotation is required.

From a quick look, list_for_each_entry_rcu() still seems *really* common
under SRCU. It also looks like list_for_each_entry_srcu() is a
relatively recent (2020) addition to the kernel.

So, this wasn't a bug when the SGX code went in, but started causing a
problem at some point? Did lockdep add some RCU warnings or something
that made this necessary?

The patch seems logical and all. I just feel like I'm missing the bigger
picture.

答复: [外部邮件] Re: [PATCH] x86/sgx: Fix SRCU list traversal

[Li,Rongqing]

> On 2/4/26 17:53, lirongqing wrote:
> > Replace list_for_each_entry_rcu() with list_for_each_entry_srcu() when
> > traversing the encl->mm_list protected by SRCU. This ensures proper
> > synchronization annotation and avoids potential lockdep warnings about
> > incorrect RCU usage.
> 
> Does lockdep trip on this today?
> 
> > The list is protected by encl->srcu, not RCU, so the SRCU-specific
> > iterator with srcu_read_lock_held() annotation is required.
> 
> From a quick look, list_for_each_entry_rcu() still seems *really* common
> under SRCU. It also looks like list_for_each_entry_srcu() is a relatively recent
> (2020) addition to the kernel.
> 
> So, this wasn't a bug when the SGX code went in, but started causing a
> problem at some point? Did lockdep add some RCU warnings or something
> that made this necessary?
> 
> The patch seems logical and all. I just feel like I'm missing the bigger picture.

Seem this patch adds the check

commit 28875945ba98d1b47a8a706812b6494d165bb0a0
Author: Joel Fernandes (Google) <joel@joelfernandes.org>
Date:   Tue Jul 16 18:12:22 2019 -0400

    rcu: Add support for consolidated-RCU reader checking

    This commit adds RCU-reader checks to list_for_each_entry_rcu() and
    hlist_for_each_entry_rcu().  These checks are optional, and are indicated
    by a lockdep expression passed to a new optional argument to these two
    macros.  If this optional lockdep expression is omitted, these two macros
    act as before, checking for an RCU read-side critical section.

    Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
    [ paulmck: Update to eliminate return within macro and update comment. ]
    Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>


And there are several similar fixes:

d681107 nvme-multipath: fix suspicious RCU usage warning
5dd18f0 nvme/multipath: Fix RCU list traversal to use SRCU primitive
6d1c699 nvme/host: Fix RCU list traversal to use SRCU primitive
6a0c617 KVM: eventfd: Fix false positive RCU usage warning
df9a30f kvm: mmu: page_track: Fix RCU list API usage

[Li,Rongqing] 
 

Re: [PATCH] x86/sgx: Fix SRCU list traversal

[Huang, Kai]

On Wed, 2026-02-04 at 20:53 -0500, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> Replace list_for_each_entry_rcu() with list_for_each_entry_srcu()
> when traversing the encl->mm_list protected by SRCU. This ensures
> proper synchronization annotation and avoids potential lockdep
> warnings about incorrect RCU usage.
> 
> The list is protected by encl->srcu, not RCU, so the SRCU-specific
> iterator with srcu_read_lock_held() annotation is required.
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>

Acked-by: Kai Huang <kai.huang@intel.com>

答复: [外部邮件] Re: [PATCH] x86/sgx: Fix SRCU list traversal

[Li,Rongqing(ACG CCN)]

> > Replace list_for_each_entry_rcu() with list_for_each_entry_srcu() when
> > traversing the encl->mm_list protected by SRCU. This ensures proper
> > synchronization annotation and avoids potential lockdep warnings about
> > incorrect RCU usage.
> >
> > The list is protected by encl->srcu, not RCU, so the SRCU-specific
> > iterator with srcu_read_lock_held() annotation is required.
> >
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> 
> Acked-by: Kai Huang <kai.huang@intel.com>

Thanks for reviewing , and ping 

-Li

Re: 答复: [外部邮件] Re: [PATCH] x86/sgx: Fix SRCU list traversal

[Dave Hansen]

On 2/23/26 16:30, Li,Rongqing(ACG CCN) wrote:
>>> Replace list_for_each_entry_rcu() with list_for_each_entry_srcu() when
>>> traversing the encl->mm_list protected by SRCU. This ensures proper
>>> synchronization annotation and avoids potential lockdep warnings about
>>> incorrect RCU usage.
>>>
>>> The list is protected by encl->srcu, not RCU, so the SRCU-specific
>>> iterator with srcu_read_lock_held() annotation is required.
>>>
>>> Signed-off-by: Li RongQing <lirongqing@baidu.com>
>> Acked-by: Kai Huang <kai.huang@intel.com>
> Thanks for reviewing , and ping 

It's a light NAK from me with the current changelog.

I want a wee bit more background and some reasoning _somewhere_ about
why this wasn't a bug when the code went in originally but arguably
became a buglet at some point.

答复: 答复: [外部邮件] Re: [PATCH] x86/sgx: Fix SRCU list traversal

[Li,Rongqing(ACG CCN)]

> >>> The list is protected by encl->srcu, not RCU, so the SRCU-specific
> >>> iterator with srcu_read_lock_held() annotation is required.
> >>>
> >>> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> >> Acked-by: Kai Huang <kai.huang@intel.com>
> > Thanks for reviewing , and ping
> 
> It's a light NAK from me with the current changelog.
> 
> I want a wee bit more background and some reasoning _somewhere_ about
> why this wasn't a bug when the code went in originally but arguably became a
> buglet at some point.


The buggy commit is 1728ab54b4be ("x86/sgx: Add a page reclaimer") in v5.11, it should use list_for_each_entry_srcu()which is introduced in v5.10, so I rewrite the commit message as below, is it ok?



    x86/sgx: Use list_for_each_entry_srcu() for mm_list traversal

    In commit 1728ab54b4be ("x86/sgx: Add a page reclaimer") (v5.11),
    list_for_each_entry_rcu() was used to traverse the enclave's mm_list.
    However, this is incorrect because the list is protected by a Sleepable
    RCU (SRCU) lock (encl->srcu).

    Since commit 28875945ba98 ("rcu: Add support for consolidated-RCU reader
    checking") (v5.4), RCU lockdep checking has become stricter. When
    CONFIG_PROVE_RCU is enabled, using the standard list_for_each_entry_rcu()
    while only holding an SRCU lock triggers "suspicious RCU usage" false
    positive warnings, as it does not recognize SRCU read-side critical
    sections.

    Fix this by switching to list_for_each_entry_srcu(), which was
    introduced specifically for this purpose in commit ae2212a7216
    ("rculist: Introduce list/hlist_for_each_entry_srcu() macros") (v5.10).
    This correctly associates the traversal with the SRCU lock and
    eliminates the lockdep warnings.

    Fixes: 1728ab54b4be ("x86/sgx: Add a page reclaimer")
    Signed-off-by: Li RongQing <lirongqing@baidu.com>