Thread (3 messages) 3 messages, 3 authors, 2021-12-30

Re: [syzbot] general protection fault in mod_memcg_page_state

From: Shakeel Butt <hidden>
Date: 2021-12-30 19:08:29
Also in: bpf, lkml

On Thu, Dec 30, 2021 at 12:53 AM Michal Hocko [off-list ref] wrote:
[...]
quoted hunk ↗ jump to hunk
This might have something to do with http://lkml.kernel.org/r/20211222052457.1960701-1-shakeelb@google.com
which has added the accounting which is blowing up. The problem happens
when a memcg is retrieved from the allocated page. This should be NULL
as the reported commit doesn't really add any __GFP_ACCOUNT user AFAICS.
Anyway vm_area_alloc_pages can fail the allocation if the current
context has fatal signals pending. array->pages array is allocated with
__GFP_ZERO so the failed allocation should have kept the pages[0] NULL.
I haven't followed the page->memcg path to double check whether that
could lead to 0xdffffc0000000001 in the end.

I believe we need something like
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 9bf838817a47..d2e392cac909 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2627,7 +2627,8 @@ static void __vunmap(const void *addr, int deallocate_pages)
                unsigned int page_order = vm_area_page_order(area);
                int i;

-               mod_memcg_page_state(area->pages[0], MEMCG_VMALLOC,
+               if (area->pages[0])
+                       mod_memcg_page_state(area->pages[0], MEMCG_VMALLOC,
                                     -area->nr_pages);

                for (i = 0; i < area->nr_pages; i += 1U << page_order) {
@@ -2968,7 +2969,8 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
                page_order, nr_small_pages, area->pages);

        atomic_long_add(area->nr_pages, &nr_vmalloc_pages);
-       mod_memcg_page_state(area->pages[0], MEMCG_VMALLOC, area->nr_pages);
+       if (area->pages[0])
+               mod_memcg_page_state(area->pages[0], MEMCG_VMALLOC, area->nr_pages);

        /*
         * If not enough pages were obtained to accomplish an
Or to account each page separately so that we do not have to rely on
pages[0].
Thanks Michal for the CC. I will add these checks in the next version
(or convert to account each page separately based on discussion on the
other thread).

Shakeel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help