Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 15 Mar 2021 13:29:02 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Alexey Dobriyan <adobriyan@gmail.com>
Cc:     akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
        gorcunov@openvz.org, security@kernel.org
Subject: Re: [PATCH] prctl: fix PR_SET_MM_AUXV kernel stack leak
Message-ID: <20210315102901.GP21246@kadam>
References: <YE53QiJuxsCUeoiu@localhost.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YE53QiJuxsCUeoiu@localhost.localdomain>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

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?

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