| 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
| ||
|
Message-ID: <5E4674BB.4020900@huawei.com>
Date: Fri, 14 Feb 2020 18:21:47 +0800
From: Yang Yingliang <yangyingliang@...wei.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
<linux-kernel@...r.kernel.org>
CC: <stable@...r.kernel.org>, Herbert Xu <herbert@...dor.apana.org.au>,
"Daniel Jordan" <daniel.m.jordan@...cle.com>,
Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 4.19 091/195] padata: Remove broken queue flushing
Hi,
On 2020/2/10 20:32, Greg Kroah-Hartman wrote:
> From: Herbert Xu <herbert@...dor.apana.org.au>
>
> [ Upstream commit 07928d9bfc81640bab36f5190e8725894d93b659 ]
>
> The function padata_flush_queues is fundamentally broken because
> it cannot force padata users to complete the request that is
> underway. IOW padata has to passively wait for the completion
> of any outstanding work.
>
> As it stands flushing is used in two places. Its use in padata_stop
> is simply unnecessary because nothing depends on the queues to
> be flushed afterwards.
>
> The other use in padata_replace is more substantial as we depend
> on it to free the old pd structure. This patch instead uses the
> pd->refcnt to dynamically free the pd structure once all requests
> are complete.
>
> Fixes: 2b73b07ab8a4 ("padata: Flush the padata queues actively")
> Cc: <stable@...r.kernel.org>
> Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>
> Reviewed-by: Daniel Jordan <daniel.m.jordan@...cle.com>
> Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
> kernel/padata.c | 46 ++++++++++++----------------------------------
> 1 file changed, 12 insertions(+), 34 deletions(-)
>
> diff --git a/kernel/padata.c b/kernel/padata.c
> index 6c06b3039faed..11c5f9c8779ea 100644
> --- a/kernel/padata.c
> +++ b/kernel/padata.c
> @@ -35,6 +35,8 @@
>
> #define MAX_OBJ_NUM 1000
>
> +static void padata_free_pd(struct parallel_data *pd);
> +
> static int padata_index_to_cpu(struct parallel_data *pd, int cpu_index)
> {
> int cpu, target_cpu;
> @@ -334,6 +336,7 @@ static void padata_serial_worker(struct work_struct *serial_work)
> struct padata_serial_queue *squeue;
> struct parallel_data *pd;
> LIST_HEAD(local_list);
> + int cnt;
>
> local_bh_disable();
> squeue = container_of(serial_work, struct padata_serial_queue, work);
> @@ -343,6 +346,8 @@ static void padata_serial_worker(struct work_struct *serial_work)
> list_replace_init(&squeue->serial.list, &local_list);
> spin_unlock(&squeue->serial.lock);
>
> + cnt = 0;
> +
> while (!list_empty(&local_list)) {
> struct padata_priv *padata;
>
> @@ -352,9 +357,12 @@ static void padata_serial_worker(struct work_struct *serial_work)
> list_del_init(&padata->list);
>
> padata->serial(padata);
> - atomic_dec(&pd->refcnt);
> + cnt++;
> }
> local_bh_enable();
> +
> + if (atomic_sub_and_test(cnt, &pd->refcnt))
> + padata_free_pd(pd);
> }
>
> /**
> @@ -501,8 +509,7 @@ static struct parallel_data *padata_alloc_pd(struct padata_instance *pinst,
> timer_setup(&pd->timer, padata_reorder_timer, 0);
> atomic_set(&pd->seq_nr, -1);
> atomic_set(&pd->reorder_objects, 0);
> - atomic_set(&pd->refcnt, 0);
> - pd->pinst = pinst;
This patch remove this assignment, it's cause a null-ptr-deref when
using pd->pinst in padata_reorder().
[39135.886908] Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000010
[39135.886909] Mem abort info:
[39135.886910] ESR = 0x96000004
[39135.886912] Exception class = DABT (current EL), IL = 32 bits
[39135.886913] SET = 0, FnV = 0
[39135.886913] EA = 0, S1PTW = 0
[39135.886914] Data abort info:
[39135.886915] ISV = 0, ISS = 0x00000004
[39135.886915] CM = 0, WnR = 0
[39135.886918] user pgtable: 4k pages, 48-bit VAs, pgdp = 00000000c66d7ef5
[39135.886919] [0000000000000010] pgd=0000000000000000
[39135.886922] Internal error: Oops: 96000004 [#1] SMP
[39135.897190] Modules linked in: authenc pcrypt crypto_user cfg80211
rfkill ib_isert iscsi_target_mod ib_srpt target_core_mod dm_mirror
dm_region_hash rpcrdma dm_log ib_srp scsi_transport_srp sunrpc dm_mod
ib_ipoib rdma_ucm ib_uverbs ib_umad ib_iser rdma_cm ib_cm iw_cm
aes_ce_blk crypto_simd cryptd aes_ce_cipher hns_roce_hw_v2 ghash_ce
sha2_ce sha256_arm64 hns_roce ib_core sha1_ce sbsa_gwdt hi_sfc sg mtd
ipmi_ssif sch_fq_codel ip_tables realtek hclge hinic hns3 ipmi_si
hisi_sas_v3_hw hibmc_drm hnae3 hisi_sas_main ipmi_devintf ipmi_msghandler
[39135.997870] Process kworker/0:2 (pid: 789, stack limit =
0x0000000047f55ba6)
[39136.012707] CPU: 0 PID: 789 Comm: kworker/0:2 Kdump: loaded Not
tainted 4.19.103+ #1
[39136.029010] Hardware name: Huawei TaiShan 2280 V2/BC82AMDD, BIOS 1.08
12/14/2019
[39136.044587] Workqueue: pencrypt padata_parallel_worker
[39136.055396] pstate: 00c00009 (nzcv daif +PAN +UAO)
[39136.065479] pc : padata_reorder+0x144/0x2e0
[39136.074274] lr : padata_reorder+0x124/0x2e0
[39136.083070] sp : ffff0000149cbc90
[39136.090036] x29: ffff0000149cbc90 x28: 0000000000000001
[39136.101215] x27: ffffa02fd14af080 x26: ffffa02fd5fe1600
[39136.112392] x25: ffff5df7bf4c0ac8 x24: ffffa02fd5fe1628
[39136.123569] x23: 0000000000000000 x22: ffff00000828a258
[39136.134747] x21: ffffa02fd5fe1618 x20: ffff000009a79788
[39136.145924] x19: ffff5df7bf4c0ac8 x18: 00000000bef9a3f7
[39136.157102] x17: 0000000066bb7710 x16: 000000009a34db62
[39136.168280] x15: 0000000000342311 x14: 0000000037c9c538
[39136.179458] x13: 00000000deb82818 x12: 000000007abb6477
[39136.190638] x11: 000000006e0b05e5 x10: 00000000ccde2d6a
[39136.201817] x9 : 0000000000000000 x8 : a544a826aa446d6a
[39136.212996] x7 : e5050b6ea3a6345c x6 : ffffa02fd74ef030
[39136.224174] x5 : 0000000000000010 x4 : ffff5df7bf4c0ac8
[39136.235354] x3 : ffff5df7bf4c0ad8 x2 : ffff5df7bf4c0ac8
[39136.246532] x1 : ffff5df7bf4c0ad8 x0 : 0000000000000000
[39136.257712] Call trace:
[39136.262851] padata_reorder+0x144/0x2e0
[39136.270922] padata_do_serial+0xc8/0x128
[39136.279177] pcrypt_aead_enc+0x60/0x70 [pcrypt]
[39136.288708] padata_parallel_worker+0xd8/0x138
[39136.298056] process_one_work+0x1bc/0x4b8
[39136.306489] worker_thread+0x164/0x580
[39136.314374] kthread+0x134/0x138
[39136.321163] ret_from_fork+0x10/0x18
[39136.328681] Code: f900033b 52800000 91004261 089ffc20 (f9400ae1)
[39136.341508] ---[ end trace fc1b4f00385f0fee ]---
[39136.351221] Kernel panic - not syncing: Fatal exception in interrupt
[39136.364591] SMP: stopping secondary CPUs
[39136.372863] Kernel Offset: disabled
[39138.438722] CPU features: 0x12,a2200a38
[39138.446797] Memory Limit: none
[39138.463025] Starting crashdump kernel...
[39138.471295] Bye!
> + atomic_set(&pd->refcnt, 1);
> spin_lock_init(&pd->lock);
>
> return pd;
> @@ -526,31 +533,6 @@ static void padata_free_pd(struct parallel_data *pd)
> kfree(pd);
> }
>
> -/* Flush all objects out of the padata queues. */
> -static void padata_flush_queues(struct parallel_data *pd)
> -{
> - int cpu;
> - struct padata_parallel_queue *pqueue;
> - struct padata_serial_queue *squeue;
> -
> - for_each_cpu(cpu, pd->cpumask.pcpu) {
> - pqueue = per_cpu_ptr(pd->pqueue, cpu);
> - flush_work(&pqueue->work);
> - }
> -
> - del_timer_sync(&pd->timer);
> -
> - if (atomic_read(&pd->reorder_objects))
> - padata_reorder(pd);
> -
> - for_each_cpu(cpu, pd->cpumask.cbcpu) {
> - squeue = per_cpu_ptr(pd->squeue, cpu);
> - flush_work(&squeue->work);
> - }
> -
> - BUG_ON(atomic_read(&pd->refcnt) != 0);
> -}
> -
> static void __padata_start(struct padata_instance *pinst)
> {
> pinst->flags |= PADATA_INIT;
> @@ -564,10 +546,6 @@ static void __padata_stop(struct padata_instance *pinst)
> pinst->flags &= ~PADATA_INIT;
>
> synchronize_rcu();
> -
> - get_online_cpus();
> - padata_flush_queues(pinst->pd);
> - put_online_cpus();
> }
>
> /* Replace the internal control structure with a new one. */
> @@ -588,8 +566,8 @@ static void padata_replace(struct padata_instance *pinst,
> if (!cpumask_equal(pd_old->cpumask.cbcpu, pd_new->cpumask.cbcpu))
> notification_mask |= PADATA_CPU_SERIAL;
>
> - padata_flush_queues(pd_old);
> - padata_free_pd(pd_old);
> + if (atomic_dec_and_test(&pd_old->refcnt))
> + padata_free_pd(pd_old);
>
> if (notification_mask)
> blocking_notifier_call_chain(&pinst->cpumask_change_notifier,
Powered by blists - more mailing lists