[PATCH v2] prctl: Deprecate non PR_SET_MM_MAP operations

[PATCH v2] prctl: Deprecate non PR_SET_MM_MAP operations

[Cyrill Gorcunov]

An ability to manipulate mm_struct fields was introduced in
sake of CRIU in first place. Later we provide more suitable
and safe operation PR_SET_MM_MAP where all fields to be modifed
are passed in one structure which allows us to make more detailed
verification.

Still old interface remains present for compatibility reason
though CRIU itself already switched to PR_SET_MM_MAP on its
own long ago.

Googling didn't reveal some other users of this operation
so I think it should be safe to remove this interface.

v2:
 - Improve warning message
 - Drop redundant args check

CC: Andrey Vagin <avagin@openvz.org>
CC: Andrew Morton <akpm@linuxfoundation.org>
CC: Pavel Emelyanov <xemul@virtuozzo.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Yang Shi <yang.shi@linux.alibaba.com>
CC: Michal Hocko <mhocko@kernel.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 kernel/sys.c |  151 -----------------------------------------------------------
 1 file changed, 2 insertions(+), 149 deletions(-)

Index: linux-ml.git/kernel/sys.c
===================================================================
--- linux-ml.git.orig/kernel/sys.c
+++ linux-ml.git/kernel/sys.c
@@ -2053,163 +2053,16 @@ static int prctl_set_mm_map(int opt, con
 }
 #endif /* CONFIG_CHECKPOINT_RESTORE */
 
-static int prctl_set_auxv(struct mm_struct *mm, unsigned long addr,
-			  unsigned long len)
-{
-	/*
-	 * This doesn't move the auxiliary vector itself since it's pinned to
-	 * mm_struct, but it permits filling the vector with new values.  It's
-	 * up to the caller to provide sane values here, otherwise userspace
-	 * tools which use this vector might be unhappy.
-	 */
-	unsigned long user_auxv[AT_VECTOR_SIZE];
-
-	if (len > sizeof(user_auxv))
-		return -EINVAL;
-
-	if (copy_from_user(user_auxv, (const void __user *)addr, len))
-		return -EFAULT;
-
-	/* Make sure the last entry is always AT_NULL */
-	user_auxv[AT_VECTOR_SIZE - 2] = 0;
-	user_auxv[AT_VECTOR_SIZE - 1] = 0;
-
-	BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
-
-	task_lock(current);
-	memcpy(mm->saved_auxv, user_auxv, len);
-	task_unlock(current);
-
-	return 0;
-}
-
 static int prctl_set_mm(int opt, unsigned long addr,
 			unsigned long arg4, unsigned long arg5)
 {
-	struct mm_struct *mm = current->mm;
-	struct prctl_mm_map prctl_map;
-	struct vm_area_struct *vma;
-	int error;
-
-	if (arg5 || (arg4 && (opt != PR_SET_MM_AUXV &&
-			      opt != PR_SET_MM_MAP &&
-			      opt != PR_SET_MM_MAP_SIZE)))
-		return -EINVAL;
-
 #ifdef CONFIG_CHECKPOINT_RESTORE
 	if (opt == PR_SET_MM_MAP || opt == PR_SET_MM_MAP_SIZE)
 		return prctl_set_mm_map(opt, (const void __user *)addr, arg4);
 #endif
 
-	if (!capable(CAP_SYS_RESOURCE))
-		return -EPERM;
-
-	if (opt == PR_SET_MM_EXE_FILE)
-		return prctl_set_mm_exe_file(mm, (unsigned int)addr);
-
-	if (opt == PR_SET_MM_AUXV)
-		return prctl_set_auxv(mm, addr, arg4);
-
-	if (addr >= TASK_SIZE || addr < mmap_min_addr)
-		return -EINVAL;
-
-	error = -EINVAL;
-
-	down_write(&mm->mmap_sem);
-	vma = find_vma(mm, addr);
-
-	prctl_map.start_code	= mm->start_code;
-	prctl_map.end_code	= mm->end_code;
-	prctl_map.start_data	= mm->start_data;
-	prctl_map.end_data	= mm->end_data;
-	prctl_map.start_brk	= mm->start_brk;
-	prctl_map.brk		= mm->brk;
-	prctl_map.start_stack	= mm->start_stack;
-	prctl_map.arg_start	= mm->arg_start;
-	prctl_map.arg_end	= mm->arg_end;
-	prctl_map.env_start	= mm->env_start;
-	prctl_map.env_end	= mm->env_end;
-	prctl_map.auxv		= NULL;
-	prctl_map.auxv_size	= 0;
-	prctl_map.exe_fd	= -1;
-
-	switch (opt) {
-	case PR_SET_MM_START_CODE:
-		prctl_map.start_code = addr;
-		break;
-	case PR_SET_MM_END_CODE:
-		prctl_map.end_code = addr;
-		break;
-	case PR_SET_MM_START_DATA:
-		prctl_map.start_data = addr;
-		break;
-	case PR_SET_MM_END_DATA:
-		prctl_map.end_data = addr;
-		break;
-	case PR_SET_MM_START_STACK:
-		prctl_map.start_stack = addr;
-		break;
-	case PR_SET_MM_START_BRK:
-		prctl_map.start_brk = addr;
-		break;
-	case PR_SET_MM_BRK:
-		prctl_map.brk = addr;
-		break;
-	case PR_SET_MM_ARG_START:
-		prctl_map.arg_start = addr;
-		break;
-	case PR_SET_MM_ARG_END:
-		prctl_map.arg_end = addr;
-		break;
-	case PR_SET_MM_ENV_START:
-		prctl_map.env_start = addr;
-		break;
-	case PR_SET_MM_ENV_END:
-		prctl_map.env_end = addr;
-		break;
-	default:
-		goto out;
-	}
-
-	error = validate_prctl_map(&prctl_map);
-	if (error)
-		goto out;
-
-	switch (opt) {
-	/*
-	 * If command line arguments and environment
-	 * are placed somewhere else on stack, we can
-	 * set them up here, ARG_START/END to setup
-	 * command line argumets and ENV_START/END
-	 * for environment.
-	 */
-	case PR_SET_MM_START_STACK:
-	case PR_SET_MM_ARG_START:
-	case PR_SET_MM_ARG_END:
-	case PR_SET_MM_ENV_START:
-	case PR_SET_MM_ENV_END:
-		if (!vma) {
-			error = -EFAULT;
-			goto out;
-		}
-	}
-
-	mm->start_code	= prctl_map.start_code;
-	mm->end_code	= prctl_map.end_code;
-	mm->start_data	= prctl_map.start_data;
-	mm->end_data	= prctl_map.end_data;
-	mm->start_brk	= prctl_map.start_brk;
-	mm->brk		= prctl_map.brk;
-	mm->start_stack	= prctl_map.start_stack;
-	mm->arg_start	= prctl_map.arg_start;
-	mm->arg_end	= prctl_map.arg_end;
-	mm->env_start	= prctl_map.env_start;
-	mm->env_end	= prctl_map.env_end;
-
-	error = 0;
-out:
-	up_write(&mm->mmap_sem);
-	return error;
+	pr_warn_once("PR_SET_MM_* has been removed. Use PR_SET_MM_MAP instead\n");
+	return -EINVAL;
 }
 
 #ifdef CONFIG_CHECKPOINT_RESTORE

Re: [PATCH v2] prctl: Deprecate non PR_SET_MM_MAP operations

[Michal Hocko]

On Thu 05-04-18 21:26:51, Cyrill Gorcunov wrote:
> An ability to manipulate mm_struct fields was introduced in
> sake of CRIU in first place. Later we provide more suitable
> and safe operation PR_SET_MM_MAP where all fields to be modifed
> are passed in one structure which allows us to make more detailed
> verification.

I hope this will serve as a memento for future single-user APIs
proposals. The whole thing was a bad idea since the beginning.

> Still old interface remains present for compatibility reason
> though CRIU itself already switched to PR_SET_MM_MAP on its
> own long ago.
> 
> Googling didn't reveal some other users of this operation
> so I think it should be safe to remove this interface.
> 
> v2:
>  - Improve warning message
>  - Drop redundant args check
> 
> CC: Andrey Vagin <avagin@openvz.org>
> CC: Andrew Morton <akpm@linuxfoundation.org>
> CC: Pavel Emelyanov <xemul@virtuozzo.com>
> CC: Michael Kerrisk <mtk.manpages@gmail.com>
> CC: Yang Shi <yang.shi@linux.alibaba.com>
> CC: Michal Hocko <mhocko@kernel.org>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>

