netdev - Re: [syzbot] general protection fault in mod_memcg_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 30 Dec 2021 11:08:12 -0800
From:   Shakeel Butt <shakeelb@...gle.com>
To:     Michal Hocko <mhocko@...e.com>
Cc:     syzbot <syzbot+864849a13d44b22de04d@...kaller.appspotmail.com>,
        Andrii Nakryiko <andrii@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>, bpf <bpf@...r.kernel.org>,
        changbin.du@...el.com, Daniel Borkmann <daniel@...earbox.net>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        hkallweit1@...il.com, John Fastabend <john.fastabend@...il.com>,
        Martin KaFai Lau <kafai@...com>, kpsingh@...nel.org,
        Jakub Kicinski <kuba@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>,
        Song Liu <songliubraving@...com>,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>,
        yajun.deng@...ux.dev, Yonghong Song <yhs@...com>
Subject: Re: [syzbot] general protection fault in mod_memcg_page_state

On Thu, Dec 30, 2021 at 12:53 AM Michal Hocko <mhocko@...e.com> wrote:
[...]
>
> 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