[PATCH] prctl: fix overwrite last but one entry in auxv vector

[PATCH] prctl: fix overwrite last but one entry in auxv vector

[Cyrill Gorcunov]

Alexey reported that current PR_SET_MM_AUXV (and PR_SET_MM_MAP) overwrite
too many entries on non 64bit kernels. This is because auxv is defined
as an array of longs and in result access to AT_VECTOR_SIZE - 2 entry
is not a type of auxv entry but rather an entry before the last one.

Since it is a common code for all architectures lets use __BITS_PER_LONG
definition to determinate each type/value pair in auxv_t is fitting
into `long` or not.

Note that on compat mode (ie Elf32 running in 64bit compiled kernel)
the preallocated vector size will be big enough to carry all entries
and zapping two entries at the end of the vector won't cause problems.

Same time lets drop useless task_lock()/task_unlock() calls from
PR_SET_MM_AUXV. It doesn't protect anything here and seems to be
sneaked in accidentally (Oleg pointed me to this moment).

Reported-by: Alexey Dobriyan <adobriyan@gmail.com>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Andrey Vagin <avagin@gmail.com>
CC: Dmitry Safonov <0x7f454c46@gmail.com>
CC: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
Take a look please, once time permit. The issue on its own
should not be that critical but better to fix it. I tested
it manually via trivial test but I think it is not enough.
Need to implement some selftesting as well. Also obviously
I ran test on x86 only.

 kernel/sys.c |   73 +++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 41 insertions(+), 32 deletions(-)

Index: linux-tip.git/kernel/sys.c
===================================================================
--- linux-tip.git.orig/kernel/sys.c
+++ linux-tip.git/kernel/sys.c
@@ -1961,6 +1961,33 @@ out:
 	return error;
 }
 