Acked-by: Michal Hocko <mhocko@suse.com>

and fingers crossed that we haven't grown other users outside of CRIU
which is quite bound to specific kernels AFAIK.

> ---
>  kernel/sys.c |  151 -----------------------------------------------------------
>  1 file changed, 2 insertions(+), 149 deletions(-)
> 
> Index: linux-ml.git/kernel/sys.c
> ===================================================================
> --- linux-ml.git.orig/kernel/sys.c
> +++ linux-ml.git/kernel/sys.c
> @@ -2053,163 +2053,16 @@ static int prctl_set_mm_map(int opt, con
>  }
>  #endif /* CONFIG_CHECKPOINT_RESTORE */
>  
> -static int prctl_set_auxv(struct mm_struct *mm, unsigned long addr,
> -			  unsigned long len)
> -{
> -	/*
> -	 * This doesn't move the auxiliary vector itself since it's pinned to
> -	 * mm_struct, but it permits filling the vector with new values.  It's
> -	 * up to the caller to provide sane values here, otherwise userspace
> -	 * tools which use this vector might be unhappy.
> -	 */
> -	unsigned long user_auxv[AT_VECTOR_SIZE];
> -
> -	if (len > sizeof(user_auxv))
> -		return -EINVAL;
> -
> -	if (copy_from_user(user_auxv, (const void __user *)addr, len))
> -		return -EFAULT;
> -
> -	/* Make sure the last entry is always AT_NULL */
> -	user_auxv[AT_VECTOR_SIZE - 2] = 0;
> -	user_auxv[AT_VECTOR_SIZE - 1] = 0;
> -
> -	BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
> -
> -	task_lock(current);
> -	memcpy(mm->saved_auxv, user_auxv, len);
> -	task_unlock(current);
> -
> -	return 0;
> -}
> -
>  static int prctl_set_mm(int opt, unsigned long addr,
>  			unsigned long arg4, unsigned long arg5)
>  {
> -	struct mm_struct *mm = current->mm;
> -	struct prctl_mm_map prctl_map;
> -	struct vm_area_struct *vma;
> -	int error;
> -
> -	if (arg5 || (arg4 && (opt != PR_SET_MM_AUXV &&
> -			      opt != PR_SET_MM_MAP &&
> -			      opt != PR_SET_MM_MAP_SIZE)))
> -		return -EINVAL;
> -
>  #ifdef CONFIG_CHECKPOINT_RESTORE
>  	if (opt == PR_SET_MM_MAP || opt == PR_SET_MM_MAP_SIZE)
>  		return prctl_set_mm_map(opt, (const void __user *)addr, arg4);
>  #endif
>  
> -	if (!capable(CAP_SYS_RESOURCE))
> -		return -EPERM;
> -
> -	if (opt == PR_SET_MM_EXE_FILE)
> -		return prctl_set_mm_exe_file(mm, (unsigned int)addr);
> -
> -	if (opt == PR_SET_MM_AUXV)
> -		return prctl_set_auxv(mm, addr, arg4);
> -
> -	if (addr >= TASK_SIZE || addr < mmap_min_addr)
> -		return -EINVAL;
> -
> -	error = -EINVAL;
> -
> -	down_write(&mm->mmap_sem);
> -	vma = find_vma(mm, addr);
> -
> -	prctl_map.start_code	= mm->start_code;
> -	prctl_map.end_code	= mm->end_code;
> -	prctl_map.start_data	= mm->start_data;
> -	prctl_map.end_data	= mm->end_data;
> -	prctl_map.start_brk	= mm->start_brk;
> -	prctl_map.brk		= mm->brk;
> -	prctl_map.start_stack	= mm->start_stack;
> -	prctl_map.arg_start	= mm->arg_start;
> -	prctl_map.arg_end	= mm->arg_end;
> -	prctl_map.env_start	= mm->env_start;
> -	prctl_map.env_end	= mm->env_end;
> -	prctl_map.auxv		= NULL;
> -	prctl_map.auxv_size	= 0;
> -	prctl_map.exe_fd	= -1;
> -
> -	switch (opt) {
> -	case PR_SET_MM_START_CODE:
> -		prctl_map.start_code = addr;
> -		break;
> -	case PR_SET_MM_END_CODE:
> -		prctl_map.end_code = addr;
> -		break;
> -	case PR_SET_MM_START_DATA:
> -		prctl_map.start_data = addr;
> -		break;
> -	case PR_SET_MM_END_DATA:
> -		prctl_map.end_data = addr;
> -		break;
> -	case PR_SET_MM_START_STACK:
> -		prctl_map.start_stack = addr;
> -		break;
> -	case PR_SET_MM_START_BRK:
> -		prctl_map.start_brk = addr;
> -		break;
> -	case PR_SET_MM_BRK:
> -		prctl_map.brk = addr;
> -		break;
> -	case PR_SET_MM_ARG_START:
> -		prctl_map.arg_start = addr;
> -		break;
> -	case PR_SET_MM_ARG_END:
> -		prctl_map.arg_end = addr;
> -		break;
> -	case PR_SET_MM_ENV_START:
> -		prctl_map.env_start = addr;
> -		break;
> -	case PR_SET_MM_ENV_END:
> -		prctl_map.env_end = addr;
> -		break;
> -	default:
> -		goto out;
> -	}
> -
> -	error = validate_prctl_map(&prctl_map);
> -	if (error)
> -		goto out;
> -
> -	switch (opt) {
> -	/*
> -	 * If command line arguments and environment
> -	 * are placed somewhere else on stack, we can
> -	 * set them up here, ARG_START/END to setup
> -	 * command line argumets and ENV_START/END
> -	 * for environment.
> -	 */
> -	case PR_SET_MM_START_STACK:
> -	case PR_SET_MM_ARG_START:
> -	case PR_SET_MM_ARG_END:
> -	case PR_SET_MM_ENV_START:
> -	case PR_SET_MM_ENV_END:
> -		if (!vma) {
> -			error = -EFAULT;
> -			goto out;
> -		}
> -	}
> -
> -	mm->start_code	= prctl_map.start_code;
> -	mm->end_code	= prctl_map.end_code;
> -	mm->start_data	= prctl_map.start_data;
> -	mm->end_data	= prctl_map.end_data;
> -	mm->start_brk	= prctl_map.start_brk;
> -	mm->brk		= prctl_map.brk;
> -	mm->start_stack	= prctl_map.start_stack;
> -	mm->arg_start	= prctl_map.arg_start;
> -	mm->arg_end	= prctl_map.arg_end;
> -	mm->env_start	= prctl_map.env_start;
> -	mm->env_end	= prctl_map.env_end;
> -
> -	error = 0;
> -out:
> -	up_write(&mm->mmap_sem);
> -	return error;
> +	pr_warn_once("PR_SET_MM_* has been removed. Use PR_SET_MM_MAP instead\n");
> +	return -EINVAL;
>  }
>  
>  #ifdef CONFIG_CHECKPOINT_RESTORE

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v2] prctl: Deprecate non PR_SET_MM_MAP operations

[Yang Shi]

On 4/5/18 11:26 AM, Cyrill Gorcunov wrote:
> An ability to manipulate mm_struct fields was introduced in
> sake of CRIU in first place. Later we provide more suitable
> and safe operation PR_SET_MM_MAP where all fields to be modifed
> are passed in one structure which allows us to make more detailed
> verification.
>
> Still old interface remains present for compatibility reason
> though CRIU itself already switched to PR_SET_MM_MAP on its
> own long ago.
>
> Googling didn't reveal some other users of this operation
> so I think it should be safe to remove this interface.
>
> v2:
>   - Improve warning message
>   - Drop redundant args check
>
> CC: Andrey Vagin <avagin@openvz.org>
> CC: Andrew Morton <akpm@linuxfoundation.org>
> CC: Pavel Emelyanov <xemul@virtuozzo.com>
> CC: Michael Kerrisk <mtk.manpages@gmail.com>
> CC: Yang Shi <yang.shi@linux.alibaba.com>
> CC: Michal Hocko <mhocko@kernel.org>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>

Acked-by: Yang Shi <yang.shi@linux.alibaba.com>

