ACK: [SRU][CVE-2020-25285][BIONIC][PATCH 1/1] mm/hugetlb: fix a race between hugetlb sysctl handlers

[Stefan Bader]

On 28.09.20 21:08, William Breathitt Gray wrote:
> From: Muchun Song <songmuchun at bytedance.com>
> 
> There is a race between the assignment of `table->data` and write value
> to the pointer of `table->data` in the __do_proc_doulongvec_minmax() on
> the other thread.
> 
>   CPU0:                                 CPU1:
>                                         proc_sys_write
>   hugetlb_sysctl_handler                  proc_sys_call_handler
>   hugetlb_sysctl_handler_common             hugetlb_sysctl_handler
>     table->data = &tmp;                       hugetlb_sysctl_handler_common
>                                                 table->data = &tmp;
>       proc_doulongvec_minmax
>         do_proc_doulongvec_minmax           sysctl_head_finish
>           __do_proc_doulongvec_minmax         unuse_table
>             i = table->data;
>             *i = val;  // corrupt CPU1's stack
> 
> Fix this by duplicating the `table`, and only update the duplicate of
> it.  And introduce a helper of proc_hugetlb_doulongvec_minmax() to
> simplify the code.
> 
> The following oops was seen:
> 
>     BUG: kernel NULL pointer dereference, address: 0000000000000000
>     #PF: supervisor instruction fetch in kernel mode
>     #PF: error_code(0x0010) - not-present page
>     Code: Bad RIP value.
>     ...
>     Call Trace:
>      ? set_max_huge_pages+0x3da/0x4f0
>      ? alloc_pool_huge_page+0x150/0x150
>      ? proc_doulongvec_minmax+0x46/0x60
>      ? hugetlb_sysctl_handler_common+0x1c7/0x200
>      ? nr_hugepages_store+0x20/0x20
>      ? copy_fd_bitmaps+0x170/0x170
>      ? hugetlb_sysctl_handler+0x1e/0x20
>      ? proc_sys_call_handler+0x2f1/0x300
>      ? unregister_sysctl_table+0xb0/0xb0
>      ? __fd_install+0x78/0x100
>      ? proc_sys_write+0x14/0x20
>      ? __vfs_write+0x4d/0x90
>      ? vfs_write+0xef/0x240
>      ? ksys_write+0xc0/0x160
>      ? __ia32_sys_read+0x50/0x50
>      ? __close_fd+0x129/0x150
>      ? __x64_sys_write+0x43/0x50
>      ? do_syscall_64+0x6c/0x200
>      ? entry_SYSCALL_64_after_hwframe+0x44/0xa9
> 
> Fixes: e5ff215941d5 ("hugetlb: multiple hstates for multiple page sizes")
> Signed-off-by: Muchun Song <songmuchun at bytedance.com>
> Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
> Reviewed-by: Mike Kravetz <mike.kravetz at oracle.com>
> Cc: Andi Kleen <ak at linux.intel.com>
> Link: http://lkml.kernel.org/r/20200828031146.43035-1-songmuchun@bytedance.com
> Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
> 
> CVE-2020-25285
> 
> (backported from commit 17743798d81238ab13050e8e2833699b54e15467)
> [ vilhelmgray: context adjustments ]
> Signed-off-by: William Breathitt Gray <william.gray at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---
>  mm/hugetlb.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index a6787cafb181..8a0d79eac03e 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2911,6 +2911,22 @@ static unsigned int cpuset_mems_nr(unsigned int *array)
>  }
>  
>  #ifdef CONFIG_SYSCTL
> +static int proc_hugetlb_doulongvec_minmax(struct ctl_table *table, int write,
> +					  void *buffer, size_t *length,
> +					  loff_t *ppos, unsigned long *out)
> +{
> +	struct ctl_table dup_table;
> +
> +	/*
> +	 * In order to avoid races with __do_proc_doulongvec_minmax(), we
> +	 * can duplicate the @table and alter the duplicate of it.
> +	 */
> +	dup_table = *table;
> +	dup_table.data = out;
> +
> +	return proc_doulongvec_minmax(&dup_table, write, buffer, length, ppos);
> +}
> +
>  static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
>  			 struct ctl_table *table, int write,
>  			 void __user *buffer, size_t *length, loff_t *ppos)
> @@ -2922,9 +2938,8 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
>  	if (!hugepages_supported())
>  		return -EOPNOTSUPP;
>  
> -	table->data = &tmp;
> -	table->maxlen = sizeof(unsigned long);
> -	ret = proc_doulongvec_minmax(table, write, buffer, length, ppos);
> +	ret = proc_hugetlb_doulongvec_minmax(table, write, buffer, length, ppos,
> +					     &tmp);
>  	if (ret)
>  		goto out;
>  
> @@ -2968,9 +2983,8 @@ int hugetlb_overcommit_handler(struct ctl_table *table, int write,
>  	if (write && hstate_is_gigantic(h))
>  		return -EINVAL;
>  
> -	table->data = &tmp;
> -	table->maxlen = sizeof(unsigned long);
> -	ret = proc_doulongvec_minmax(table, write, buffer, length, ppos);
> +	ret = proc_hugetlb_doulongvec_minmax(table, write, buffer, length, ppos,
> +					     &tmp);
>  	if (ret)
>  		goto out;
>  
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20200929/201f926b/attachment-0001.sig>