Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20211001215630.810592-1-eric.dumazet@gmail.com> <20211001154949.98956c092734590e781ce672@linux-foundation.org>
In-Reply-To: <20211001154949.98956c092734590e781ce672@linux-foundation.org>
From:   Eric Dumazet <edumazet@google.com>
Date:   Fri, 1 Oct 2021 16:37:40 -0700
Message-ID: <CANn89i+diZ9cmMo+y3KhBK3xiWbzAsC_RF86=hN4KJ1Kxg1SwA@mail.gmail.com>
Subject: Re: [PATCH v2] mm/mempolicy: do not allow illegal MPOL_F_NUMA_BALANCING
 | MPOL_LOCAL in mbind()
To:     Andrew Morton <akpm@linux-foundation.org>
Cc:     Eric Dumazet <eric.dumazet@gmail.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-mm <linux-mm@kvack.org>,
        syzbot <syzkaller@googlegroups.com>,
        "Huang, Ying" <ying.huang@intel.com>, Mel Gorman <mgorman@suse.de>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Oct 1, 2021 at 3:49 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri,  1 Oct 2021 14:56:30 -0700 Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > From: Eric Dumazet <edumazet@google.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)