> ---
>   kernel/sys.c |  151 -----------------------------------------------------------
>   1 file changed, 2 insertions(+), 149 deletions(-)
>
> Index: linux-ml.git/kernel/sys.c
> ===================================================================
> --- linux-ml.git.orig/kernel/sys.c
> +++ linux-ml.git/kernel/sys.c
> @@ -2053,163 +2053,16 @@ static int prctl_set_mm_map(int opt, con
>   }
>   #endif /* CONFIG_CHECKPOINT_RESTORE */
>   
> -static int prctl_set_auxv(struct mm_struct *mm, unsigned long addr,
> -			  unsigned long len)
> -{
> -	/*
> -	 * This doesn't move the auxiliary vector itself since it's pinned to
> -	 * mm_struct, but it permits filling the vector with new values.  It's
> -	 * up to the caller to provide sane values here, otherwise userspace
> -	 * tools which use this vector might be unhappy.
> -	 */
> -	unsigned long user_auxv[AT_VECTOR_SIZE];
> -
> -	if (len > sizeof(user_auxv))
> -		return -EINVAL;
> -
> -	if (copy_from_user(user_auxv, (const void __user *)addr, len))
> -		return -EFAULT;
> -
> -	/* Make sure the last entry is always AT_NULL */
> -	user_auxv[AT_VECTOR_SIZE - 2] = 0;
> -	user_auxv[AT_VECTOR_SIZE - 1] = 0;
> -
> -	BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
> -
> -	task_lock(current);
> -	memcpy(mm->saved_auxv, user_auxv, len);
> -	task_unlock(current);
> -
> -	return 0;
> -}
> -
>   static int prctl_set_mm(int opt, unsigned long addr,
>   			unsigned long arg4, unsigned long arg5)
>   {
> -	struct mm_struct *mm = current->mm;
> -	struct prctl_mm_map prctl_map;
> -	struct vm_area_struct *vma;
> -	int error;
> -
> -	if (arg5 || (arg4 && (opt != PR_SET_MM_AUXV &&
> -			      opt != PR_SET_MM_MAP &&
> -			      opt != PR_SET_MM_MAP_SIZE)))
> -		return -EINVAL;
> -
>   #ifdef CONFIG_CHECKPOINT_RESTORE
>   	if (opt == PR_SET_MM_MAP || opt == PR_SET_MM_MAP_SIZE)
>   		return prctl_set_mm_map(opt, (const void __user *)addr, arg4);
>   #endif
>   
> -	if (!capable(CAP_SYS_RESOURCE))
> -		return -EPERM;
> -
> -	if (opt == PR_SET_MM_EXE_FILE)
> -		return prctl_set_mm_exe_file(mm, (unsigned int)addr);
> -
> -	if (opt == PR_SET_MM_AUXV)
> -		return prctl_set_auxv(mm, addr, arg4);
> -
> -	if (addr >= TASK_SIZE || addr < mmap_min_addr)
> -		return -EINVAL;
> -
> -	error = -EINVAL;
> -
> -	down_write(&mm->mmap_sem);
> -	vma = find_vma(mm, addr);
> -
> -	prctl_map.start_code	= mm->start_code;
> -	prctl_map.end_code	= mm->end_code;
> -	prctl_map.start_data	= mm->start_data;
> -	prctl_map.end_data	= mm->end_data;
> -	prctl_map.start_brk	= mm->start_brk;
> -	prctl_map.brk		= mm->brk;
> -	prctl_map.start_stack	= mm->start_stack;
> -	prctl_map.arg_start	= mm->arg_start;
> -	prctl_map.arg_end	= mm->arg_end;
> -	prctl_map.env_start	= mm->env_start;
> -	prctl_map.env_end	= mm->env_end;
> -	prctl_map.auxv		= NULL;
> -	prctl_map.auxv_size	= 0;
> -	prctl_map.exe_fd	= -1;
> -
> -	switch (opt) {
> -	case PR_SET_MM_START_CODE:
> -		prctl_map.start_code = addr;
> -		break;
> -	case PR_SET_MM_END_CODE:
> -		prctl_map.end_code = addr;
> -		break;
> -	case PR_SET_MM_START_DATA:
> -		prctl_map.start_data = addr;
> -		break;
> -	case PR_SET_MM_END_DATA:
> -		prctl_map.end_data = addr;
> -		break;
> -	case PR_SET_MM_START_STACK:
> -		prctl_map.start_stack = addr;
> -		break;
> -	case PR_SET_MM_START_BRK:
> -		prctl_map.start_brk = addr;
> -		break;
> -	case PR_SET_MM_BRK:
> -		prctl_map.brk = addr;
> -		break;
> -	case PR_SET_MM_ARG_START:
> -		prctl_map.arg_start = addr;
> -		break;
> -	case PR_SET_MM_ARG_END:
> -		prctl_map.arg_end = addr;
> -		break;
> -	case PR_SET_MM_ENV_START:
> -		prctl_map.env_start = addr;
> -		break;
> -	case PR_SET_MM_ENV_END:
> -		prctl_map.env_end = addr;
> -		break;
> -	default:
> -		goto out;
> -	}
> -
> -	error = validate_prctl_map(&prctl_map);
> -	if (error)
> -		goto out;
> -
> -	switch (opt) {
> -	/*
> -	 * If command line arguments and environment
> -	 * are placed somewhere else on stack, we can
> -	 * set them up here, ARG_START/END to setup
> -	 * command line argumets and ENV_START/END
> -	 * for environment.
> -	 */
> -	case PR_SET_MM_START_STACK:
> -	case PR_SET_MM_ARG_START:
> -	case PR_SET_MM_ARG_END:
> -	case PR_SET_MM_ENV_START:
> -	case PR_SET_MM_ENV_END:
> -		if (!vma) {
> -			error = -EFAULT;
> -			goto out;
> -		}
> -	}
> -
> -	mm->start_code	= prctl_map.start_code;
> -	mm->end_code	= prctl_map.end_code;
> -	mm->start_data	= prctl_map.start_data;
> -	mm->end_data	= prctl_map.end_data;
> -	mm->start_brk	= prctl_map.start_brk;
> -	mm->brk		= prctl_map.brk;
> -	mm->start_stack	= prctl_map.start_stack;
> -	mm->arg_start	= prctl_map.arg_start;
> -	mm->arg_end	= prctl_map.arg_end;
> -	mm->env_start	= prctl_map.env_start;
> -	mm->env_end	= prctl_map.env_end;
> -
> -	error = 0;
> -out:
> -	up_write(&mm->mmap_sem);
> -	return error;
> +	pr_warn_once("PR_SET_MM_* has been removed. Use PR_SET_MM_MAP instead\n");
> +	return -EINVAL;
>   }
>   
>   #ifdef CONFIG_CHECKPOINT_RESTORE

Re: [PATCH v2] prctl: Deprecate non PR_SET_MM_MAP operations

[Cyrill Gorcunov]

On Thu, Apr 05, 2018 at 08:56:50PM +0200, Michal Hocko wrote:
> On Thu 05-04-18 21:26:51, Cyrill Gorcunov wrote:
> > An ability to manipulate mm_struct fields was introduced in
> > sake of CRIU in first place. Later we provide more suitable
> > and safe operation PR_SET_MM_MAP where all fields to be modifed
> > are passed in one structure which allows us to make more detailed
> > verification.
> 
> I hope this will serve as a memento for future single-user APIs
> proposals. The whole thing was a bad idea since the beginning.

It wasn't a bad idea at all, kernel evolves with time and the
first version of the api served as it should (actually a number
of things get involved like user-namespaces, mm reworks and
such things). So no, it wasn't bad. Rather to make it more
suitable we provided a second version of the api.

> > Still old interface remains present for compatibility reason
> > though CRIU itself already switched to PR_SET_MM_MAP on its
> > own long ago.
> > 
> > Googling didn't reveal some other users of this operation
> > so I think it should be safe to remove this interface.
> > 
> > v2:
> >  - Improve warning message
> >  - Drop redundant args check
> > 
> > CC: Andrey Vagin <avagin@openvz.org>
> > CC: Andrew Morton <akpm@linuxfoundation.org>
> > CC: Pavel Emelyanov <xemul@virtuozzo.com>
> > CC: Michael Kerrisk <mtk.manpages@gmail.com>
> > CC: Yang Shi <yang.shi@linux.alibaba.com>
> > CC: Michal Hocko <mhocko@kernel.org>
> > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> 
> Acked-by: Michal Hocko <mhocko@suse.com>
> 
> and fingers crossed that we haven't grown other users outside of CRIU
> which is quite bound to specific kernels AFAIK.

Surely.

Re: [PATCH v2] prctl: Deprecate non PR_SET_MM_MAP operations

[Andrew Morton]

On Thu, 5 Apr 2018 21:26:51 +0300 Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> Subject: [PATCH v2] prctl: Deprecate non PR_SET_MM_MAP operations

s/Deprecate/remove/ !

