Re: [PATCH v2] mm/mempolicy: do not allow illegal MPOL_F_NUMA_BALANCING | MPOL_LOCAL in mbind()

[Eric Dumazet]

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