On Fri, Dec 01, 2023 at 09:46:32AM +0000, Yafang Shao wrote:
> Considering that MPOL_F_NUMA_BALANCING or mbind(2) using either
> MPOL_MF_MOVE or MPOL_MF_MOVE_ALL are capable of memory movement, it's
> essential to include security_task_movememory() to cover this
> functionality as well. It was identified during a code review.
Hm - this doesn't have any bad side effects for you when using selinux?
The selinux_task_movememory() hook checks for PROCESS__SETSCHED privs.
The two existing security_task_movememory() calls are in cases where we
expect the caller to be affecting another task identified by pid, so
that makes sense. Is an MPOL_MV_MOVE to move your own pages actually
analogous to that?
Much like the concern you mentioned in your intro about requiring
CAP_SYS_NICE and thereby expanding its use, it seems that here you
will be regressing some mbind users unless the granting of PROCESS__SETSCHED
is widened.
> Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> ---
> mm/mempolicy.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 10a590ee1c89..1eafe81d782e 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -1259,8 +1259,15 @@ static long do_mbind(unsigned long start, unsigned long len,
> if (!new)
> flags |= MPOL_MF_DISCONTIG_OK;
>
> - if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL))
> + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) {
MPOL_MF_MOVE_ALL already has a CAP_SYS_NICE check. Does that
suffice for that one?
> + err = security_task_movememory(current);
> + if (err) {
> + mpol_put(new);
> + return err;
> + }
> lru_cache_disable();
> + }
> +
> {
> NODEMASK_SCRATCH(scratch);
> if (scratch) {
> @@ -1450,6 +1457,8 @@ static int copy_nodes_to_user(unsigned long __user *mask, unsigned long maxnode,
> /* Basic parameter sanity check used by both mbind() and set_mempolicy() */
> static inline int sanitize_mpol_flags(int *mode, unsigned short *flags)
> {
> + int err;
> +
> *flags = *mode & MPOL_MODE_FLAGS;
> *mode &= ~MPOL_MODE_FLAGS;
>
> @@ -1460,6 +1469,9 @@ static inline int sanitize_mpol_flags(int *mode, unsigned short *flags)
> if (*flags & MPOL_F_NUMA_BALANCING) {
> if (*mode != MPOL_BIND)
> return -EINVAL;
> + err = security_task_movememory(current);
> + if (err)
> + return err;
> *flags |= (MPOL_F_MOF | MPOL_F_MORON);
> }
> return 0;
> --
> 2.30.1 (Apple Git-130)