> An ability to manipulate mm_struct fields was introduced in
> sake of CRIU in first place. Later we provide more suitable
> and safe operation PR_SET_MM_MAP where all fields to be modifed
> are passed in one structure which allows us to make more detailed
> verification.
> 
> Still old interface remains present for compatibility reason
> though CRIU itself already switched to PR_SET_MM_MAP on its
> own long ago.
> 
> Googling didn't reveal some other users of this operation
> so I think it should be safe to remove this interface.

It does seem a bit sudden.  We could just add a "this interface is
scheduled for removal" warning, then delete the code for real later in
the year.  To give people time to migrate or to complain to us.

Re: [PATCH v2] prctl: Deprecate non PR_SET_MM_MAP operations

[Cyrill Gorcunov]

On Wed, Apr 18, 2018 at 03:28:20PM -0700, Andrew Morton wrote:
> On Thu, 5 Apr 2018 21:26:51 +0300 Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> 
> > Subject: [PATCH v2] prctl: Deprecate non PR_SET_MM_MAP operations
> 
> s/Deprecate/remove/ !

Thanks!

> > 
> > Googling didn't reveal some other users of this operation
> > so I think it should be safe to remove this interface.
> 
> It does seem a bit sudden.  We could just add a "this interface is
> scheduled for removal" warning, then delete the code for real later in
> the year. To give people time to migrate or to complain to us.

Yes, I remember this rule of deprecation but taking into account
how special this code is (and I spent significant amount of time
trying to find a real example of its usage outside of criu)
I think it should be safe to simply remove it. Surely I won't
ever do this with some common syscall.

Re: [v2] prctl: Deprecate non PR_SET_MM_MAP operations

[Sergey Senozhatsky]

On (04/05/18 21:26), Cyrill Gorcunov wrote:
[..]
> -
>  #ifdef CONFIG_CHECKPOINT_RESTORE
>  	if (opt == PR_SET_MM_MAP || opt == PR_SET_MM_MAP_SIZE)
>  		return prctl_set_mm_map(opt, (const void __user *)addr, arg4);
>  #endif
>  
> -	if (!capable(CAP_SYS_RESOURCE))
> -		return -EPERM;
> -
> -	if (opt == PR_SET_MM_EXE_FILE)
> -		return prctl_set_mm_exe_file(mm, (unsigned int)addr);
> -
> -	if (opt == PR_SET_MM_AUXV)
> -		return prctl_set_auxv(mm, addr, arg4);

Then validate_prctl_map() and prctl_set_mm_exe_file() can be moved
under CONFIG_CHECKPOINT_RESTORE ifdef.

---

 kernel/sys.c | 126 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 63 insertions(+), 63 deletions(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index 6bdffe264303..86e5ef1a5612 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1815,68 +1815,7 @@ SYSCALL_DEFINE1(umask, int, mask)
 	return mask;
 }
 
