Re: [PATCH] prctl: fix PR_SET_MM_AUXV kernel stack leak

Alexey Dobriyan
2021-03-15T13:30:57Z

On Mon, Mar 15, 2021 at 01:29:02PM +0300, Dan Carpenter wrote:
> On Sun, Mar 14, 2021 at 11:51:14PM +0300, Alexey Dobriyan wrote:
> > prctl(PR_SET_MM, PR_SET_MM_AUXV, addr, 1);
> >
> > will copy 1 byte from userspace to (quite big) on-stack array
> > and then stash everything to mm->saved_auxv.
>
> What? It won't save everything, only the 1 byte. What am I not seeing?

It does copy 1 byte. How embarassing of me.

I was confused by another way of setting auxv data:

if (prctl_map.auxv_size)
memcpy(mm->saved_auxv, user_auxv, sizeof(user_auxv));

This does full array copy but the array is fully initialised so there is
no problem.

Stop the presses!

> kernel/sys.c
> 2073 static int prctl_set_auxv(struct mm_struct *mm, unsigned long addr,
> 2074 unsigned long len)
> 2075 {
> 2076 /*
> 2077 * This doesn't move the auxiliary vector itself since it's pinned to
> 2078 * mm_struct, but it permits filling the vector with new values. It's
> 2079 * up to the caller to provide sane values here, otherwise userspace
> 2080 * tools which use this vector might be unhappy.
> 2081 */
> 2082 unsigned long user_auxv[AT_VECTOR_SIZE] = {};
> 2083
> 2084 if (len > sizeof(user_auxv))
> 2085 return -EINVAL;
> 2086
> 2087 if (copy_from_user(user_auxv, (const void __user *)addr, len))
> ^^^^^^^^^ ^^^
> Copies one byte from user space.
>
> 2088 return -EFAULT;
> 2089
> 2090 /* Make sure the last entry is always AT_NULL */
> 2091 user_auxv[AT_VECTOR_SIZE - 2] = 0;
> 2092 user_auxv[AT_VECTOR_SIZE - 1] = 0;
> 2093
> 2094 BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
> 2095
> 2096 task_lock(current);
> 2097 memcpy(mm->saved_auxv, user_auxv, len);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Saves one byte to mm->saved_auxv.
>
> 2098 task_unlock(current);
> 2099
> 2100 return 0;
> 2101 }
>
> regards,
> dan carpenter
>