+static int copy_auxv_from_user(unsigned long *auxv, size_t auxv_size,
+			       const void __user *addr, size_t len)
+{
+	BUG_ON(auxv_size != sizeof(current->mm->saved_auxv));
+
+	if (!addr || len > auxv_size)
+		return -EINVAL;
+
+	memset(auxv, 0, auxv_size);
+	if (len && copy_from_user(auxv, addr, len))
+		return -EFAULT;
+
+	/*
+	 * Specification requires the vector to be
+	 * ended up with AT_NULL entry thus userspace
+	 * will notice where to stop enumerating. Thus
+	 * if someone is passing a screwed data make sure
+	 * at least it has the end of vector sign.
+	 */
+	if (len == auxv_size) {
+		if (__BITS_PER_LONG == 64)
+			auxv[AT_VECTOR_SIZE - 2] = AT_NULL;
+		auxv[AT_VECTOR_SIZE - 1] = AT_NULL;
+	}
+	return 0;
+}
+
 #ifdef CONFIG_CHECKPOINT_RESTORE
 static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data_size)
 {
@@ -1987,22 +2014,12 @@ static int prctl_set_mm_map(int opt, con
 		return error;
 
 	if (prctl_map.auxv_size) {
-		/*
-		 * Someone is trying to cheat the auxv vector.
-		 */
-		if (!prctl_map.auxv ||
-				prctl_map.auxv_size > sizeof(mm->saved_auxv))
-			return -EINVAL;
-
-		memset(user_auxv, 0, sizeof(user_auxv));
-		if (copy_from_user(user_auxv,
-				   (const void __user *)prctl_map.auxv,
-				   prctl_map.auxv_size))
-			return -EFAULT;
-
-		/* Last entry must be AT_NULL as specification requires */
-		user_auxv[AT_VECTOR_SIZE - 2] = AT_NULL;
-		user_auxv[AT_VECTOR_SIZE - 1] = AT_NULL;
+		int error = copy_auxv_from_user(user_auxv,
+						sizeof(user_auxv),
+						prctl_map.auxv,
+						prctl_map.auxv_size);
+		if (error)
+			return error;
 	}
 
 	if (prctl_map.exe_fd != (u32)-1) {
@@ -2079,25 +2096,17 @@ static int prctl_set_auxv(struct mm_stru
 	 * 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;
+	unsigned long user_auxv[AT_VECTOR_SIZE];
+	int error;
 
 	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;
+	error = copy_auxv_from_user(user_auxv, sizeof(user_auxv),
+				    (const void __user *)addr,
+				    len);
+	if (!error)
+		memcpy(mm->saved_auxv, user_auxv, len);
+	return error;
 }
 
 static int prctl_set_mm(int opt, unsigned long addr,

Re: [PATCH] prctl: fix overwrite last but one entry in auxv vector

[Cyrill Gorcunov]

On Sun, Mar 21, 2021 at 11:36:42PM +0300, Cyrill Gorcunov wrote:
> Alexey reported that current PR_SET_MM_AUXV (and PR_SET_MM_MAP) overwrite
> too many entries on non 64bit kernels. This is because auxv is defined
> as an array of longs and in result access to AT_VECTOR_SIZE - 2 entry
> is not a type of auxv entry but rather an entry before the last one.

Drop this patch, please. I'll make a new version.

[PATCH v2] prctl: PR_SET_MM - unify copying of user's auvx

[Cyrill Gorcunov]

prctl(PR_SET_MM, PR_SET_MM_AUXV | PR_SET_MM_MAP, ...) copies user
provided auxiliary vector to kernel and saves it to mm::saved_auxv,
this involves same code in to places. Lets move it into one helper
instead.

When we copy data from user space we make sure that the vector ends
up with AT_NULL key/value pair as specification requires. And here
is a bit vague moment if task is running in compat mode: instead of
one last value we zeroing two entries at the end. This is done for
code simplicity (if arch supports compat mode then the initial vector
size must be big enough to store values needed for the native mode
as well, that's why we define the vector as an array of longs. In
particular when Elf executable is loaded the vector is considered
as pairs of elf_addr_t elements, which is 4 byte per each on 32
bit environment and 8 byte per each in 64 bit kernel).

Same time lets drop useless task_lock()/task_unlock() calls from
PR_SET_MM_AUXV. It doesn't protect anything here and seems to be
sneaked in accidentally (Oleg pointed me this moment).

Reported-by: Alexey Dobriyan <adobriyan@gmail.com>
Reported-by: Oleg Nesterov <oleg@redhat.com>
CC: Andrey Vagin <avagin@gmail.com>
CC: Dmitry Safonov <0x7f454c46@gmail.com>
CC: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 kernel/sys.c |   70 ++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 38 insertions(+), 32 deletions(-)

Index: linux-tip.git/kernel/sys.c
===================================================================
--- linux-tip.git.orig/kernel/sys.c
+++ linux-tip.git/kernel/sys.c
@@ -1961,6 +1961,30 @@ out:
 	return error;
 }
 
+static int copy_auxv_from_user(unsigned long *auxv, size_t auxv_size,
+			       const void __user *addr, size_t len)
+{
+	BUG_ON(auxv_size != sizeof(current->mm->saved_auxv));
+
+	if (!addr || len > auxv_size)
+		return -EINVAL;
+
+	memset(auxv, 0, auxv_size);
+	if (len && copy_from_user(auxv, addr, len))
+		return -EFAULT;
+
+	/*
+	 * Specification requires the vector to be
+	 * ended up with AT_NULL entry so user space
+	 * will notice where to stop enumerating.
+	 */
+	if (len == auxv_size) {
+		auxv[AT_VECTOR_SIZE - 2] = AT_NULL;
+		auxv[AT_VECTOR_SIZE - 1] = AT_NULL;
+	}
+	return 0;
+}
+
 #ifdef CONFIG_CHECKPOINT_RESTORE
 static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data_size)
 {
@@ -1987,22 +2011,12 @@ static int prctl_set_mm_map(int opt, con
 		return error;
 
 	if (prctl_map.auxv_size) {
-		/*
-		 * Someone is trying to cheat the auxv vector.
-		 */
-		if (!prctl_map.auxv ||
-				prctl_map.auxv_size > sizeof(mm->saved_auxv))
-			return -EINVAL;
-
-		memset(user_auxv, 0, sizeof(user_auxv));
-		if (copy_from_user(user_auxv,
-				   (const void __user *)prctl_map.auxv,
-				   prctl_map.auxv_size))
-			return -EFAULT;
-
-		/* Last entry must be AT_NULL as specification requires */
-		user_auxv[AT_VECTOR_SIZE - 2] = AT_NULL;
-		user_auxv[AT_VECTOR_SIZE - 1] = AT_NULL;
+		int error = copy_auxv_from_user(user_auxv,
+						sizeof(user_auxv),
+						prctl_map.auxv,
+						prctl_map.auxv_size);
+		if (error)
+			return error;
 	}
 
 	if (prctl_map.exe_fd != (u32)-1) {
@@ -2079,25 +2093,17 @@ static int prctl_set_auxv(struct mm_stru
 	 * 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;
+	unsigned long user_auxv[AT_VECTOR_SIZE];
+	int error;
 
 	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;
+	error = copy_auxv_from_user(user_auxv, sizeof(user_auxv),
+				    (const void __user *)addr,
+				    len);
+	if (!error)
+		memcpy(mm->saved_auxv, user_auxv, len);
+	return error;
 }
 
 static int prctl_set_mm(int opt, unsigned long addr,

Re: [PATCH v2] prctl: PR_SET_MM - unify copying of user's auvx

[Dmitry Safonov]

Hi Cyrill,

On 3/23/21 10:06 PM, Cyrill Gorcunov wrote:
[..]
> --- linux-tip.git.orig/kernel/sys.c
> +++ linux-tip.git/kernel/sys.c
> @@ -1961,6 +1961,30 @@ out:
>  	return error;
>  }
>  
> +static int copy_auxv_from_user(unsigned long *auxv, size_t auxv_size,
> +			       const void __user *addr, size_t len)
> +{
> +	BUG_ON(auxv_size != sizeof(current->mm->saved_auxv));

Nit:
size_t auxv_size = sizeof(user_auxv);
BUILD_BUG_ON(sizeof(user_auxv) != sizeof(current->mm->saved_auxv));

(to make it local variable instead of a parameter and get rid of a new
BUG_ON())

> +
> +	if (!addr || len > auxv_size)
> +		return -EINVAL;
> +
> +	memset(auxv, 0, auxv_size);
> +	if (len && copy_from_user(auxv, addr, len))
> +		return -EFAULT;
> +
> +	/*
> +	 * Specification requires the vector to be
> +	 * ended up with AT_NULL entry so user space
> +	 * will notice where to stop enumerating.
> +	 */
> +	if (len == auxv_size) {
> +		auxv[AT_VECTOR_SIZE - 2] = AT_NULL;
> +		auxv[AT_VECTOR_SIZE - 1] = AT_NULL;

I don't follow why it became conditional.
Perhaps, you meant that memset(0) above will zerofy it anyway, but in
case (len == auxv_size - 1) it won't work. Or I'm missing something
obvious :-)

Thanks,
           Dima

Re: [PATCH v2] prctl: PR_SET_MM - unify copying of user's auvx

[kernel test robot]

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

Hi Cyrill,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on hnaz-mm/master linus/master v5.15-rc3 next-20210922]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Cyrill-Gorcunov/prctl-PR_SET_MM-unify-copying-of-user-s-auvx/20210929-123259
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5816b3e6577eaa676ceb00a848f0fd65fe2adc29
config: parisc-randconfig-s032-20210929 (attached as .config)
compiler: hppa-linux-gcc (GCC) 11.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/37297835c68662e1781118a01b7a271277e965d0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cyrill-Gorcunov/prctl-PR_SET_MM-unify-copying-of-user-s-auvx/20210929-123259
        git checkout 37297835c68662e1781118a01b7a271277e965d0
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> kernel/sys.c:1997:58: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected void const [noderef] __user *addr @@     got unsigned long long [usertype] *[addressable] auxv @@
   kernel/sys.c:1997:58: sparse:     expected void const [noderef] __user *addr
   kernel/sys.c:1997:58: sparse:     got unsigned long long [usertype] *[addressable] auxv
   kernel/sys.c:1068:32: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct task_struct *p1 @@     got struct task_struct [noderef] __rcu *real_parent @@
   kernel/sys.c:1068:32: sparse:     expected struct task_struct *p1
   kernel/sys.c:1068:32: sparse:     got struct task_struct [noderef] __rcu *real_parent
   kernel/sys.c: note: in included file (through include/linux/rcuwait.h, include/linux/percpu-rwsem.h, include/linux/fs.h, ...):
   include/linux/sched/signal.h:710:37: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   include/linux/sched/signal.h:710:37: sparse:     expected struct spinlock [usertype] *lock
   include/linux/sched/signal.h:710:37: sparse:     got struct spinlock [noderef] __rcu *

vim +1997 kernel/sys.c

  1968	
  1969	#ifdef CONFIG_CHECKPOINT_RESTORE
  1970	static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data_size)
  1971	{
  1972		struct prctl_mm_map prctl_map = { .exe_fd = (u32)-1, };
  1973		unsigned long user_auxv[AT_VECTOR_SIZE];
  1974		struct mm_struct *mm = current->mm;
  1975		int error;
  1976	
  1977		BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
  1978		BUILD_BUG_ON(sizeof(struct prctl_mm_map) > 256);
  1979	
  1980		if (opt == PR_SET_MM_MAP_SIZE)
  1981			return put_user((unsigned int)sizeof(prctl_map),
  1982					(unsigned int __user *)addr);
  1983	
  1984		if (data_size != sizeof(prctl_map))
  1985			return -EINVAL;
  1986	
  1987		if (copy_from_user(&prctl_map, addr, sizeof(prctl_map)))
  1988			return -EFAULT;
  1989	
  1990		error = validate_prctl_map_addr(&prctl_map);
  1991		if (error)
  1992			return error;
  1993	
  1994		if (prctl_map.auxv_size) {
  1995			int error = copy_auxv_from_user(user_auxv,
  1996							sizeof(user_auxv),
> 1997							prctl_map.auxv,
  1998							prctl_map.auxv_size);
  1999			if (error)
  2000				return error;
  2001		}
  2002	
  2003		if (prctl_map.exe_fd != (u32)-1) {
  2004			/*
  2005			 * Check if the current user is checkpoint/restore capable.
  2006			 * At the time of this writing, it checks for CAP_SYS_ADMIN
  2007			 * or CAP_CHECKPOINT_RESTORE.
  2008			 * Note that a user with access to ptrace can masquerade an
  2009			 * arbitrary program as any executable, even setuid ones.
  2010			 * This may have implications in the tomoyo subsystem.
  2011			 */
  2012			if (!checkpoint_restore_ns_capable(current_user_ns()))
  2013				return -EPERM;
  2014	
  2015			error = prctl_set_mm_exe_file(mm, prctl_map.exe_fd);
  2016			if (error)
  2017				return error;
  2018		}
  2019	
  2020		/*
  2021		 * arg_lock protects concurrent updates but we still need mmap_lock for
  2022		 * read to exclude races with sys_brk.
  2023		 */
  2024		mmap_read_lock(mm);
  2025	
  2026		/*
  2027		 * We don't validate if these members are pointing to
  2028		 * real present VMAs because application may have correspond
  2029		 * VMAs already unmapped and kernel uses these members for statistics
  2030		 * output in procfs mostly, except
  2031		 *
  2032		 *  - @start_brk/@brk which are used in do_brk_flags but kernel lookups
  2033		 *    for VMAs when updating these members so anything wrong written
  2034		 *    here cause kernel to swear at userspace program but won't lead
  2035		 *    to any problem in kernel itself
  2036		 */
  2037	
  2038		spin_lock(&mm->arg_lock);
  2039		mm->start_code	= prctl_map.start_code;
  2040		mm->end_code	= prctl_map.end_code;
  2041		mm->start_data	= prctl_map.start_data;
  2042		mm->end_data	= prctl_map.end_data;
  2043		mm->start_brk	= prctl_map.start_brk;
  2044		mm->brk		= prctl_map.brk;
  2045		mm->start_stack	= prctl_map.start_stack;
  2046		mm->arg_start	= prctl_map.arg_start;
  2047		mm->arg_end	= prctl_map.arg_end;
  2048		mm->env_start	= prctl_map.env_start;
  2049		mm->env_end	= prctl_map.env_end;
  2050		spin_unlock(&mm->arg_lock);
  2051	
  2052		/*
  2053		 * Note this update of @saved_auxv is lockless thus
  2054		 * if someone reads this member in procfs while we're
  2055		 * updating -- it may get partly updated results. It's
  2056		 * known and acceptable trade off: we leave it as is to
  2057		 * not introduce additional locks here making the kernel
  2058		 * more complex.
  2059		 */
  2060		if (prctl_map.auxv_size)
  2061			memcpy(mm->saved_auxv, user_auxv, sizeof(user_auxv));
  2062	
  2063		mmap_read_unlock(mm);
  2064		return 0;
  2065	}
  2066	#endif /* CONFIG_CHECKPOINT_RESTORE */
  2067	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

Re: [PATCH v2] prctl: PR_SET_MM - unify copying of user's auvx

[kernel test robot]

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

Hi Cyrill,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on hnaz-mm/master linus/master v5.15-rc3 next-20210922]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Cyrill-Gorcunov/prctl-PR_SET_MM-unify-copying-of-user-s-auvx/20210929-123259
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5816b3e6577eaa676ceb00a848f0fd65fe2adc29
config: parisc-randconfig-s032-20210929 (attached as .config)
compiler: hppa-linux-gcc (GCC) 11.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/37297835c68662e1781118a01b7a271277e965d0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cyrill-Gorcunov/prctl-PR_SET_MM-unify-copying-of-user-s-auvx/20210929-123259
        git checkout 37297835c68662e1781118a01b7a271277e965d0
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> kernel/sys.c:1997:58: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected void const [noderef] __user *addr @@     got unsigned long long [usertype] *[addressable] auxv @@
   kernel/sys.c:1997:58: sparse:     expected void const [noderef] __user *addr
   kernel/sys.c:1997:58: sparse:     got unsigned long long [usertype] *[addressable] auxv
   kernel/sys.c:1068:32: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct task_struct *p1 @@     got struct task_struct [noderef] __rcu *real_parent @@
   kernel/sys.c:1068:32: sparse:     expected struct task_struct *p1
   kernel/sys.c:1068:32: sparse:     got struct task_struct [noderef] __rcu *real_parent
   kernel/sys.c: note: in included file (through include/linux/rcuwait.h, include/linux/percpu-rwsem.h, include/linux/fs.h, ...):
   include/linux/sched/signal.h:710:37: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   include/linux/sched/signal.h:710:37: sparse:     expected struct spinlock [usertype] *lock
   include/linux/sched/signal.h:710:37: sparse:     got struct spinlock [noderef] __rcu *

vim +1997 kernel/sys.c

  1968	
  1969	#ifdef CONFIG_CHECKPOINT_RESTORE
  1970	static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data_size)
  1971	{
  1972		struct prctl_mm_map prctl_map = { .exe_fd = (u32)-1, };
  1973		unsigned long user_auxv[AT_VECTOR_SIZE];
  1974		struct mm_struct *mm = current->mm;
  1975		int error;
  1976	
  1977		BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
  1978		BUILD_BUG_ON(sizeof(struct prctl_mm_map) > 256);
  1979	
  1980		if (opt == PR_SET_MM_MAP_SIZE)
  1981			return put_user((unsigned int)sizeof(prctl_map),
  1982					(unsigned int __user *)addr);
  1983	
  1984		if (data_size != sizeof(prctl_map))
  1985			return -EINVAL;
  1986	
  1987		if (copy_from_user(&prctl_map, addr, sizeof(prctl_map)))
  1988			return -EFAULT;
  1989	
  1990		error = validate_prctl_map_addr(&prctl_map);
  1991		if (error)
  1992			return error;
  1993	
  1994		if (prctl_map.auxv_size) {
  1995			int error = copy_auxv_from_user(user_auxv,
  1996							sizeof(user_auxv),
> 1997							prctl_map.auxv,
  1998							prctl_map.auxv_size);
  1999			if (error)
  2000				return error;
  2001		}
  2002	
  2003		if (prctl_map.exe_fd != (u32)-1) {
  2004			/*
  2005			 * Check if the current user is checkpoint/restore capable.
  2006			 * At the time of this writing, it checks for CAP_SYS_ADMIN
  2007			 * or CAP_CHECKPOINT_RESTORE.
  2008			 * Note that a user with access to ptrace can masquerade an
  2009			 * arbitrary program as any executable, even setuid ones.
  2010			 * This may have implications in the tomoyo subsystem.
  2011			 */
  2012			if (!checkpoint_restore_ns_capable(current_user_ns()))
  2013				return -EPERM;
  2014	
  2015			error = prctl_set_mm_exe_file(mm, prctl_map.exe_fd);
  2016			if (error)
  2017				return error;
  2018		}
  2019	
  2020		/*
  2021		 * arg_lock protects concurrent updates but we still need mmap_lock for
  2022		 * read to exclude races with sys_brk.
  2023		 */
  2024		mmap_read_lock(mm);
  2025	
  2026		/*
  2027		 * We don't validate if these members are pointing to
  2028		 * real present VMAs because application may have correspond
  2029		 * VMAs already unmapped and kernel uses these members for statistics
  2030		 * output in procfs mostly, except
  2031		 *
  2032		 *  - @start_brk/@brk which are used in do_brk_flags but kernel lookups
  2033		 *    for VMAs when updating these members so anything wrong written
  2034		 *    here cause kernel to swear at userspace program but won't lead
  2035		 *    to any problem in kernel itself
  2036		 */
  2037	
  2038		spin_lock(&mm->arg_lock);
  2039		mm->start_code	= prctl_map.start_code;
  2040		mm->end_code	= prctl_map.end_code;
  2041		mm->start_data	= prctl_map.start_data;
  2042		mm->end_data	= prctl_map.end_data;
  2043		mm->start_brk	= prctl_map.start_brk;
  2044		mm->brk		= prctl_map.brk;
  2045		mm->start_stack	= prctl_map.start_stack;
  2046		mm->arg_start	= prctl_map.arg_start;
  2047		mm->arg_end	= prctl_map.arg_end;
  2048		mm->env_start	= prctl_map.env_start;
  2049		mm->env_end	= prctl_map.env_end;
  2050		spin_unlock(&mm->arg_lock);
  2051	
  2052		/*
  2053		 * Note this update of @saved_auxv is lockless thus
  2054		 * if someone reads this member in procfs while we're
  2055		 * updating -- it may get partly updated results. It's
  2056		 * known and acceptable trade off: we leave it as is to
  2057		 * not introduce additional locks here making the kernel
  2058		 * more complex.
  2059		 */
  2060		if (prctl_map.auxv_size)
  2061			memcpy(mm->saved_auxv, user_auxv, sizeof(user_auxv));
  2062	
  2063		mmap_read_unlock(mm);
  2064		return 0;
  2065	}
  2066	#endif /* CONFIG_CHECKPOINT_RESTORE */
  2067	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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

Re: [PATCH v2] prctl: PR_SET_MM - unify copying of user's auvx

[Cyrill Gorcunov]

On Wed, Sep 29, 2021 at 11:20:23PM +0800, kernel test robot wrote:
...
> sparse warnings: (new ones prefixed by >>)
> >> kernel/sys.c:1997:58: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected void const [noderef] __user *addr @@     got unsigned long long [usertype] *[addressable] auxv @@
>    kernel/sys.c:1997:58: sparse:     expected void const [noderef] __user *addr
>    kernel/sys.c:1997:58: sparse:     got unsigned long long [usertype] *[addressable] auxv
>    kernel/sys.c:1068:32: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct task_struct *p1 @@     got struct task_struct [noderef] __rcu *real_parent @@
>    kernel/sys.c:1068:32: sparse:     expected struct task_struct *p1
>    kernel/sys.c:1068:32: sparse:     got struct task_struct [noderef] __rcu *real_parent
>    kernel/sys.c: note: in included file (through include/linux/rcuwait.h, include/linux/percpu-rwsem.h, include/linux/fs.h, ...):
>    include/linux/sched/signal.h:710:37: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
>    include/linux/sched/signal.h:710:37: sparse:     expected struct spinlock [usertype] *lock
>    include/linux/sched/signal.h:710:37: sparse:     got struct spinlock [noderef] __rcu *
> 
> vim +1997 kernel/sys.c

Thanks for report! I happen to miss Dima's reply in first place as well :(
I'll take a look on this patch. The issie itself is implicit type conversion,
shouldn't be a bug in general but need to address as well. Will do.

Re: [PATCH v2] prctl: PR_SET_MM - unify copying of user's auvx

[Cyrill Gorcunov]

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

On Wed, Sep 29, 2021 at 11:20:23PM +0800, kernel test robot wrote:
...
> sparse warnings: (new ones prefixed by >>)
> >> kernel/sys.c:1997:58: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected void const [noderef] __user *addr @@     got unsigned long long [usertype] *[addressable] auxv @@
>    kernel/sys.c:1997:58: sparse:     expected void const [noderef] __user *addr
>    kernel/sys.c:1997:58: sparse:     got unsigned long long [usertype] *[addressable] auxv
>    kernel/sys.c:1068:32: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct task_struct *p1 @@     got struct task_struct [noderef] __rcu *real_parent @@
>    kernel/sys.c:1068:32: sparse:     expected struct task_struct *p1
>    kernel/sys.c:1068:32: sparse:     got struct task_struct [noderef] __rcu *real_parent
>    kernel/sys.c: note: in included file (through include/linux/rcuwait.h, include/linux/percpu-rwsem.h, include/linux/fs.h, ...):
>    include/linux/sched/signal.h:710:37: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
>    include/linux/sched/signal.h:710:37: sparse:     expected struct spinlock [usertype] *lock
>    include/linux/sched/signal.h:710:37: sparse:     got struct spinlock [noderef] __rcu *
> 
> vim +1997 kernel/sys.c

Thanks for report! I happen to miss Dima's reply in first place as well :(
I'll take a look on this patch. The issie itself is implicit type conversion,
shouldn't be a bug in general but need to address as well. Will do.

Re: [PATCH v2] prctl: PR_SET_MM - unify copying of user's auvx

[kernel test robot]

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

Hi Cyrill,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on v5.15]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Cyrill-Gorcunov/prctl-PR_SET_MM-unify-copying-of-user-s-auvx/20210929-123259
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5816b3e6577eaa676ceb00a848f0fd65fe2adc29
config: parisc-randconfig-s032-20210929 (attached as .config)
compiler: hppa-linux-gcc (GCC) 11.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/37297835c68662e1781118a01b7a271277e965d0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cyrill-Gorcunov/prctl-PR_SET_MM-unify-copying-of-user-s-auvx/20210929-123259
        git checkout 37297835c68662e1781118a01b7a271277e965d0
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> kernel/sys.c:1997:58: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected void const [noderef] __user *addr @@     got unsigned long long [usertype] *[addressable] auxv @@
   kernel/sys.c:1997:58: sparse:     expected void const [noderef] __user *addr
   kernel/sys.c:1997:58: sparse:     got unsigned long long [usertype] *[addressable] auxv
   kernel/sys.c:1068:32: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct task_struct *p1 @@     got struct task_struct [noderef] __rcu *real_parent @@
   kernel/sys.c:1068:32: sparse:     expected struct task_struct *p1
   kernel/sys.c:1068:32: sparse:     got struct task_struct [noderef] __rcu *real_parent
   kernel/sys.c: note: in included file (through include/linux/rcuwait.h, include/linux/percpu-rwsem.h, include/linux/fs.h, ...):
   include/linux/sched/signal.h:710:37: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   include/linux/sched/signal.h:710:37: sparse:     expected struct spinlock [usertype] *lock
   include/linux/sched/signal.h:710:37: sparse:     got struct spinlock [noderef] __rcu *

vim +1997 kernel/sys.c

  1968	
  1969	#ifdef CONFIG_CHECKPOINT_RESTORE
  1970	static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data_size)
  1971	{
  1972		struct prctl_mm_map prctl_map = { .exe_fd = (u32)-1, };
  1973		unsigned long user_auxv[AT_VECTOR_SIZE];
  1974		struct mm_struct *mm = current->mm;
  1975		int error;
  1976	
  1977		BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
  1978		BUILD_BUG_ON(sizeof(struct prctl_mm_map) > 256);
  1979	
  1980		if (opt == PR_SET_MM_MAP_SIZE)
  1981			return put_user((unsigned int)sizeof(prctl_map),
  1982					(unsigned int __user *)addr);
  1983	
  1984		if (data_size != sizeof(prctl_map))
  1985			return -EINVAL;
  1986	
  1987		if (copy_from_user(&prctl_map, addr, sizeof(prctl_map)))
  1988			return -EFAULT;
  1989	
  1990		error = validate_prctl_map_addr(&prctl_map);
  1991		if (error)
  1992			return error;
  1993	
  1994		if (prctl_map.auxv_size) {
  1995			int error = copy_auxv_from_user(user_auxv,
  1996							sizeof(user_auxv),
> 1997							prctl_map.auxv,
  1998							prctl_map.auxv_size);
  1999			if (error)
  2000				return error;
  2001		}
  2002	
  2003		if (prctl_map.exe_fd != (u32)-1) {
  2004			/*
  2005			 * Check if the current user is checkpoint/restore capable.
  2006			 * At the time of this writing, it checks for CAP_SYS_ADMIN
  2007			 * or CAP_CHECKPOINT_RESTORE.
  2008			 * Note that a user with access to ptrace can masquerade an
  2009			 * arbitrary program as any executable, even setuid ones.
  2010			 * This may have implications in the tomoyo subsystem.
  2011			 */
  2012			if (!checkpoint_restore_ns_capable(current_user_ns()))
  2013				return -EPERM;
  2014	
  2015			error = prctl_set_mm_exe_file(mm, prctl_map.exe_fd);
  2016			if (error)
  2017				return error;
  2018		}
  2019	
  2020		/*
  2021		 * arg_lock protects concurrent updates but we still need mmap_lock for
  2022		 * read to exclude races with sys_brk.
  2023		 */
  2024		mmap_read_lock(mm);
  2025	
  2026		/*
  2027		 * We don't validate if these members are pointing to
  2028		 * real present VMAs because application may have correspond
  2029		 * VMAs already unmapped and kernel uses these members for statistics
  2030		 * output in procfs mostly, except
  2031		 *
  2032		 *  - @start_brk/@brk which are used in do_brk_flags but kernel lookups
  2033		 *    for VMAs when updating these members so anything wrong written
  2034		 *    here cause kernel to swear at userspace program but won't lead
  2035		 *    to any problem in kernel itself
  2036		 */
  2037	
  2038		spin_lock(&mm->arg_lock);
  2039		mm->start_code	= prctl_map.start_code;
  2040		mm->end_code	= prctl_map.end_code;
  2041		mm->start_data	= prctl_map.start_data;
  2042		mm->end_data	= prctl_map.end_data;
  2043		mm->start_brk	= prctl_map.start_brk;
  2044		mm->brk		= prctl_map.brk;
  2045		mm->start_stack	= prctl_map.start_stack;
  2046		mm->arg_start	= prctl_map.arg_start;
  2047		mm->arg_end	= prctl_map.arg_end;
  2048		mm->env_start	= prctl_map.env_start;
  2049		mm->env_end	= prctl_map.env_end;
  2050		spin_unlock(&mm->arg_lock);
  2051	
  2052		/*
  2053		 * Note this update of @saved_auxv is lockless thus
  2054		 * if someone reads this member in procfs while we're
  2055		 * updating -- it may get partly updated results. It's
  2056		 * known and acceptable trade off: we leave it as is to
  2057		 * not introduce additional locks here making the kernel
  2058		 * more complex.
  2059		 */
  2060		if (prctl_map.auxv_size)
  2061			memcpy(mm->saved_auxv, user_auxv, sizeof(user_auxv));
  2062	
  2063		mmap_read_unlock(mm);
  2064		return 0;
  2065	}
  2066	#endif /* CONFIG_CHECKPOINT_RESTORE */
  2067	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