-static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
-{
-	struct fd exe;
-	struct file *old_exe, *exe_file;
-	struct inode *inode;
-	int err;
-
-	exe = fdget(fd);
-	if (!exe.file)
-		return -EBADF;
-
-	inode = file_inode(exe.file);
-
-	/*
-	 * Because the original mm->exe_file points to executable file, make
-	 * sure that this one is executable as well, to avoid breaking an
-	 * overall picture.
-	 */
-	err = -EACCES;
-	if (!S_ISREG(inode->i_mode) || path_noexec(&exe.file->f_path))
-		goto exit;
-
-	err = inode_permission(inode, MAY_EXEC);
-	if (err)
-		goto exit;
-
-	/*
-	 * Forbid mm->exe_file change if old file still mapped.
-	 */
-	exe_file = get_mm_exe_file(mm);
-	err = -EBUSY;
-	if (exe_file) {
-		struct vm_area_struct *vma;
-
-		down_read(&mm->mmap_sem);
-		for (vma = mm->mmap; vma; vma = vma->vm_next) {
-			if (!vma->vm_file)
-				continue;
-			if (path_equal(&vma->vm_file->f_path,
-				       &exe_file->f_path))
-				goto exit_err;
-		}
-
-		up_read(&mm->mmap_sem);
-		fput(exe_file);
-	}
-
-	err = 0;
-	/* set the new file, lockless */
-	get_file(exe.file);
-	old_exe = xchg(&mm->exe_file, exe.file);
-	if (old_exe)
-		fput(old_exe);
-exit:
-	fdput(exe);
-	return err;
-exit_err:
-	up_read(&mm->mmap_sem);
-	fput(exe_file);
-	goto exit;
-}
-
+#ifdef CONFIG_CHECKPOINT_RESTORE
 /*
  * WARNING: we don't require any capability here so be very careful
  * in what is allowed for modification from userspace.
@@ -1968,7 +1907,68 @@ static int validate_prctl_map(struct prctl_mm_map *prctl_map)
 	return error;
 }
 
-#ifdef CONFIG_CHECKPOINT_RESTORE
+static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
+{
+	struct fd exe;
+	struct file *old_exe, *exe_file;
+	struct inode *inode;
+	int err;
+
+	exe = fdget(fd);
+	if (!exe.file)
+		return -EBADF;
+
+	inode = file_inode(exe.file);
+
+	/*
+	 * Because the original mm->exe_file points to executable file, make
+	 * sure that this one is executable as well, to avoid breaking an
+	 * overall picture.
+	 */
+	err = -EACCES;
+	if (!S_ISREG(inode->i_mode) || path_noexec(&exe.file->f_path))
+		goto exit;
+
+	err = inode_permission(inode, MAY_EXEC);
+	if (err)
+		goto exit;
+
+	/*
+	 * Forbid mm->exe_file change if old file still mapped.
+	 */
+	exe_file = get_mm_exe_file(mm);
+	err = -EBUSY;
+	if (exe_file) {
+		struct vm_area_struct *vma;
+
+		down_read(&mm->mmap_sem);
+		for (vma = mm->mmap; vma; vma = vma->vm_next) {
+			if (!vma->vm_file)
+				continue;
+			if (path_equal(&vma->vm_file->f_path,
+				       &exe_file->f_path))
+				goto exit_err;
+		}
+
+		up_read(&mm->mmap_sem);
+		fput(exe_file);
+	}
+
+	err = 0;
+	/* set the new file, lockless */
+	get_file(exe.file);
+	old_exe = xchg(&mm->exe_file, exe.file);
+	if (old_exe)
+		fput(old_exe);
+exit:
+	fdput(exe);
+	return err;
+exit_err:
+	up_read(&mm->mmap_sem);
+	fput(exe_file);
+	goto exit;
+}
+
 static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data_size)
 {
 	struct prctl_mm_map prctl_map = { .exe_fd = (u32)-1, };

Re: [v2] prctl: Deprecate non PR_SET_MM_MAP operations

[Cyrill Gorcunov]

On Fri, Apr 20, 2018 at 11:38:09AM +0900, Sergey Senozhatsky wrote:
> On (04/05/18 21:26), Cyrill Gorcunov wrote:
> [..]
> > -
> >  #ifdef CONFIG_CHECKPOINT_RESTORE
> >  	if (opt == PR_SET_MM_MAP || opt == PR_SET_MM_MAP_SIZE)
> >  		return prctl_set_mm_map(opt, (const void __user *)addr, arg4);
> >  #endif
> >  
> > -	if (!capable(CAP_SYS_RESOURCE))
> > -		return -EPERM;
> > -
> > -	if (opt == PR_SET_MM_EXE_FILE)
> > -		return prctl_set_mm_exe_file(mm, (unsigned int)addr);
> > -
> > -	if (opt == PR_SET_MM_AUXV)
> > -		return prctl_set_auxv(mm, addr, arg4);
> 
> Then validate_prctl_map() and prctl_set_mm_exe_file() can be moved
> under CONFIG_CHECKPOINT_RESTORE ifdef.

I don't mind. Could you please make the patch on top of linux-next?

Re: [v2] prctl: Deprecate non PR_SET_MM_MAP operations

[Sergey Senozhatsky]

On (04/20/18 10:02), Cyrill Gorcunov wrote:
> On Fri, Apr 20, 2018 at 11:38:09AM +0900, Sergey Senozhatsky wrote:
> > On (04/05/18 21:26), Cyrill Gorcunov wrote:
> > [..]
> > > -
> > >  #ifdef CONFIG_CHECKPOINT_RESTORE
> > >  	if (opt == PR_SET_MM_MAP || opt == PR_SET_MM_MAP_SIZE)
> > >  		return prctl_set_mm_map(opt, (const void __user *)addr, arg4);
> > >  #endif
> > >  
> > > -	if (!capable(CAP_SYS_RESOURCE))
> > > -		return -EPERM;
> > > -
> > > -	if (opt == PR_SET_MM_EXE_FILE)
> > > -		return prctl_set_mm_exe_file(mm, (unsigned int)addr);
> > > -
> > > -	if (opt == PR_SET_MM_AUXV)
> > > -		return prctl_set_auxv(mm, addr, arg4);
> > 
> > Then validate_prctl_map() and prctl_set_mm_exe_file() can be moved
> > under CONFIG_CHECKPOINT_RESTORE ifdef.
> 
> I don't mind. Could you please make the patch on top of linux-next?

As far as I can see, it's not in linux-next yet. So the following is
against the mmots tree. I wouldn't mind it if we could just squash the
patches.

=======================================================================

From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: [PATCH] prctl: Don't compile some of prctl functions when CRUI
 disabled

CHECKPOINT_RESTORE is the only user of validate_prctl_map()
and prctl_set_mm_exe_file(), so we can move those two under
CONFIG_CHECKPOINT_RESTORE.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 kernel/sys.c | 126 +++++++++++++++++++++++++--------------------------
 1 file changed, 63 insertions(+), 63 deletions(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index 6bdffe264303..86e5ef1a5612 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1815,68 +1815,7 @@ SYSCALL_DEFINE1(umask, int, mask)
 	return mask;
 }
 
-static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
-{
-	struct fd exe;
-	struct file *old_exe, *exe_file;
-	struct inode *inode;
-	int err;
-
-	exe = fdget(fd);
-	if (!exe.file)
-		return -EBADF;
-
-	inode = file_inode(exe.file);
-
-	/*
-	 * Because the original mm->exe_file points to executable file, make
-	 * sure that this one is executable as well, to avoid breaking an
-	 * overall picture.
-	 */
-	err = -EACCES;
-	if (!S_ISREG(inode->i_mode) || path_noexec(&exe.file->f_path))
-		goto exit;
-
-	err = inode_permission(inode, MAY_EXEC);
-	if (err)
-		goto exit;
-
-	/*
-	 * Forbid mm->exe_file change if old file still mapped.
-	 */
-	exe_file = get_mm_exe_file(mm);
-	err = -EBUSY;
-	if (exe_file) {
-		struct vm_area_struct *vma;
-
-		down_read(&mm->mmap_sem);
-		for (vma = mm->mmap; vma; vma = vma->vm_next) {
-			if (!vma->vm_file)
-				continue;
-			if (path_equal(&vma->vm_file->f_path,
-				       &exe_file->f_path))
-				goto exit_err;
-		}
-
-		up_read(&mm->mmap_sem);
-		fput(exe_file);
-	}
-
-	err = 0;
-	/* set the new file, lockless */
-	get_file(exe.file);
-	old_exe = xchg(&mm->exe_file, exe.file);
-	if (old_exe)
-		fput(old_exe);
-exit:
-	fdput(exe);
-	return err;
-exit_err:
-	up_read(&mm->mmap_sem);
-	fput(exe_file);
-	goto exit;
-}
-
+#ifdef CONFIG_CHECKPOINT_RESTORE
 /*
  * WARNING: we don't require any capability here so be very careful
  * in what is allowed for modification from userspace.
@@ -1968,7 +1907,68 @@ static int validate_prctl_map(struct prctl_mm_map *prctl_map)
 	return error;
 }
 
-#ifdef CONFIG_CHECKPOINT_RESTORE
+static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
+{
+	struct fd exe;
+	struct file *old_exe, *exe_file;
+	struct inode *inode;
+	int err;
+
+	exe = fdget(fd);
+	if (!exe.file)
+		return -EBADF;
+
+	inode = file_inode(exe.file);
+
+	/*
+	 * Because the original mm->exe_file points to executable file, make
+	 * sure that this one is executable as well, to avoid breaking an
+	 * overall picture.
+	 */
+	err = -EACCES;
+	if (!S_ISREG(inode->i_mode) || path_noexec(&exe.file->f_path))
+		goto exit;
+
+	err = inode_permission(inode, MAY_EXEC);
+	if (err)
+		goto exit;
+
+	/*
+	 * Forbid mm->exe_file change if old file still mapped.
+	 */
+	exe_file = get_mm_exe_file(mm);
+	err = -EBUSY;
+	if (exe_file) {
+		struct vm_area_struct *vma;
+
+		down_read(&mm->mmap_sem);
+		for (vma = mm->mmap; vma; vma = vma->vm_next) {
+			if (!vma->vm_file)
+				continue;
+			if (path_equal(&vma->vm_file->f_path,
+				       &exe_file->f_path))
+				goto exit_err;
+		}
+
+		up_read(&mm->mmap_sem);
+		fput(exe_file);
+	}
+
+	err = 0;
+	/* set the new file, lockless */
+	get_file(exe.file);
+	old_exe = xchg(&mm->exe_file, exe.file);
+	if (old_exe)
+		fput(old_exe);
+exit:
+	fdput(exe);
+	return err;
+exit_err:
+	up_read(&mm->mmap_sem);
+	fput(exe_file);
+	goto exit;
+}
+
 static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data_size)
 {
 	struct prctl_mm_map prctl_map = { .exe_fd = (u32)-1, };
-- 
2.17.0

Re: [v2] prctl: Deprecate non PR_SET_MM_MAP operations

[Cyrill Gorcunov]

On Fri, Apr 20, 2018 at 04:29:03PM +0900, Sergey Senozhatsky wrote:
> > 
> > I don't mind. Could you please make the patch on top of linux-next?
> 
> As far as I can see, it's not in linux-next yet. So the following is
> against the mmots tree. I wouldn't mind it if we could just squash the
> patches.
> 
> =======================================================================
> 
> From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Subject: [PATCH] prctl: Don't compile some of prctl functions when CRUI
>  disabled
> 
> CHECKPOINT_RESTORE is the only user of validate_prctl_map()
> and prctl_set_mm_exe_file(), so we can move those two under
> CONFIG_CHECKPOINT_RESTORE.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>

Thanks a lot, Sergey!

Re: [PATCH] prctl: Don't compile some of prctl functions when CRUI

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 9159 bytes --]

Hi Sergey,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc1 next-20180420]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sergey-Senozhatsky/prctl-Don-t-compile-some-of-prctl-functions-when-CRUI/20180421-040826
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   kernel/sys.c: In function 'prctl_set_mm':
>> kernel/sys.c:2108:10: error: implicit declaration of function 'prctl_set_mm_exe_file'; did you mean 'set_mm_exe_file'? [-Werror=implicit-function-declaration]
      return prctl_set_mm_exe_file(mm, (unsigned int)addr);
             ^~~~~~~~~~~~~~~~~~~~~
             set_mm_exe_file
>> kernel/sys.c:2174:10: error: implicit declaration of function 'validate_prctl_map'; did you mean 'validate_creds'? [-Werror=implicit-function-declaration]
     error = validate_prctl_map(&prctl_map);
             ^~~~~~~~~~~~~~~~~~
             validate_creds
   cc1: some warnings being treated as errors

vim +2108 kernel/sys.c

