Re: [Bug 189181] New: BUG: unable to handle kernel NULL pointer dereference in mem_cgroup_node_nr_lru_pages

[Balbir Singh <bsingharora@gmail.com>]

On 01/12/16 05:30, Michal Hocko wrote:
> On Wed 30-11-16 13:16:53, Johannes Weiner wrote:
>> Hi Michael,
>>
>> On Wed, Nov 30, 2016 at 06:00:40PM +0100, Michal Hocko wrote:
> [...]
>>> diff --git a/mm/workingset.c b/mm/workingset.c
>>> index 617475f529f4..0f07522c5c0e 100644
>>> --- a/mm/workingset.c
>>> +++ b/mm/workingset.c
>>> @@ -348,7 +348,7 @@ static unsigned long count_shadow_nodes(struct shrinker *shrinker,
>>>  	shadow_nodes = list_lru_shrink_count(&workingset_shadow_nodes, sc);
>>>  	local_irq_enable();
>>>  
>>> -	if (memcg_kmem_enabled()) {
>>> +	if (memcg_kmem_enabled() && sc->memcg) {
>>>  		pages = mem_cgroup_node_nr_lru_pages(sc->memcg, sc->nid,
>>>  						     LRU_ALL_FILE);
>>>  	} else {
>>
>> If we do that, I'd remove the racy memcg_kmem_enabled() check
>> altogether and just check for whether we have a memcg or not.
> 
> But that would make this a memcg aware shrinker even when kmem is not
> enabled...
> 
> But now that I am looking into the code
> shrink_slab:
> 		if (memcg_kmem_enabled() &&
> 		    !!memcg != !!(shrinker->flags & SHRINKER_MEMCG_AWARE))
> 			continue;
> 
> this should be taken care of already. So sc->memcg should be indeed
> sufficient. So unless I am missing something I will respin my local
> patch and post it later after the reporter has some time to test the
> current one.
>  

I did a quick dis-assembly of the code

R9 and RDI are NULL and the instruction seems to be

mov rsi, [rdi+r9*8+0x400]

RDI is NULL, sc->memcg is NULL, which indicates global reclaim

The check referred to earlier

                /*
                 * If kernel memory accounting is disabled, we ignore
                 * SHRINKER_MEMCG_AWARE flag and call all shrinkers
                 * passing NULL for memcg.
                 */
                if (memcg_kmem_enabled() &&
                    !!memcg != !!(shrinker->flags & SHRINKER_MEMCG_AWARE))
                        continue;

So we do pass NULL for memcg

A check for sc->memcg should be enough in count_shadow_nodes and a VM_BUG_ON
for memcg == NULL in mem_cgroup_node_nr_lru_pages would be nice

Balbir Singh.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>