Re: [PATCH v2] prctl: PR_SET_MM - unify copying of user's auvx

[kernel test robot]

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

Hi Cyrill,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on v5.15]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Cyrill-Gorcunov/prctl-PR_SET_MM-unify-copying-of-user-s-auvx/20210929-123259
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5816b3e6577eaa676ceb00a848f0fd65fe2adc29
config: parisc-randconfig-s032-20210929 (attached as .config)
compiler: hppa-linux-gcc (GCC) 11.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/37297835c68662e1781118a01b7a271277e965d0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Cyrill-Gorcunov/prctl-PR_SET_MM-unify-copying-of-user-s-auvx/20210929-123259
        git checkout 37297835c68662e1781118a01b7a271277e965d0
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> kernel/sys.c:1997:58: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected void const [noderef] __user *addr @@     got unsigned long long [usertype] *[addressable] auxv @@
   kernel/sys.c:1997:58: sparse:     expected void const [noderef] __user *addr
   kernel/sys.c:1997:58: sparse:     got unsigned long long [usertype] *[addressable] auxv
   kernel/sys.c:1068:32: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct task_struct *p1 @@     got struct task_struct [noderef] __rcu *real_parent @@
   kernel/sys.c:1068:32: sparse:     expected struct task_struct *p1
   kernel/sys.c:1068:32: sparse:     got struct task_struct [noderef] __rcu *real_parent
   kernel/sys.c: note: in included file (through include/linux/rcuwait.h, include/linux/percpu-rwsem.h, include/linux/fs.h, ...):
   include/linux/sched/signal.h:710:37: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   include/linux/sched/signal.h:710:37: sparse:     expected struct spinlock [usertype] *lock
   include/linux/sched/signal.h:710:37: sparse:     got struct spinlock [noderef] __rcu *