f606b77f1 Cyrill Gorcunov 2014-10-09  2103  
79f0713d4 Cyrill Gorcunov 2012-03-15  2104  	if (!capable(CAP_SYS_RESOURCE))
028ee4be3 Cyrill Gorcunov 2012-01-12  2105  		return -EPERM;
028ee4be3 Cyrill Gorcunov 2012-01-12  2106  
6e399cd14 Davidlohr Bueso 2015-04-16  2107  	if (opt == PR_SET_MM_EXE_FILE)
6e399cd14 Davidlohr Bueso 2015-04-16 @2108  		return prctl_set_mm_exe_file(mm, (unsigned int)addr);
b32dfe377 Cyrill Gorcunov 2012-05-31  2109  
4a00e9df2 Alexey Dobriyan 2015-06-25  2110  	if (opt == PR_SET_MM_AUXV)
4a00e9df2 Alexey Dobriyan 2015-06-25  2111  		return prctl_set_auxv(mm, addr, arg4);
4a00e9df2 Alexey Dobriyan 2015-06-25  2112  
1ad75b9e1 Cyrill Gorcunov 2012-06-07  2113  	if (addr >= TASK_SIZE || addr < mmap_min_addr)
028ee4be3 Cyrill Gorcunov 2012-01-12  2114  		return -EINVAL;
028ee4be3 Cyrill Gorcunov 2012-01-12  2115  
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2116  	error = -EINVAL;
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2117  
ddf1d398e Mateusz Guzik   2016-01-20  2118  	down_write(&mm->mmap_sem);
028ee4be3 Cyrill Gorcunov 2012-01-12  2119  	vma = find_vma(mm, addr);
028ee4be3 Cyrill Gorcunov 2012-01-12  2120  
4a00e9df2 Alexey Dobriyan 2015-06-25  2121  	prctl_map.start_code	= mm->start_code;
4a00e9df2 Alexey Dobriyan 2015-06-25  2122  	prctl_map.end_code	= mm->end_code;
4a00e9df2 Alexey Dobriyan 2015-06-25  2123  	prctl_map.start_data	= mm->start_data;
4a00e9df2 Alexey Dobriyan 2015-06-25  2124  	prctl_map.end_data	= mm->end_data;
4a00e9df2 Alexey Dobriyan 2015-06-25  2125  	prctl_map.start_brk	= mm->start_brk;
4a00e9df2 Alexey Dobriyan 2015-06-25  2126  	prctl_map.brk		= mm->brk;
4a00e9df2 Alexey Dobriyan 2015-06-25  2127  	prctl_map.start_stack	= mm->start_stack;
4a00e9df2 Alexey Dobriyan 2015-06-25  2128  	prctl_map.arg_start	= mm->arg_start;
4a00e9df2 Alexey Dobriyan 2015-06-25  2129  	prctl_map.arg_end	= mm->arg_end;
4a00e9df2 Alexey Dobriyan 2015-06-25  2130  	prctl_map.env_start	= mm->env_start;
4a00e9df2 Alexey Dobriyan 2015-06-25  2131  	prctl_map.env_end	= mm->env_end;
4a00e9df2 Alexey Dobriyan 2015-06-25  2132  	prctl_map.auxv		= NULL;
4a00e9df2 Alexey Dobriyan 2015-06-25  2133  	prctl_map.auxv_size	= 0;
4a00e9df2 Alexey Dobriyan 2015-06-25  2134  	prctl_map.exe_fd	= -1;
4a00e9df2 Alexey Dobriyan 2015-06-25  2135  
028ee4be3 Cyrill Gorcunov 2012-01-12  2136  	switch (opt) {
028ee4be3 Cyrill Gorcunov 2012-01-12  2137  	case PR_SET_MM_START_CODE:
4a00e9df2 Alexey Dobriyan 2015-06-25  2138  		prctl_map.start_code = addr;
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2139  		break;
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2140  	case PR_SET_MM_END_CODE:
4a00e9df2 Alexey Dobriyan 2015-06-25  2141  		prctl_map.end_code = addr;
028ee4be3 Cyrill Gorcunov 2012-01-12  2142  		break;
028ee4be3 Cyrill Gorcunov 2012-01-12  2143  	case PR_SET_MM_START_DATA:
4a00e9df2 Alexey Dobriyan 2015-06-25  2144  		prctl_map.start_data = addr;
028ee4be3 Cyrill Gorcunov 2012-01-12  2145  		break;
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2146  	case PR_SET_MM_END_DATA:
4a00e9df2 Alexey Dobriyan 2015-06-25  2147  		prctl_map.end_data = addr;
4a00e9df2 Alexey Dobriyan 2015-06-25  2148  		break;
4a00e9df2 Alexey Dobriyan 2015-06-25  2149  	case PR_SET_MM_START_STACK:
4a00e9df2 Alexey Dobriyan 2015-06-25  2150  		prctl_map.start_stack = addr;
028ee4be3 Cyrill Gorcunov 2012-01-12  2151  		break;
028ee4be3 Cyrill Gorcunov 2012-01-12  2152  	case PR_SET_MM_START_BRK:
4a00e9df2 Alexey Dobriyan 2015-06-25  2153  		prctl_map.start_brk = addr;
028ee4be3 Cyrill Gorcunov 2012-01-12  2154  		break;
028ee4be3 Cyrill Gorcunov 2012-01-12  2155  	case PR_SET_MM_BRK:
4a00e9df2 Alexey Dobriyan 2015-06-25  2156  		prctl_map.brk = addr;
4a00e9df2 Alexey Dobriyan 2015-06-25  2157  		break;
4a00e9df2 Alexey Dobriyan 2015-06-25  2158  	case PR_SET_MM_ARG_START:
4a00e9df2 Alexey Dobriyan 2015-06-25  2159  		prctl_map.arg_start = addr;
4a00e9df2 Alexey Dobriyan 2015-06-25  2160  		break;
4a00e9df2 Alexey Dobriyan 2015-06-25  2161  	case PR_SET_MM_ARG_END:
4a00e9df2 Alexey Dobriyan 2015-06-25  2162  		prctl_map.arg_end = addr;
4a00e9df2 Alexey Dobriyan 2015-06-25  2163  		break;
4a00e9df2 Alexey Dobriyan 2015-06-25  2164  	case PR_SET_MM_ENV_START:
4a00e9df2 Alexey Dobriyan 2015-06-25  2165  		prctl_map.env_start = addr;
4a00e9df2 Alexey Dobriyan 2015-06-25  2166  		break;
4a00e9df2 Alexey Dobriyan 2015-06-25  2167  	case PR_SET_MM_ENV_END:
4a00e9df2 Alexey Dobriyan 2015-06-25  2168  		prctl_map.env_end = addr;
4a00e9df2 Alexey Dobriyan 2015-06-25  2169  		break;
4a00e9df2 Alexey Dobriyan 2015-06-25  2170  	default:
028ee4be3 Cyrill Gorcunov 2012-01-12  2171  		goto out;
4a00e9df2 Alexey Dobriyan 2015-06-25  2172  	}
028ee4be3 Cyrill Gorcunov 2012-01-12  2173  
4a00e9df2 Alexey Dobriyan 2015-06-25 @2174  	error = validate_prctl_map(&prctl_map);
4a00e9df2 Alexey Dobriyan 2015-06-25  2175  	if (error)
028ee4be3 Cyrill Gorcunov 2012-01-12  2176  		goto out;
028ee4be3 Cyrill Gorcunov 2012-01-12  2177  
4a00e9df2 Alexey Dobriyan 2015-06-25  2178  	switch (opt) {
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2179  	/*
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2180  	 * If command line arguments and environment
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2181  	 * are placed somewhere else on stack, we can
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2182  	 * set them up here, ARG_START/END to setup
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2183  	 * command line argumets and ENV_START/END
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2184  	 * for environment.
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2185  	 */
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2186  	case PR_SET_MM_START_STACK:
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2187  	case PR_SET_MM_ARG_START:
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2188  	case PR_SET_MM_ARG_END:
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2189  	case PR_SET_MM_ENV_START:
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2190  	case PR_SET_MM_ENV_END:
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2191  		if (!vma) {
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2192  			error = -EFAULT;
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2193  			goto out;
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2194  		}
028ee4be3 Cyrill Gorcunov 2012-01-12  2195  	}
028ee4be3 Cyrill Gorcunov 2012-01-12  2196  
4a00e9df2 Alexey Dobriyan 2015-06-25  2197  	mm->start_code	= prctl_map.start_code;
4a00e9df2 Alexey Dobriyan 2015-06-25  2198  	mm->end_code	= prctl_map.end_code;
4a00e9df2 Alexey Dobriyan 2015-06-25  2199  	mm->start_data	= prctl_map.start_data;
4a00e9df2 Alexey Dobriyan 2015-06-25  2200  	mm->end_data	= prctl_map.end_data;
4a00e9df2 Alexey Dobriyan 2015-06-25  2201  	mm->start_brk	= prctl_map.start_brk;
4a00e9df2 Alexey Dobriyan 2015-06-25  2202  	mm->brk		= prctl_map.brk;
4a00e9df2 Alexey Dobriyan 2015-06-25  2203  	mm->start_stack	= prctl_map.start_stack;
4a00e9df2 Alexey Dobriyan 2015-06-25  2204  	mm->arg_start	= prctl_map.arg_start;
4a00e9df2 Alexey Dobriyan 2015-06-25  2205  	mm->arg_end	= prctl_map.arg_end;
4a00e9df2 Alexey Dobriyan 2015-06-25  2206  	mm->env_start	= prctl_map.env_start;
4a00e9df2 Alexey Dobriyan 2015-06-25  2207  	mm->env_end	= prctl_map.env_end;
4a00e9df2 Alexey Dobriyan 2015-06-25  2208  
028ee4be3 Cyrill Gorcunov 2012-01-12  2209  	error = 0;
028ee4be3 Cyrill Gorcunov 2012-01-12  2210  out:
ddf1d398e Mateusz Guzik   2016-01-20  2211  	up_write(&mm->mmap_sem);
028ee4be3 Cyrill Gorcunov 2012-01-12  2212  	return error;
028ee4be3 Cyrill Gorcunov 2012-01-12  2213  }
300f786b2 Cyrill Gorcunov 2012-06-07  2214  

