[PATCH] mm: list_lru: fix UAF for memory cgroup

[PATCH] mm: list_lru: fix UAF for memory cgroup

[Muchun Song]

The mem_cgroup_from_slab_obj() is supposed to be called under rcu
lock or cgroup_mutex or others which could prevent returned memcg
from being freed. Fix it by adding missing rcu read lock.

Fixes: 0a97c01cd20bb ("list_lru: allow explicit memcg and NUMA node selection)
Signed-off-by: Muchun Song <songmuchun@bytedance.com>
---
 mm/list_lru.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/mm/list_lru.c b/mm/list_lru.c
index 3fd64736bc458..225da0778a3be 100644
--- a/mm/list_lru.c
+++ b/mm/list_lru.c
@@ -85,6 +85,7 @@ list_lru_from_memcg_idx(struct list_lru *lru, int nid, int idx)
 }
 #endif /* CONFIG_MEMCG_KMEM */
 
+/* The caller must ensure the memcg lifetime. */
 bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
 		    struct mem_cgroup *memcg)
 {
@@ -109,14 +110,20 @@ EXPORT_SYMBOL_GPL(list_lru_add);
 
 bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
 {
+	bool ret;
 	int nid = page_to_nid(virt_to_page(item));
-	struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
-		mem_cgroup_from_slab_obj(item) : NULL;
+	struct mem_cgroup *memcg;
 
-	return list_lru_add(lru, item, nid, memcg);
+	rcu_read_lock();
+	memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL;
+	ret = list_lru_add(lru, item, nid, memcg);
+	rcu_read_unlock();
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(list_lru_add_obj);
 
+/* The caller must ensure the memcg lifetime. */
 bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid,
 		    struct mem_cgroup *memcg)
 {
@@ -139,11 +146,16 @@ EXPORT_SYMBOL_GPL(list_lru_del);
 
 bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
 {
+	bool ret;
 	int nid = page_to_nid(virt_to_page(item));
-	struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
-		mem_cgroup_from_slab_obj(item) : NULL;
+	struct mem_cgroup *memcg;
 
-	return list_lru_del(lru, item, nid, memcg);
+	rcu_read_lock();
+	memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL;
+	ret = list_lru_del(lru, item, nid, memcg);
+	rcu_read_unlock();
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(list_lru_del_obj);
 
-- 
2.20.1

Re: [PATCH] mm: list_lru: fix UAF for memory cgroup

[Vlastimil Babka]

On 7/18/24 10:36 AM, Muchun Song wrote:
> The mem_cgroup_from_slab_obj() is supposed to be called under rcu
> lock or cgroup_mutex or others which could prevent returned memcg
> from being freed. Fix it by adding missing rcu read lock.

Was the UAF ever observed, or is this due to code review?
Should there be some lockdep_assert somwhere?

> Fixes: 0a97c01cd20bb ("list_lru: allow explicit memcg and NUMA node selection)
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  mm/list_lru.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index 3fd64736bc458..225da0778a3be 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -85,6 +85,7 @@ list_lru_from_memcg_idx(struct list_lru *lru, int nid, int idx)
>  }
>  #endif /* CONFIG_MEMCG_KMEM */
>  
> +/* The caller must ensure the memcg lifetime. */
>  bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
>  		    struct mem_cgroup *memcg)
>  {
> @@ -109,14 +110,20 @@ EXPORT_SYMBOL_GPL(list_lru_add);
>  
>  bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
>  {
> +	bool ret;
>  	int nid = page_to_nid(virt_to_page(item));
> -	struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
> -		mem_cgroup_from_slab_obj(item) : NULL;
> +	struct mem_cgroup *memcg;
>  
> -	return list_lru_add(lru, item, nid, memcg);
> +	rcu_read_lock();
> +	memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL;
> +	ret = list_lru_add(lru, item, nid, memcg);
> +	rcu_read_unlock();
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(list_lru_add_obj);
>  
> +/* The caller must ensure the memcg lifetime. */
>  bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid,
>  		    struct mem_cgroup *memcg)
>  {
> @@ -139,11 +146,16 @@ EXPORT_SYMBOL_GPL(list_lru_del);
>  
>  bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
>  {
> +	bool ret;
>  	int nid = page_to_nid(virt_to_page(item));
> -	struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
> -		mem_cgroup_from_slab_obj(item) : NULL;
> +	struct mem_cgroup *memcg;
>  
> -	return list_lru_del(lru, item, nid, memcg);
> +	rcu_read_lock();
> +	memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL;
> +	ret = list_lru_del(lru, item, nid, memcg);
> +	rcu_read_unlock();
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(list_lru_del_obj);
>  

Re: [PATCH] mm: list_lru: fix UAF for memory cgroup

[Muchun Song]

> On Jul 18, 2024, at 18:30, Vlastimil Babka <vbabka@suse.cz> wrote:
> 
> On 7/18/24 10:36 AM, Muchun Song wrote:
>> The mem_cgroup_from_slab_obj() is supposed to be called under rcu
>> lock or cgroup_mutex or others which could prevent returned memcg
>> from being freed. Fix it by adding missing rcu read lock.
> 
> Was the UAF ever observed, or is this due to code review?

Just code review.

Thanks.

> Should there be some lockdep_assert somwhere?
> 

It’s a good option to improve this. Maybe mem_cgroup_from_slab_obj() is a good place.

>> Fixes: 0a97c01cd20bb ("list_lru: allow explicit memcg and NUMA node selection)
>> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>> ---
>> mm/list_lru.c | 24 ++++++++++++++++++------
>> 1 file changed, 18 insertions(+), 6 deletions(-)
>> 
>> diff --git a/mm/list_lru.c b/mm/list_lru.c
>> index 3fd64736bc458..225da0778a3be 100644
>> --- a/mm/list_lru.c
>> +++ b/mm/list_lru.c
>> @@ -85,6 +85,7 @@ list_lru_from_memcg_idx(struct list_lru *lru, int nid, int idx)
>> }
>> #endif /* CONFIG_MEMCG_KMEM */
>> 
>> +/* The caller must ensure the memcg lifetime. */
>> bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
>>            struct mem_cgroup *memcg)
>> {
>> @@ -109,14 +110,20 @@ EXPORT_SYMBOL_GPL(list_lru_add);
>> 
>> bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
>> {
>> +    bool ret;
>>    int nid = page_to_nid(virt_to_page(item));
>> -    struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
>> -        mem_cgroup_from_slab_obj(item) : NULL;
>> +    struct mem_cgroup *memcg;
>> 
>> -    return list_lru_add(lru, item, nid, memcg);
>> +    rcu_read_lock();
>> +    memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL;
>> +    ret = list_lru_add(lru, item, nid, memcg);
>> +    rcu_read_unlock();
>> +
>> +    return ret;
>> }
>> EXPORT_SYMBOL_GPL(list_lru_add_obj);
>> 
>> +/* The caller must ensure the memcg lifetime. */
>> bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid,
>>            struct mem_cgroup *memcg)
>> {
>> @@ -139,11 +146,16 @@ EXPORT_SYMBOL_GPL(list_lru_del);
>> 
>> bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
>> {
>> +    bool ret;
>>    int nid = page_to_nid(virt_to_page(item));
>> -    struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
>> -        mem_cgroup_from_slab_obj(item) : NULL;
>> +    struct mem_cgroup *memcg;
>> 
>> -    return list_lru_del(lru, item, nid, memcg);
>> +    rcu_read_lock();
>> +    memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL;
>> +    ret = list_lru_del(lru, item, nid, memcg);
>> +    rcu_read_unlock();
>> +
>> +    return ret;
>> }
>> EXPORT_SYMBOL_GPL(list_lru_del_obj);
>> 
> 

Re: [PATCH] mm: list_lru: fix UAF for memory cgroup

[Muchun Song]

> On Jul 18, 2024, at 19:20, Muchun Song <muchun.song@linux.dev> wrote:
> 
> 
> 
>> On Jul 18, 2024, at 18:30, Vlastimil Babka <vbabka@suse.cz> wrote:
>> 
>> On 7/18/24 10:36 AM, Muchun Song wrote:
>>> The mem_cgroup_from_slab_obj() is supposed to be called under rcu
>>> lock or cgroup_mutex or others which could prevent returned memcg
>>> from being freed. Fix it by adding missing rcu read lock.
>> 
>> Was the UAF ever observed, or is this due to code review?
> 
> Just code review.
> 
> Thanks.
> 
>> Should there be some lockdep_assert somwhere?
>> 
> 
> It’s a good option to improve this. Maybe mem_cgroup_from_slab_obj() is a good place.

I added it to obj_cgroup_memcg() [1]. And CC memory cgroup maintainers to this thread.


[1] https://lore.kernel.org/linux-mm/20859F67-A80C-4FD0-990C-40C70905E55B@linux.dev/T/

> 
>>> Fixes: 0a97c01cd20bb ("list_lru: allow explicit memcg and NUMA node selection)
>>> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>>> ---
>>> mm/list_lru.c | 24 ++++++++++++++++++------
>>> 1 file changed, 18 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/mm/list_lru.c b/mm/list_lru.c
>>> index 3fd64736bc458..225da0778a3be 100644
>>> --- a/mm/list_lru.c
>>> +++ b/mm/list_lru.c
>>> @@ -85,6 +85,7 @@ list_lru_from_memcg_idx(struct list_lru *lru, int nid, int idx)
>>> }
>>> #endif /* CONFIG_MEMCG_KMEM */
>>> 
>>> +/* The caller must ensure the memcg lifetime. */
>>> bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
>>>           struct mem_cgroup *memcg)
>>> {
>>> @@ -109,14 +110,20 @@ EXPORT_SYMBOL_GPL(list_lru_add);
>>> 
>>> bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
>>> {
>>> +    bool ret;
>>>   int nid = page_to_nid(virt_to_page(item));
>>> -    struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
>>> -        mem_cgroup_from_slab_obj(item) : NULL;
>>> +    struct mem_cgroup *memcg;
>>> 
>>> -    return list_lru_add(lru, item, nid, memcg);
>>> +    rcu_read_lock();
>>> +    memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL;
>>> +    ret = list_lru_add(lru, item, nid, memcg);
>>> +    rcu_read_unlock();
>>> +
>>> +    return ret;
>>> }
>>> EXPORT_SYMBOL_GPL(list_lru_add_obj);
>>> 
>>> +/* The caller must ensure the memcg lifetime. */
>>> bool list_lru_del(struct list_lru *lru, struct list_head *item, int nid,
>>>           struct mem_cgroup *memcg)
>>> {
>>> @@ -139,11 +146,16 @@ EXPORT_SYMBOL_GPL(list_lru_del);
>>> 
>>> bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
>>> {
>>> +    bool ret;
>>>   int nid = page_to_nid(virt_to_page(item));
>>> -    struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
>>> -        mem_cgroup_from_slab_obj(item) : NULL;
>>> +    struct mem_cgroup *memcg;
>>> 
>>> -    return list_lru_del(lru, item, nid, memcg);
>>> +    rcu_read_lock();
>>> +    memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL;
>>> +    ret = list_lru_del(lru, item, nid, memcg);
>>> +    rcu_read_unlock();
>>> +
>>> +    return ret;
>>> }
>>> EXPORT_SYMBOL_GPL(list_lru_del_obj);
>>> 
>> 

Re: [PATCH] mm: list_lru: fix UAF for memory cgroup

[Shakeel Butt]

On Thu, Jul 18, 2024 at 04:36:07PM GMT, Muchun Song wrote:
> The mem_cgroup_from_slab_obj() is supposed to be called under rcu
> lock or cgroup_mutex or others which could prevent returned memcg
> from being freed. Fix it by adding missing rcu read lock.
> 
> Fixes: 0a97c01cd20bb ("list_lru: allow explicit memcg and NUMA node selection)
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>

Yup I noticed these as well while reviewing Kairui's patches.

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>

Re: [PATCH] mm: list_lru: fix UAF for memory cgroup

[Andrew Morton]

On Thu, 18 Jul 2024 16:36:07 +0800 Muchun Song <songmuchun@bytedance.com> wrote:

> The mem_cgroup_from_slab_obj() is supposed to be called under rcu
> lock or cgroup_mutex or others which could prevent returned memcg
> from being freed. Fix it by adding missing rcu read lock.

"or others" is rather vague.  What others?

> @@ -109,14 +110,20 @@ EXPORT_SYMBOL_GPL(list_lru_add);
>  
>  bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
>  {
> +	bool ret;
>  	int nid = page_to_nid(virt_to_page(item));
> -	struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
> -		mem_cgroup_from_slab_obj(item) : NULL;
> +	struct mem_cgroup *memcg;
>  
> -	return list_lru_add(lru, item, nid, memcg);
> +	rcu_read_lock();
> +	memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL;
> +	ret = list_lru_add(lru, item, nid, memcg);
> +	rcu_read_unlock();

We don't need rcu_read_lock() to evaluate NULL.

	memcg = NULL;
	if (list_lru_memcg_aware(lru)) {
		rcu_read_lock();
		memcg = mem_cgroup_from_slab_obj(item);
		rcu_read_unlock();
	}

Seems worthwhile?

Re: [PATCH] mm: list_lru: fix UAF for memory cgroup

[Muchun Song]

> On Jul 24, 2024, at 08:45, Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> On Thu, 18 Jul 2024 16:36:07 +0800 Muchun Song <songmuchun@bytedance.com> wrote:
> 
>> The mem_cgroup_from_slab_obj() is supposed to be called under rcu
>> lock or cgroup_mutex or others which could prevent returned memcg
>> from being freed. Fix it by adding missing rcu read lock.
> 
> "or others" is rather vague.  What others?

Like objcg_lock. I have added this one into obj_cgroup_memcg().

> 
>> @@ -109,14 +110,20 @@ EXPORT_SYMBOL_GPL(list_lru_add);
>> 
>> bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
>> {
>> + 	bool ret;
>> 	int nid = page_to_nid(virt_to_page(item));
>> - 	struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
>> - 	mem_cgroup_from_slab_obj(item) : NULL;
>> + 	struct mem_cgroup *memcg;
>> 
>> - 	return list_lru_add(lru, item, nid, memcg);
>> + 	rcu_read_lock();
>> + 	memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL;
>> + 	ret = list_lru_add(lru, item, nid, memcg);
>> + 	rcu_read_unlock();
> 
> We don't need rcu_read_lock() to evaluate NULL.
> 
> 	memcg = NULL;
> 	if (list_lru_memcg_aware(lru)) {
> 		rcu_read_lock();
> 		memcg = mem_cgroup_from_slab_obj(item);
> 		rcu_read_unlock();

Actually, the access to memcg is in list_lru_add(), so the rcu lock should
also cover this function rather than only mem_cgroup_from_slab_obj().
Something like:

memcg = NULL;
if (list_lru_memcg_aware(lru)) {
	rcu_read_lock();
	memcg = mem_cgroup_from_slab_obj(item);
}
ret = list_lru_add(lru, item, nid, memcg);
if (list_lru_memcg_aware(lru))
	rcu_read_unlock();

Not concise. I don't know if this is good.

> 	}
> 
> Seems worthwhile?
> 
> 

Re: [PATCH] mm: list_lru: fix UAF for memory cgroup

[Vlastimil Babka (SUSE)]

On 7/24/24 4:23 AM, Muchun Song wrote:
> 
> 
>> On Jul 24, 2024, at 08:45, Andrew Morton <akpm@linux-foundation.org> wrote:
>> 
>> On Thu, 18 Jul 2024 16:36:07 +0800 Muchun Song <songmuchun@bytedance.com> wrote:
>> 
>>> The mem_cgroup_from_slab_obj() is supposed to be called under rcu
>>> lock or cgroup_mutex or others which could prevent returned memcg
>>> from being freed. Fix it by adding missing rcu read lock.
>> 
>> "or others" is rather vague.  What others?
> 
> Like objcg_lock. I have added this one into obj_cgroup_memcg().
> 
>> 
>>> @@ -109,14 +110,20 @@ EXPORT_SYMBOL_GPL(list_lru_add);
>>> 
>>> bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
>>> {
>>> + 	bool ret;
>>> 	int nid = page_to_nid(virt_to_page(item));
>>> - 	struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
>>> - 	mem_cgroup_from_slab_obj(item) : NULL;
>>> + 	struct mem_cgroup *memcg;
>>> 
>>> - 	return list_lru_add(lru, item, nid, memcg);
>>> + 	rcu_read_lock();
>>> + 	memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL;
>>> + 	ret = list_lru_add(lru, item, nid, memcg);
>>> + 	rcu_read_unlock();
>> 
>> We don't need rcu_read_lock() to evaluate NULL.
>> 
>> 	memcg = NULL;
>> 	if (list_lru_memcg_aware(lru)) {
>> 		rcu_read_lock();
>> 		memcg = mem_cgroup_from_slab_obj(item);
>> 		rcu_read_unlock();
> 
> Actually, the access to memcg is in list_lru_add(), so the rcu lock should
> also cover this function rather than only mem_cgroup_from_slab_obj().
> Something like:
> 
> memcg = NULL;
> if (list_lru_memcg_aware(lru)) {
> 	rcu_read_lock();
> 	memcg = mem_cgroup_from_slab_obj(item);
> }
> ret = list_lru_add(lru, item, nid, memcg);
> if (list_lru_memcg_aware(lru))
> 	rcu_read_unlock();
> 
> Not concise. I don't know if this is good.

At such point, it's probably best to just:

if (list_lru_memcg_aware(lru)) {
	rcu_read_lock();
	ret = list_lru_add(lru, item, nid, mem_cgroup_from_slab_obj(item));
	rcu_read_unlock();
} else {
	list_lru_add(lru, item, nid, NULL);
}

?

> 
>> 	}
>> 
>> Seems worthwhile?
>> 
>> 
> 
> 

Re: [PATCH] mm: list_lru: fix UAF for memory cgroup

[Andrew Morton]

On Thu, 18 Jul 2024 16:36:07 +0800 Muchun Song <songmuchun@bytedance.com> wrote:

> The mem_cgroup_from_slab_obj() is supposed to be called under rcu
> lock or cgroup_mutex or others which could prevent returned memcg
> from being freed. Fix it by adding missing rcu read lock.
> 

Well I grabbed this, but the review led me to expect a v2.

Should it have cc:stable?

Re: [PATCH] mm: list_lru: fix UAF for memory cgroup

[Muchun Song]

> On Aug 1, 2024, at 04:28, Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> On Thu, 18 Jul 2024 16:36:07 +0800 Muchun Song <songmuchun@bytedance.com> wrote:
> 
>> The mem_cgroup_from_slab_obj() is supposed to be called under rcu
>> lock or cgroup_mutex or others which could prevent returned memcg
>> from being freed. Fix it by adding missing rcu read lock.
>> 
> 
> Well I grabbed this, but the review led me to expect a v2.

Will do.

> 
> Should it have cc:stable?

Yes.

Re: [PATCH] mm: list_lru: fix UAF for memory cgroup

[Muchun Song]

> On Jul 31, 2024, at 18:06, Vlastimil Babka (SUSE) <vbabka@kernel.org> wrote:
> 
> On 7/24/24 4:23 AM, Muchun Song wrote:
>> 
>> 
>>> On Jul 24, 2024, at 08:45, Andrew Morton <akpm@linux-foundation.org> wrote:
>>> 
>>> On Thu, 18 Jul 2024 16:36:07 +0800 Muchun Song <songmuchun@bytedance.com> wrote:
>>> 
>>>> The mem_cgroup_from_slab_obj() is supposed to be called under rcu
>>>> lock or cgroup_mutex or others which could prevent returned memcg
>>>> from being freed. Fix it by adding missing rcu read lock.
>>> 
>>> "or others" is rather vague.  What others?
>> 
>> Like objcg_lock. I have added this one into obj_cgroup_memcg().
>> 
>>> 
>>>> @@ -109,14 +110,20 @@ EXPORT_SYMBOL_GPL(list_lru_add);
>>>> 
>>>> bool list_lru_add_obj(struct list_lru *lru, struct list_head *item)
>>>> {
>>>> +  bool ret;
>>>> int nid = page_to_nid(virt_to_page(item));
>>>> -  struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ?
>>>> -  mem_cgroup_from_slab_obj(item) : NULL;
>>>> +  struct mem_cgroup *memcg;
>>>> 
>>>> -  return list_lru_add(lru, item, nid, memcg);
>>>> +  rcu_read_lock();
>>>> +  memcg = list_lru_memcg_aware(lru) ? mem_cgroup_from_slab_obj(item) : NULL;
>>>> +  ret = list_lru_add(lru, item, nid, memcg);
>>>> +  rcu_read_unlock();
>>> 
>>> We don't need rcu_read_lock() to evaluate NULL.
>>> 
>>> memcg = NULL;
>>> if (list_lru_memcg_aware(lru)) {
>>> rcu_read_lock();
>>> memcg = mem_cgroup_from_slab_obj(item);
>>> rcu_read_unlock();
>> 
>> Actually, the access to memcg is in list_lru_add(), so the rcu lock should
>> also cover this function rather than only mem_cgroup_from_slab_obj().
>> Something like:
>> 
>> memcg = NULL;
>> if (list_lru_memcg_aware(lru)) {
>> rcu_read_lock();
>> memcg = mem_cgroup_from_slab_obj(item);
>> }
>> ret = list_lru_add(lru, item, nid, memcg);
>> if (list_lru_memcg_aware(lru))
>> rcu_read_unlock();
>> 
>> Not concise. I don't know if this is good.
> 
> At such point, it's probably best to just:
> 
> if (list_lru_memcg_aware(lru)) {
> 	rcu_read_lock();
> 	ret = list_lru_add(lru, item, nid, mem_cgroup_from_slab_obj(item));
> 	rcu_read_unlock();
> } else {
> 	list_lru_add(lru, item, nid, NULL);
> }

Good. Will update v2.

Thanks.

> 
> ?
> 
>> 
>>> }
>>> 
>>> Seems worthwhile?