On Mon, May 23, 2016 at 05:35:01PM -0400, Tejun Heo wrote:
> Hello,
>
> Can you please test whether this patch resolves the issue? While
> adding support for atomic allocations, I reduced alloc_mutex covered
> region too much.
after the patch the use-after-free is no longer seen.
Tested-by: Alexei Starovoitov <a...@kernel.org>
>
> Thanks.
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 0c59684..bd2df70 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -162,7 +162,7 @@ static struct pcpu_chunk *pcpu_reserved_chunk;
> static int pcpu_reserved_chunk_limit;
>
> static DEFINE_SPINLOCK(pcpu_lock); /* all internal data structures */
> -static DEFINE_MUTEX(pcpu_alloc_mutex); /* chunk create/destroy,
> [de]pop */
> +static DEFINE_MUTEX(pcpu_alloc_mutex); /* chunk create/destroy,
> [de]pop, map extension */
>
> static struct list_head *pcpu_slot __read_mostly; /* chunk list slots */
>
> @@ -435,6 +435,8 @@ static int pcpu_extend_area_map(struct pcpu_chunk *chunk,
> int new_alloc)
> size_t old_size = 0, new_size = new_alloc * sizeof(new[0]);
> unsigned long flags;
>
> + lockdep_assert_held(&pcpu_alloc_mutex);
> +
> new = pcpu_mem_zalloc(new_size);
> if (!new)
> return -ENOMEM;
> @@ -895,6 +897,9 @@ static void __percpu *pcpu_alloc(size_t size, size_t
> align, bool reserved,
> return NULL;
> }
>
> + if (!is_atomic)
> + mutex_lock(&pcpu_alloc_mutex);
> +
> spin_lock_irqsave(&pcpu_lock, flags);
>
> /* serve reserved allocations from the reserved chunk if available */
> @@ -967,12 +972,11 @@ static void __percpu *pcpu_alloc(size_t size, size_t
> align, bool reserved,
> if (is_atomic)
> goto fail;
>
> - mutex_lock(&pcpu_alloc_mutex);
> + lockdep_assert_held(&pcpu_alloc_mutex);
>
> if (list_empty(&pcpu_slot[pcpu_nr_slots - 1])) {
> chunk = pcpu_create_chunk();
> if (!chunk) {
> - mutex_unlock(&pcpu_alloc_mutex);
> err = "failed to allocate new chunk";
> goto fail;
> }
> @@ -983,7 +987,6 @@ static void __percpu *pcpu_alloc(size_t size, size_t
> align, bool reserved,
> spin_lock_irqsave(&pcpu_lock, flags);
> }
>
> - mutex_unlock(&pcpu_alloc_mutex);
> goto restart;
>
> area_found:
> @@ -993,8 +996,6 @@ static void __percpu *pcpu_alloc(size_t size, size_t
> align, bool reserved,
> if (!is_atomic) {
> int page_start, page_end, rs, re;
>
> - mutex_lock(&pcpu_alloc_mutex);
> -
> page_start = PFN_DOWN(off);
> page_end = PFN_UP(off + size);
>
> @@ -1005,7 +1006,6 @@ static void __percpu *pcpu_alloc(size_t size, size_t
> align, bool reserved,
>
> spin_lock_irqsave(&pcpu_lock, flags);
> if (ret) {
> - mutex_unlock(&pcpu_alloc_mutex);
> pcpu_free_area(chunk, off, &occ_pages);
> err = "failed to populate";
> goto fail_unlock;
> @@ -1045,6 +1045,8 @@ static void __percpu *pcpu_alloc(size_t size, size_t
> align, bool reserved,
> /* see the flag handling in pcpu_blance_workfn() */
> pcpu_atomic_alloc_failed = true;
> pcpu_schedule_balance_work();
> + } else {
> + mutex_unlock(&pcpu_alloc_mutex);
> }
> return NULL;
> }
> @@ -1137,6 +1139,8 @@ static void pcpu_balance_workfn(struct work_struct
> *work)
> list_for_each_entry_safe(chunk, next, &to_free, list) {
> int rs, re;
>
> + cancel_work_sync(&chunk->map_extend_work);
> +
> pcpu_for_each_pop_region(chunk, rs, re, 0, pcpu_unit_pages) {
> pcpu_depopulate_chunk(chunk, rs, re);
> spin_lock_irq(&pcpu_lock);