:::::: The code at line 2108 was first introduced by commit
:::::: 6e399cd144d8500ffb5d40fa6848890e2580a80a prctl: avoid using mmap_sem for exe_file serialization

:::::: TO: Davidlohr Bueso <dave@stgolabs.net>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6293 bytes --]

Re: [PATCH] prctl: Don't compile some of prctl functions when CRUI

[Andrew Morton]

On Sat, 21 Apr 2018 04:37:41 +0800 kbuild test robot <lkp@intel.com> wrote:

> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.17-rc1 next-20180420]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Sergey-Senozhatsky/prctl-Don-t-compile-some-of-prctl-functions-when-CRUI/20180421-040826
> config: i386-tinyconfig (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
>    kernel/sys.c: In function 'prctl_set_mm':
> >> kernel/sys.c:2108:10: error: implicit declaration of function 'prctl_set_mm_exe_file'; did you mean 'set_mm_exe_file'? [-Werror=implicit-function-declaration]
>       return prctl_set_mm_exe_file(mm, (unsigned int)addr);
>              ^~~~~~~~~~~~~~~~~~~~~

"prctl: don't compile some of prctl functions when CRUI disabled" was a
fix against "prctl: remove non PR_SET_MM_MAP operations", and it
appears that the base patch wasn't applied when this test was
performed.

Re: [PATCH] prctl: Don't compile some of prctl functions when CRUI

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 9079 bytes --]

Hi Sergey,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc1 next-20180420]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sergey-Senozhatsky/prctl-Don-t-compile-some-of-prctl-functions-when-CRUI/20180421-040826
config: i386-randconfig-s1-201815 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   kernel/sys.c: In function 'prctl_set_mm':
>> kernel/sys.c:2108:10: error: implicit declaration of function 'prctl_set_mm_exe_file' [-Werror=implicit-function-declaration]
      return prctl_set_mm_exe_file(mm, (unsigned int)addr);
             ^~~~~~~~~~~~~~~~~~~~~
>> kernel/sys.c:2174:10: error: implicit declaration of function 'validate_prctl_map' [-Werror=implicit-function-declaration]
     error = validate_prctl_map(&prctl_map);
             ^~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/prctl_set_mm_exe_file +2108 kernel/sys.c

f606b77f1 Cyrill Gorcunov 2014-10-09  2103  
79f0713d4 Cyrill Gorcunov 2012-03-15  2104  	if (!capable(CAP_SYS_RESOURCE))
028ee4be3 Cyrill Gorcunov 2012-01-12  2105  		return -EPERM;
028ee4be3 Cyrill Gorcunov 2012-01-12  2106  
6e399cd14 Davidlohr Bueso 2015-04-16  2107  	if (opt == PR_SET_MM_EXE_FILE)
6e399cd14 Davidlohr Bueso 2015-04-16 @2108  		return prctl_set_mm_exe_file(mm, (unsigned int)addr);
b32dfe377 Cyrill Gorcunov 2012-05-31  2109  
4a00e9df2 Alexey Dobriyan 2015-06-25  2110  	if (opt == PR_SET_MM_AUXV)
4a00e9df2 Alexey Dobriyan 2015-06-25  2111  		return prctl_set_auxv(mm, addr, arg4);
4a00e9df2 Alexey Dobriyan 2015-06-25  2112  
1ad75b9e1 Cyrill Gorcunov 2012-06-07  2113  	if (addr >= TASK_SIZE || addr < mmap_min_addr)
028ee4be3 Cyrill Gorcunov 2012-01-12  2114  		return -EINVAL;
028ee4be3 Cyrill Gorcunov 2012-01-12  2115  
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2116  	error = -EINVAL;
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2117  
ddf1d398e Mateusz Guzik   2016-01-20  2118  	down_write(&mm->mmap_sem);
028ee4be3 Cyrill Gorcunov 2012-01-12  2119  	vma = find_vma(mm, addr);
028ee4be3 Cyrill Gorcunov 2012-01-12  2120  
4a00e9df2 Alexey Dobriyan 2015-06-25  2121  	prctl_map.start_code	= mm->start_code;
4a00e9df2 Alexey Dobriyan 2015-06-25  2122  	prctl_map.end_code	= mm->end_code;
4a00e9df2 Alexey Dobriyan 2015-06-25  2123  	prctl_map.start_data	= mm->start_data;
4a00e9df2 Alexey Dobriyan 2015-06-25  2124  	prctl_map.end_data	= mm->end_data;
4a00e9df2 Alexey Dobriyan 2015-06-25  2125  	prctl_map.start_brk	= mm->start_brk;
4a00e9df2 Alexey Dobriyan 2015-06-25  2126  	prctl_map.brk		= mm->brk;
4a00e9df2 Alexey Dobriyan 2015-06-25  2127  	prctl_map.start_stack	= mm->start_stack;
4a00e9df2 Alexey Dobriyan 2015-06-25  2128  	prctl_map.arg_start	= mm->arg_start;
4a00e9df2 Alexey Dobriyan 2015-06-25  2129  	prctl_map.arg_end	= mm->arg_end;
4a00e9df2 Alexey Dobriyan 2015-06-25  2130  	prctl_map.env_start	= mm->env_start;
4a00e9df2 Alexey Dobriyan 2015-06-25  2131  	prctl_map.env_end	= mm->env_end;
4a00e9df2 Alexey Dobriyan 2015-06-25  2132  	prctl_map.auxv		= NULL;
4a00e9df2 Alexey Dobriyan 2015-06-25  2133  	prctl_map.auxv_size	= 0;
4a00e9df2 Alexey Dobriyan 2015-06-25  2134  	prctl_map.exe_fd	= -1;
4a00e9df2 Alexey Dobriyan 2015-06-25  2135  
028ee4be3 Cyrill Gorcunov 2012-01-12  2136  	switch (opt) {
028ee4be3 Cyrill Gorcunov 2012-01-12  2137  	case PR_SET_MM_START_CODE:
4a00e9df2 Alexey Dobriyan 2015-06-25  2138  		prctl_map.start_code = addr;
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2139  		break;
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2140  	case PR_SET_MM_END_CODE:
4a00e9df2 Alexey Dobriyan 2015-06-25  2141  		prctl_map.end_code = addr;
028ee4be3 Cyrill Gorcunov 2012-01-12  2142  		break;
028ee4be3 Cyrill Gorcunov 2012-01-12  2143  	case PR_SET_MM_START_DATA:
4a00e9df2 Alexey Dobriyan 2015-06-25  2144  		prctl_map.start_data = addr;
028ee4be3 Cyrill Gorcunov 2012-01-12  2145  		break;
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2146  	case PR_SET_MM_END_DATA:
4a00e9df2 Alexey Dobriyan 2015-06-25  2147  		prctl_map.end_data = addr;
4a00e9df2 Alexey Dobriyan 2015-06-25  2148  		break;
4a00e9df2 Alexey Dobriyan 2015-06-25  2149  	case PR_SET_MM_START_STACK:
4a00e9df2 Alexey Dobriyan 2015-06-25  2150  		prctl_map.start_stack = addr;
028ee4be3 Cyrill Gorcunov 2012-01-12  2151  		break;
028ee4be3 Cyrill Gorcunov 2012-01-12  2152  	case PR_SET_MM_START_BRK:
4a00e9df2 Alexey Dobriyan 2015-06-25  2153  		prctl_map.start_brk = addr;
028ee4be3 Cyrill Gorcunov 2012-01-12  2154  		break;
028ee4be3 Cyrill Gorcunov 2012-01-12  2155  	case PR_SET_MM_BRK:
4a00e9df2 Alexey Dobriyan 2015-06-25  2156  		prctl_map.brk = addr;
4a00e9df2 Alexey Dobriyan 2015-06-25  2157  		break;
4a00e9df2 Alexey Dobriyan 2015-06-25  2158  	case PR_SET_MM_ARG_START:
4a00e9df2 Alexey Dobriyan 2015-06-25  2159  		prctl_map.arg_start = addr;
4a00e9df2 Alexey Dobriyan 2015-06-25  2160  		break;
4a00e9df2 Alexey Dobriyan 2015-06-25  2161  	case PR_SET_MM_ARG_END:
4a00e9df2 Alexey Dobriyan 2015-06-25  2162  		prctl_map.arg_end = addr;
4a00e9df2 Alexey Dobriyan 2015-06-25  2163  		break;
4a00e9df2 Alexey Dobriyan 2015-06-25  2164  	case PR_SET_MM_ENV_START:
4a00e9df2 Alexey Dobriyan 2015-06-25  2165  		prctl_map.env_start = addr;
4a00e9df2 Alexey Dobriyan 2015-06-25  2166  		break;
4a00e9df2 Alexey Dobriyan 2015-06-25  2167  	case PR_SET_MM_ENV_END:
4a00e9df2 Alexey Dobriyan 2015-06-25  2168  		prctl_map.env_end = addr;
4a00e9df2 Alexey Dobriyan 2015-06-25  2169  		break;
4a00e9df2 Alexey Dobriyan 2015-06-25  2170  	default:
028ee4be3 Cyrill Gorcunov 2012-01-12  2171  		goto out;
4a00e9df2 Alexey Dobriyan 2015-06-25  2172  	}
028ee4be3 Cyrill Gorcunov 2012-01-12  2173  
4a00e9df2 Alexey Dobriyan 2015-06-25 @2174  	error = validate_prctl_map(&prctl_map);
4a00e9df2 Alexey Dobriyan 2015-06-25  2175  	if (error)
028ee4be3 Cyrill Gorcunov 2012-01-12  2176  		goto out;
028ee4be3 Cyrill Gorcunov 2012-01-12  2177  
4a00e9df2 Alexey Dobriyan 2015-06-25  2178  	switch (opt) {
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2179  	/*
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2180  	 * If command line arguments and environment
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2181  	 * are placed somewhere else on stack, we can
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2182  	 * set them up here, ARG_START/END to setup
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2183  	 * command line argumets and ENV_START/END
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2184  	 * for environment.
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2185  	 */
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2186  	case PR_SET_MM_START_STACK:
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2187  	case PR_SET_MM_ARG_START:
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2188  	case PR_SET_MM_ARG_END:
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2189  	case PR_SET_MM_ENV_START:
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2190  	case PR_SET_MM_ENV_END:
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2191  		if (!vma) {
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2192  			error = -EFAULT;
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2193  			goto out;
fe8c7f5cb Cyrill Gorcunov 2012-05-31  2194  		}
028ee4be3 Cyrill Gorcunov 2012-01-12  2195  	}
028ee4be3 Cyrill Gorcunov 2012-01-12  2196  
4a00e9df2 Alexey Dobriyan 2015-06-25  2197  	mm->start_code	= prctl_map.start_code;
4a00e9df2 Alexey Dobriyan 2015-06-25  2198  	mm->end_code	= prctl_map.end_code;
4a00e9df2 Alexey Dobriyan 2015-06-25  2199  	mm->start_data	= prctl_map.start_data;
4a00e9df2 Alexey Dobriyan 2015-06-25  2200  	mm->end_data	= prctl_map.end_data;
4a00e9df2 Alexey Dobriyan 2015-06-25  2201  	mm->start_brk	= prctl_map.start_brk;
4a00e9df2 Alexey Dobriyan 2015-06-25  2202  	mm->brk		= prctl_map.brk;
4a00e9df2 Alexey Dobriyan 2015-06-25  2203  	mm->start_stack	= prctl_map.start_stack;
4a00e9df2 Alexey Dobriyan 2015-06-25  2204  	mm->arg_start	= prctl_map.arg_start;
4a00e9df2 Alexey Dobriyan 2015-06-25  2205  	mm->arg_end	= prctl_map.arg_end;
4a00e9df2 Alexey Dobriyan 2015-06-25  2206  	mm->env_start	= prctl_map.env_start;
4a00e9df2 Alexey Dobriyan 2015-06-25  2207  	mm->env_end	= prctl_map.env_end;
4a00e9df2 Alexey Dobriyan 2015-06-25  2208  
028ee4be3 Cyrill Gorcunov 2012-01-12  2209  	error = 0;
028ee4be3 Cyrill Gorcunov 2012-01-12  2210  out:
ddf1d398e Mateusz Guzik   2016-01-20  2211  	up_write(&mm->mmap_sem);
028ee4be3 Cyrill Gorcunov 2012-01-12  2212  	return error;
028ee4be3 Cyrill Gorcunov 2012-01-12  2213  }
300f786b2 Cyrill Gorcunov 2012-06-07  2214  

