Re: [PATCH] mm/mempolicy: fix mpol_rebind_nodemask() for MPOL_F_NUMA_BALANCING

[Gregory Price <gourry@gourry.net>]

On Sun, Dec 14, 2025 at 04:04:59PM -0800, Andrew Morton wrote:
> On Sat, 13 Dec 2025 16:29:11 +0800 Jinjiang Tu <tujinjiang@huawei.com> wrote:
...
> The intended behavior was for the rebinding logic to remain the same as
> the default `MPOL_BIND` behavior.  However, the function
> `mpol_store_user_nodemask()` was incorrectly returning `true` for
> policies containing `MPOL_F_NUMA_BALANCING`.
> 
> This led to a bug where:
> 
> 1.  `mempolicy.w.cpuset_mems_allowed` stored the **user's passed
>     nodemask** instead of the actual nodemask allowed by the current
>     cpuset context (`cpuset_current_mems_allowed`).
>

Hm... these things are a union.  

It's probably simpler to state the following

----

Setting MPOL_F_NUMA_BALANCING causes pol->w.cpuset_mems_allowed to be
erroneously overwritten, causing mpol_rebind_nodemask to rebind the
policy based on the wrong nodemask.

1. The intended rebind behavior of MPOL_F_NUMA_BALANCING when neither
   MPOL_F_STATIC_NODES or MPOL_F_RELATIVE_NODES flags are present is
   to remap nodes based on mempolicy.w.cpuset_mems_allowed.

2. `mempolicy.w.cpuset_mems_allowed` is overwritten by mpol_set_nodemask
   setting `mempolicy.w.user_nodemask` (these are unioned) when
   MPOL_F_NUMA_BALANCING is set because it mpol_store_user_nodemask()
   check for any mode flag.

   union {
       nodemask_t cpuset_mems_allowed; /* relative to these nodes */
       nodemask_t user_nodemask;       /* nodemask passed by user */
   } w;

   static inline int mpol_store_user_nodemask(const struct mempolicy *pol)
   {               
        return pol->flags & MPOL_MODE_FLAGS;
   }

   static int mpol_set_nodemask(...)
   {
        if (mpol_store_user_nodemask(pol))
                pol->w.user_nodemask = *nodes;
        else    
                pol->w.cpuset_mems_allowed = cpuset_current_mems_allowed;
   }


3. `mpol_rebind_nodemask()` consequently ends up rebinding based on the
   user-passed nodemask rather than the cpuset_mems_allowed nodemask
   as intended.

   static void mpol_rebind_nodemask()
   {
        if (pol->flags & MPOL_F_STATIC_NODES)
                nodes_and(tmp, pol->w.user_nodemask, *nodes);
        else if (pol->flags & MPOL_F_RELATIVE_NODES)
                mpol_relative_nodemask(&tmp, &pol->w.user_nodemask, nodes);
        else
                nodes_remap(tmp, pol->nodes, pol->w.cpuset_mems_allowed,
                                                                *nodes);
       ...
   }

To fix this, only store the user nodemask if MPOL_F_STATIC_NODES or
MPOL_F_RELATIVE_NODES are present.

-----------

On another note... what's even the reason for this union to exist if you
need to know the flag state to determine which one to access????

and they're both nodemask_t!

May as well call it `pol->w.rebind_mask` or something and let the flags do
the talking.

Otherwise the fix looks good, I will respond to the original with a
review tag.

~Gregory