vim +1997 kernel/sys.c

  1968	
  1969	#ifdef CONFIG_CHECKPOINT_RESTORE
  1970	static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data_size)
  1971	{
  1972		struct prctl_mm_map prctl_map = { .exe_fd = (u32)-1, };
  1973		unsigned long user_auxv[AT_VECTOR_SIZE];
  1974		struct mm_struct *mm = current->mm;
  1975		int error;
  1976	
  1977		BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
  1978		BUILD_BUG_ON(sizeof(struct prctl_mm_map) > 256);
  1979	
  1980		if (opt == PR_SET_MM_MAP_SIZE)
  1981			return put_user((unsigned int)sizeof(prctl_map),
  1982					(unsigned int __user *)addr);
  1983	
  1984		if (data_size != sizeof(prctl_map))
  1985			return -EINVAL;
  1986	
  1987		if (copy_from_user(&prctl_map, addr, sizeof(prctl_map)))
  1988			return -EFAULT;
  1989	
  1990		error = validate_prctl_map_addr(&prctl_map);
  1991		if (error)
  1992			return error;
  1993	
  1994		if (prctl_map.auxv_size) {
  1995			int error = copy_auxv_from_user(user_auxv,
  1996							sizeof(user_auxv),
> 1997							prctl_map.auxv,
  1998							prctl_map.auxv_size);
  1999			if (error)
  2000				return error;
  2001		}
  2002	
  2003		if (prctl_map.exe_fd != (u32)-1) {
  2004			/*
  2005			 * Check if the current user is checkpoint/restore capable.
  2006			 * At the time of this writing, it checks for CAP_SYS_ADMIN
  2007			 * or CAP_CHECKPOINT_RESTORE.
  2008			 * Note that a user with access to ptrace can masquerade an
  2009			 * arbitrary program as any executable, even setuid ones.
  2010			 * This may have implications in the tomoyo subsystem.
  2011			 */
  2012			if (!checkpoint_restore_ns_capable(current_user_ns()))
  2013				return -EPERM;
  2014	
  2015			error = prctl_set_mm_exe_file(mm, prctl_map.exe_fd);
  2016			if (error)
  2017				return error;
  2018		}
  2019	
  2020		/*
  2021		 * arg_lock protects concurrent updates but we still need mmap_lock for
  2022		 * read to exclude races with sys_brk.
  2023		 */
  2024		mmap_read_lock(mm);
  2025	
  2026		/*
  2027		 * We don't validate if these members are pointing to
  2028		 * real present VMAs because application may have correspond
  2029		 * VMAs already unmapped and kernel uses these members for statistics
  2030		 * output in procfs mostly, except
  2031		 *
  2032		 *  - @start_brk/@brk which are used in do_brk_flags but kernel lookups
  2033		 *    for VMAs when updating these members so anything wrong written
  2034		 *    here cause kernel to swear at userspace program but won't lead
  2035		 *    to any problem in kernel itself
  2036		 */
  2037	
  2038		spin_lock(&mm->arg_lock);
  2039		mm->start_code	= prctl_map.start_code;
  2040		mm->end_code	= prctl_map.end_code;
  2041		mm->start_data	= prctl_map.start_data;
  2042		mm->end_data	= prctl_map.end_data;
  2043		mm->start_brk	= prctl_map.start_brk;
  2044		mm->brk		= prctl_map.brk;
  2045		mm->start_stack	= prctl_map.start_stack;
  2046		mm->arg_start	= prctl_map.arg_start;
  2047		mm->arg_end	= prctl_map.arg_end;
  2048		mm->env_start	= prctl_map.env_start;
  2049		mm->env_end	= prctl_map.env_end;
  2050		spin_unlock(&mm->arg_lock);
  2051	
  2052		/*
  2053		 * Note this update of @saved_auxv is lockless thus
  2054		 * if someone reads this member in procfs while we're
  2055		 * updating -- it may get partly updated results. It's
  2056		 * known and acceptable trade off: we leave it as is to
  2057		 * not introduce additional locks here making the kernel
  2058		 * more complex.
  2059		 */
  2060		if (prctl_map.auxv_size)
  2061			memcpy(mm->saved_auxv, user_auxv, sizeof(user_auxv));
  2062	
  2063		mmap_read_unlock(mm);
  2064		return 0;
  2065	}
  2066	#endif /* CONFIG_CHECKPOINT_RESTORE */
  2067	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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