:::::: The code at line 2108 was first introduced by commit
:::::: 6e399cd144d8500ffb5d40fa6848890e2580a80a prctl: avoid using mmap_sem for exe_file serialization

:::::: TO: Davidlohr Bueso <dave@stgolabs.net>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28656 bytes --]

Re: [PATCH] prctl: Don't compile some of prctl functions when CRUI

[Michal Koutný]

Hi.

I see this discussion somewhat faded away since the previous year.

There was rework [1] that reduced (ab)use of mmap_sem in prctl
functions.

Actually, there still remains the down_write() in prctl_set_mm.
I considered at least replacing it with the mm_struct.arg_lock +
down_read() but then I learnt about this thread intending to remove that
part completely. I wouldn't oppose if CRIU is the sole (aware) user.

Ad the bot build issue, I could build the kernel both with
CONFIG_CHECKPOINT_RESTORE and without CONFIG_CHECKPOINT_RESTORE just
fine after applying the two proposed patches.

What is the current state? Perhaps, this change should be CCed to
linux-api@vger.kernel.org(?).

Thanks,
Michal

[1] https://lore.kernel.org/lkml/1523730291-109696-1-git-send-email-yang.shi@linux.alibaba.com/T/

Re: [PATCH] prctl: Don't compile some of prctl functions when CRUI

[Cyrill Gorcunov]

On Wed, Apr 17, 2019 at 02:23:50PM +0200, Michal Koutný wrote:
> Hi.
> 
> I see this discussion somewhat faded away since the previous year.
> 
> There was rework [1] that reduced (ab)use of mmap_sem in prctl
> functions.
> 
> Actually, there still remains the down_write() in prctl_set_mm.
> I considered at least replacing it with the mm_struct.arg_lock +
> down_read() but then I learnt about this thread intending to remove that
> part completely. I wouldn't oppose if CRIU is the sole (aware) user.
> 
> Ad the bot build issue, I could build the kernel both with
> CONFIG_CHECKPOINT_RESTORE and without CONFIG_CHECKPOINT_RESTORE just
> fine after applying the two proposed patches.
> 
> What is the current state? Perhaps, this change should be CCed to
> linux-api@vger.kernel.org(?).
> 
> [1] https://lore.kernel.org/lkml/1523730291-109696-1-git-send-email-yang.shi@linux.alibaba.com/T/

Hi! I've a bit vague memory what we've ended up with, but iirc there was
a problem with brk() syscall or similar. Then I think we left everything
as is. I think there is no much activity in this prctl area now as far
as i know (replying to what is current state).

Re: [PATCH] prctl: Don't compile some of prctl functions when CRUI

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 365 bytes --]

On Wed, Apr 17, 2019 at 03:38:41PM +0300, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> I've a bit vague memory what we've ended up with, but iirc there was
> a problem with brk() syscall or similar. Then I think we left everything
> as is.
Was this related to the removal of non PR_SET_MM_MAP operations too?
Do you have any pointers to the topic?

Thanks,
Michal

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] prctl: Don't compile some of prctl functions when CRUI

[Cyrill Gorcunov]

On Wed, Apr 17, 2019 at 04:44:54PM +0200, Michal Koutný wrote:
> On Wed, Apr 17, 2019 at 03:38:41PM +0300, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> > I've a bit vague memory what we've ended up with, but iirc there was
> > a problem with brk() syscall or similar. Then I think we left everything
> > as is.
>
> Was this related to the removal of non PR_SET_MM_MAP operations too?
> Do you have any pointers to the topic?

Gimme some time, will reply later.

Re: [PATCH] prctl: Don't compile some of prctl functions when CRUI

[Cyrill Gorcunov]

On Wed, Apr 17, 2019 at 04:44:54PM +0200, Michal Koutný wrote:
> On Wed, Apr 17, 2019 at 03:38:41PM +0300, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> > I've a bit vague memory what we've ended up with, but iirc there was
> > a problem with brk() syscall or similar. Then I think we left everything
> > as is.
> Was this related to the removal of non PR_SET_MM_MAP operations too?
> Do you have any pointers to the topic?

Here some details: suprisingly there are already users of PR_SET_MM_x
operands, so we can't deprecate it (https://lkml.org/lkml/2018/5/6/127).
And then there was an attempt to change locks https://lore.kernel.org/patchwork/patch/
but since we're modifying mm-> entries we should gurd them against
simultaneous sys_brk call. Or I misunderstood you and you meant somthing
different?