linux-kernel - Re: [PATCH v2] mm/mempolicy: do not allow illegal MPOL_F_NUMA_BALANCING

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89i+diZ9cmMo+y3KhBK3xiWbzAsC_RF86=hN4KJ1Kxg1SwA@mail.gmail.com>
Date:   Fri, 1 Oct 2021 16:37:40 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-mm <linux-mm@...ck.org>,
        syzbot <syzkaller@...glegroups.com>,
        "Huang, Ying" <ying.huang@...el.com>, Mel Gorman <mgorman@...e.de>
Subject: Re: [PATCH v2] mm/mempolicy: do not allow illegal MPOL_F_NUMA_BALANCING
 | MPOL_LOCAL in mbind()

On Fri, Oct 1, 2021 at 3:49 PM Andrew Morton <akpm@...ux-foundation.org> wrote:
>
> On Fri,  1 Oct 2021 14:56:30 -0700 Eric Dumazet <eric.dumazet@...il.com> wrote:
>
> > From: Eric Dumazet <edumazet@...gle.com>
> >
> > syzbot reported access to unitialized memory in mbind() [1]
>
> I'm lazy.  What memory is being accessed-unintialized?

I think you can clearly see that with this debug patch (courtesy of
Alexander Potapenko) :
(Then issue various mbind( ...MPOL_F_NUMA_BALANCING | MPOL_LOCAL ...)
in a loop... )


diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 1592b081c58e..95a4d71efe99 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -291,6 +291,7 @@ static struct mempolicy *mpol_new(unsigned short
mode, unsigned short flags,
        } else if (nodes_empty(*nodes))
                return ERR_PTR(-EINVAL);
        policy = kmem_cache_alloc(policy_cache, GFP_KERNEL);
+       memset(policy, 0xAA, sizeof(struct mempolicy));
        if (!policy)
                return ERR_PTR(-ENOMEM);
        atomic_set(&policy->refcnt, 1);
@@ -2256,9 +2257,12 @@ bool __mpol_equal(struct mempolicy *a, struct
mempolicy *b)
                return false;
        if (a->flags != b->flags)
                return false;
-       if (mpol_store_user_nodemask(a))
+       if (mpol_store_user_nodemask(a)) {
+               pr_err("struct mempolicy *a: %px, nodemask: %px\n", a,
*(void**)&(a->w.user_nodemask));
+               pr_err("struct mempolicy *b: %px, nodemask: %px\n", b,
*(void**)&(b->w.user_nodemask));
                if (!nodes_equal(a->w.user_nodemask, b->w.user_nodemask))
                        return false;
+       }

        switch (a->mode) {
        case MPOL_BIND:


>
> > Issue came with commit bda420b98505 ("numa balancing: migrate on
> > fault among multiple bound nodes")
>
> No cc:stable?  What's the worst-case user-visible impact here?

I added the more precise tag  :  Fixes: bda420b98505 ("numa balancing:
migrate on fault among multiple bound nodes")
I only put Fixes: tag, so that stable teams can use their automation just fine.

worst-case impact, I am not sure if any application ever used this
undocumented combinations of flags ?
Also, it is generally advised that accessing garbage values has
undocumented behavior.
A host could for example crash (it certainly does with KMSAN)