On 08/31/2016 12:52 AM, David Rientjes wrote:
> On Fri, 26 Aug 2016, Vegard Nossum wrote:
>
[...]
>>
>> If nobody beats me to it, I can submit a new patch. The reason I haven't
>> submitted it just yet is because I want a chance to test that I get it
>> right and that the bug goes away. In the meantime this patch does fix a
>> bug that I ran into frequently. I don't think the patch above is total
>> crap.
>>
>
> What we want to do is stash away current->mempolicy, do
> current->mempolicy = NULL so functions like alloc_pages() don't look at
> it, and then do mpol_put() on it. task_unlock() acts a memory barrier in
> this case, lib/stackdepot.c needs no change.
>
> I think we need this backported to stable for 4.5+.
>
4.6+, as this fixes: cd11016e5f52 ("mm, kasan: stackdepot implementation.
Enable stackdepot for SLAB")
$ git describe --contains cd11016e5f5212c
v4.6-rc1~6^2~4
> Whaddyathink?
>
Acked-by: Andrey Ryabinin <[email protected]>
> --->o---
>
> mm, mempolicy: task->mempolicy must be NULL before dropping final reference
>
> KASAN allocates memory from the page allocator as part of
> kmem_cache_free(), and that can reference current->mempolicy through any
> number of allocation functions. It needs to be NULL'd out before the
> final reference is dropped to prevent a use-after-free bug:
>
> BUG: KASAN: use-after-free in alloc_pages_current+0x363/0x370 at addr
> ffff88010b48102c
> CPU: 0 PID: 15425 Comm: trinity-c2 Not tainted 4.8.0-rc2+ #140
> ...
> Call Trace:
> dump_stack
> kasan_object_err
> kasan_report_error
> __asan_report_load2_noabort
> alloc_pages_current <-- use after free
> depot_save_stack
> save_stack
> kasan_slab_free
> kmem_cache_free
> __mpol_put <-- free
> do_exit
>
> This patch sets current->mempolicy to NULL before dropping the final
> reference.
>
> Reported-by: Vegard Nossum <[email protected]>
> Signed-off-by: David Rientjes <[email protected]>
> ---
> include/linux/mempolicy.h | 4 ++++
> kernel/exit.c | 7 +------
> mm/mempolicy.c | 17 +++++++++++++++++
> 3 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/mempolicy.h b/include/linux/mempolicy.h
> --- a/include/linux/mempolicy.h
> +++ b/include/linux/mempolicy.h
> @@ -195,6 +195,7 @@ static inline bool vma_migratable(struct vm_area_struct
> *vma)
> }
>
> extern int mpol_misplaced(struct page *, struct vm_area_struct *, unsigned
> long);
> +extern void mpol_put_task_policy(struct task_struct *);
>
> #else
>
> @@ -297,5 +298,8 @@ static inline int mpol_misplaced(struct page *page,
> struct vm_area_struct *vma,
> return -1; /* no node preference */
> }
>
> +static inline void mpol_put_task_policy(struct task_struct *task)
> +{
> +}
> #endif /* CONFIG_NUMA */
> #endif
> diff --git a/kernel/exit.c b/kernel/exit.c
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -848,12 +848,7 @@ void do_exit(long code)
> TASKS_RCU(preempt_enable());
> exit_notify(tsk, group_dead);
> proc_exit_connector(tsk);
> -#ifdef CONFIG_NUMA
> - task_lock(tsk);
> - mpol_put(tsk->mempolicy);
> - tsk->mempolicy = NULL;
> - task_unlock(tsk);
> -#endif
> + mpol_put_task_policy(tsk);
> #ifdef CONFIG_FUTEX
> if (unlikely(current->pi_state_cache))
> kfree(current->pi_state_cache);
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2336,6 +2336,23 @@ out:
> return ret;
> }
>
> +/*
> + * Drop the (possibly final) reference to task->mempolicy. It needs to be
> + * dropped after task->mempolicy is set to NULL so that any allocation done
> as
> + * part of its kmem_cache_free(), such as by KASAN, doesn't reference a freed
> + * policy.
> + */
> +void mpol_put_task_policy(struct task_struct *task)
> +{
> + struct mempolicy *pol;
> +
> + task_lock(task);
> + pol = task->mempolicy;
> + task->mempolicy = NULL;
> + task_unlock(task);
> + mpol_put(pol);
> +}
> +
> static void sp_delete(struct shared_policy *sp, struct sp_node *n)
> {
> pr_debug("deleting %lx-l%lx\n", n->start, n->end);
>
