| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 24 May 2016 13:43:26 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Tejun Heo <tj@...nel.org>
Cc: Vlastimil Babka <vbabka@...e.cz>,
Sasha Levin <sasha.levin@...cle.com>,
Alexei Starovoitov <ast@...nel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
Christoph Lameter <cl@...ux.com>,
Linux-MM layout <linux-mm@...ck.org>,
Marco Grassi <marco.gra@...il.com>
Subject: Re: bpf: use-after-free in array_map_alloc
On Tue, May 24, 2016 at 12:04 PM, Tejun Heo <tj@...nel.org> wrote:
> Hello,
>
> Alexei, can you please verify this patch? Map extension got rolled
> into balance work so that there's no sync issues between the two async
> operations.
tests look good. No uaf and basic bpf tests exercise per-cpu map are fine.
>
> Thanks.
>
> Index: work/mm/percpu.c
> ===================================================================
> --- work.orig/mm/percpu.c
> +++ work/mm/percpu.c
> @@ -112,7 +112,7 @@ struct pcpu_chunk {
> int map_used; /* # of map entries used before the sentry */
> int map_alloc; /* # of map entries allocated */
> int *map; /* allocation map */
> - struct work_struct map_extend_work;/* async ->map[] extension */
> + struct list_head map_extend_list;/* on pcpu_map_extend_chunks */
>
> void *data; /* chunk data */
> int first_free; /* no free below this */
> @@ -162,10 +162,13 @@ static struct pcpu_chunk *pcpu_reserved_
> 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 ext */
>
> static struct list_head *pcpu_slot __read_mostly; /* chunk list slots */
>
> +/* chunks which need their map areas extended, protected by pcpu_lock */
> +static LIST_HEAD(pcpu_map_extend_chunks);
> +
> /*
> * The number of empty populated pages, protected by pcpu_lock. The
> * reserved chunk doesn't contribute to the count.
> @@ -395,13 +398,19 @@ static int pcpu_need_to_extend(struct pc
> {
> int margin, new_alloc;
>
> + lockdep_assert_held(&pcpu_lock);
> +
> if (is_atomic) {
> margin = 3;
>
> if (chunk->map_alloc <
> - chunk->map_used + PCPU_ATOMIC_MAP_MARGIN_LOW &&
> - pcpu_async_enabled)
> - schedule_work(&chunk->map_extend_work);
> + chunk->map_used + PCPU_ATOMIC_MAP_MARGIN_LOW) {
> + if (list_empty(&chunk->map_extend_list)) {
> + list_add_tail(&chunk->map_extend_list,
> + &pcpu_map_extend_chunks);
> + pcpu_schedule_balance_work();
> + }
> + }
> } else {
> margin = PCPU_ATOMIC_MAP_MARGIN_HIGH;
> }
> @@ -435,6 +444,8 @@ static int pcpu_extend_area_map(struct p
> 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;
> @@ -467,20 +478,6 @@ out_unlock:
> return 0;
> }
>
> -static void pcpu_map_extend_workfn(struct work_struct *work)
> -{
> - struct pcpu_chunk *chunk = container_of(work, struct pcpu_chunk,
> - map_extend_work);
> - int new_alloc;
> -
> - spin_lock_irq(&pcpu_lock);
> - new_alloc = pcpu_need_to_extend(chunk, false);
> - spin_unlock_irq(&pcpu_lock);
> -
> - if (new_alloc)
> - pcpu_extend_area_map(chunk, new_alloc);
> -}
> -
> /**
> * pcpu_fit_in_area - try to fit the requested allocation in a candidate area
> * @chunk: chunk the candidate area belongs to
> @@ -740,7 +737,7 @@ static struct pcpu_chunk *pcpu_alloc_chu
> chunk->map_used = 1;
>
> INIT_LIST_HEAD(&chunk->list);
> - INIT_WORK(&chunk->map_extend_work, pcpu_map_extend_workfn);
> + INIT_LIST_HEAD(&chunk->map_extend_list);
> chunk->free_size = pcpu_unit_size;
> chunk->contig_hint = pcpu_unit_size;
>
> @@ -895,6 +892,9 @@ static void __percpu *pcpu_alloc(size_t
> 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 +967,9 @@ restart:
> if (is_atomic)
> goto fail;
>
> - mutex_lock(&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 +980,6 @@ restart:
> spin_lock_irqsave(&pcpu_lock, flags);
> }
>
> - mutex_unlock(&pcpu_alloc_mutex);
> goto restart;
>
> area_found:
> @@ -993,8 +989,6 @@ area_found:
> 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 +999,6 @@ area_found:
>
> 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 +1038,8 @@ fail:
> /* 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;
> }
> @@ -1129,6 +1124,7 @@ static void pcpu_balance_workfn(struct w
> if (chunk == list_first_entry(free_head, struct pcpu_chunk, list))
> continue;
>
> + list_del_init(&chunk->map_extend_list);
> list_move(&chunk->list, &to_free);
> }
>
> @@ -1146,6 +1142,24 @@ static void pcpu_balance_workfn(struct w
> pcpu_destroy_chunk(chunk);
> }
>
> + do {
> + int new_alloc = 0;
> +
> + spin_lock_irq(&pcpu_lock);
> +
> + chunk = list_first_entry_or_null(&pcpu_map_extend_chunks,
> + struct pcpu_chunk, map_extend_list);
> + if (chunk) {
> + list_del_init(&chunk->map_extend_list);
> + new_alloc = pcpu_need_to_extend(chunk, false);
> + }
> +
> + spin_unlock_irq(&pcpu_lock);
> +
> + if (new_alloc)
> + pcpu_extend_area_map(chunk, new_alloc);
> + } while (chunk);
> +
> /*
> * Ensure there are certain number of free populated pages for
> * atomic allocs. Fill up from the most packed so that atomic
> @@ -1644,7 +1658,7 @@ int __init pcpu_setup_first_chunk(const
> */
> schunk = memblock_virt_alloc(pcpu_chunk_struct_size, 0);
> INIT_LIST_HEAD(&schunk->list);
> - INIT_WORK(&schunk->map_extend_work, pcpu_map_extend_workfn);
> + INIT_LIST_HEAD(&schunk->map_extend_list);
> schunk->base_addr = base_addr;
> schunk->map = smap;
> schunk->map_alloc = ARRAY_SIZE(smap);
> @@ -1673,7 +1687,7 @@ int __init pcpu_setup_first_chunk(const
> if (dyn_size) {
> dchunk = memblock_virt_alloc(pcpu_chunk_struct_size, 0);
> INIT_LIST_HEAD(&dchunk->list);
> - INIT_WORK(&dchunk->map_extend_work, pcpu_map_extend_workfn);
> + INIT_LIST_HEAD(&dchunk->map_extend_list);
> dchunk->base_addr = base_addr;
> dchunk->map = dmap;
> dchunk->map_alloc = ARRAY_SIZE(dmap);
Powered by blists - more mailing lists