Received-SPF: pass (google.com: domain of linux-kernel+bounces-11907-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: "Huang, Ying" <ying.huang@intel.com>
To: Gregory Price <gourry.memverge@gmail.com>
Cc: linux-mm@kvack.org,  linux-doc@vger.kernel.org,
  linux-fsdevel@vger.kernel.org,  linux-kernel@vger.kernel.org,
  linux-api@vger.kernel.org,  x86@kernel.org,  akpm@linux-foundation.org,
  arnd@arndb.de,  tglx@linutronix.de,  luto@kernel.org,  mingo@redhat.com,
  bp@alien8.de,  dave.hansen@linux.intel.com,  hpa@zytor.com,
  mhocko@kernel.org,  tj@kernel.org,  gregory.price@memverge.com,
  corbet@lwn.net,  rakie.kim@sk.com,  hyeongtak.ji@sk.com,
  honggyu.kim@sk.com,  vtavarespetr@micron.com,  peterz@infradead.org,
  jgroves@micron.com,  ravis.opensrc@micron.com,  sthanneeru@micron.com,
  emirakhur@micron.com,  Hasan.Maruf@amd.com,  seungjun.ha@samsung.com
Subject: Re: [PATCH v5 03/11] mm/mempolicy: refactor sanitize_mpol_flags for
 reuse
In-Reply-To: <20231223181101.1954-4-gregory.price@memverge.com> (Gregory
	Price's message of "Sat, 23 Dec 2023 13:10:53 -0500")
References: <20231223181101.1954-1-gregory.price@memverge.com>
	<20231223181101.1954-4-gregory.price@memverge.com>
Date: Wed, 27 Dec 2023 16:39:29 +0800
Message-ID: <87y1dgdoou.fsf@yhuang6-desk2.ccr.corp.intel.com>
User-Agent: Gnus/5.13 (Gnus v5.13)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=ascii

Gregory Price <gourry.memverge@gmail.com> writes:

> split sanitize_mpol_flags into sanitize and validate.
>
> Sanitize is used by set_mempolicy to split (int mode) into mode
> and mode_flags, and then validates them.
>
> Validate validates already split flags.
>
> Validate will be reused for new syscalls that accept already
> split mode and mode_flags.
>
> Signed-off-by: Gregory Price <gregory.price@memverge.com>
> ---
>  mm/mempolicy.c | 29 ++++++++++++++++++++++-------
>  1 file changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 0a180c670f0c..59ac0da24f56 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -1463,24 +1463,39 @@ static int copy_nodes_to_user(unsigned long __user *mask, unsigned long maxnode,
>  	return copy_to_user(mask, nodes_addr(*nodes), copy) ? -EFAULT : 0;
>  }
>  
> -/* Basic parameter sanity check used by both mbind() and set_mempolicy() */
> -static inline int sanitize_mpol_flags(int *mode, unsigned short *flags)
> +/*
> + * Basic parameter sanity check used by mbind/set_mempolicy
> + * May modify flags to include internal flags (e.g. MPOL_F_MOF/F_MORON)
> + */
> +static inline int validate_mpol_flags(unsigned short mode, unsigned short *flags)
>  {
> -	*flags = *mode & MPOL_MODE_FLAGS;
> -	*mode &= ~MPOL_MODE_FLAGS;
> -
> -	if ((unsigned int)(*mode) >=  MPOL_MAX)
> +	if ((unsigned int)(mode) >= MPOL_MAX)
>  		return -EINVAL;
>  	if ((*flags & MPOL_F_STATIC_NODES) && (*flags & MPOL_F_RELATIVE_NODES))
>  		return -EINVAL;
>  	if (*flags & MPOL_F_NUMA_BALANCING) {
> -		if (*mode != MPOL_BIND)
> +		if (mode != MPOL_BIND)
>  			return -EINVAL;
>  		*flags |= (MPOL_F_MOF | MPOL_F_MORON);
>  	}
>  	return 0;
>  }
>  
> +/*
> + * Used by mbind/set_memplicy to split and validate mode/flags
> + * set_mempolicy combines (mode | flags), split them out into separate

mbind() uses mode flags too.

> + * fields and return just the mode in mode_arg and flags in flags.
> + */
> +static inline int sanitize_mpol_flags(int *mode_arg, unsigned short *flags)
> +{
> +	unsigned short mode = (*mode_arg & ~MPOL_MODE_FLAGS);
> +
> +	*flags = *mode_arg & MPOL_MODE_FLAGS;
> +	*mode_arg = mode;

It appears that it's unnecessary to introduce a local variable to split
mode/flags.  Just reuse the original code?

> +
> +	return validate_mpol_flags(mode, flags);
> +}
> +
>  static long kernel_mbind(unsigned long start, unsigned long len,
>  			 unsigned long mode, const unsigned long __user *nmask,
>  			 unsigned long maxnode, unsigned int flags)

--
Best Regards,
Huang, Ying