* [PATCH 0/4] padata/pcrypt: fixes
@ 2010-07-20 6:47 ` Steffen Klassert
0 siblings, 0 replies; 20+ messages in thread
From: Steffen Klassert @ 2010-07-20 6:47 UTC (permalink / raw)
To: Herbert Xu; +Cc: Dan Kruchinin, Andrew Morton, linux-crypto, linux-kernel
This patchset contains the following fixes:
1. padata - Fix a potential hang of the parallel worker threads on
cpu hotplug.
2. padata - Fix two NULL pointer dereference crashes if one of the
padata cpumasks is empty. The crashes can be triggered by doing
echo 0 > /sys/kernel/pcrypt/pencrypt/[parallel_cpumask/serial_cpumask]
3. padata - Fix a division by zero crash if the parallel cpumask
contains no active cpu. Can be triggered by doing
echo 0 > /sys/kernel/pcrypt/pencrypt/parallel_cpumask
4. pcrypt - Fix division by zero crash if the callback cpumask
is empty. Can be triggered by doing
echo 0 > /sys/kernel/pcrypt/pencrypt/serial_cpumask
I ran a script that does cpumask changes and cpu hotplugs as fast as
possible while sending IPsec packets on maximal rate with pcrypt.
It showed no furter crashes overnight, so I hope I've got them all.
padata API corrections and some minor fixes/cleanups are comming later.
Steffen
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/4] padata: Fix cpu index counting
2010-07-20 6:47 ` Steffen Klassert
@ 2010-07-20 6:48 ` Steffen Klassert
-1 siblings, 0 replies; 20+ messages in thread
From: Steffen Klassert @ 2010-07-20 6:48 UTC (permalink / raw)
To: Herbert Xu; +Cc: Dan Kruchinin, Andrew Morton, linux-crypto, linux-kernel
The counting of the cpu index got lost with a recent commit.
This patch restores it. This fixes a hang of the parallel worker
threads on cpu hotplug.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
kernel/padata.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/kernel/padata.c b/kernel/padata.c
index 526f9ea..4287868 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -408,6 +408,7 @@ static void padata_init_pqueues(struct parallel_data *pd)
pqueue = per_cpu_ptr(pd->pqueue, cpu);
pqueue->pd = pd;
pqueue->cpu_index = cpu_index;
+ cpu_index++;
__padata_list_init(&pqueue->reorder);
__padata_list_init(&pqueue->parallel);
--
1.5.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/4] padata: Fix cpu index counting
@ 2010-07-20 6:48 ` Steffen Klassert
0 siblings, 0 replies; 20+ messages in thread
From: Steffen Klassert @ 2010-07-20 6:48 UTC (permalink / raw)
To: Herbert Xu; +Cc: Dan Kruchinin, Andrew Morton, linux-crypto, linux-kernel
The counting of the cpu index got lost with a recent commit.
This patch restores it. This fixes a hang of the parallel worker
threads on cpu hotplug.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
kernel/padata.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/kernel/padata.c b/kernel/padata.c
index 526f9ea..4287868 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -408,6 +408,7 @@ static void padata_init_pqueues(struct parallel_data *pd)
pqueue = per_cpu_ptr(pd->pqueue, cpu);
pqueue->pd = pd;
pqueue->cpu_index = cpu_index;
+ cpu_index++;
__padata_list_init(&pqueue->reorder);
__padata_list_init(&pqueue->parallel);
--
1.5.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/4] padata: Allocate cpumask dependend recources in any case
2010-07-20 6:47 ` Steffen Klassert
@ 2010-07-20 6:49 ` Steffen Klassert
-1 siblings, 0 replies; 20+ messages in thread
From: Steffen Klassert @ 2010-07-20 6:49 UTC (permalink / raw)
To: Herbert Xu; +Cc: Dan Kruchinin, Andrew Morton, linux-crypto, linux-kernel
The cpumask separation work assumes the cpumask dependend recources
present regardless of valid or invalid cpumasks. With this patch
we allocate the cpumask dependend recources in any case. This fixes
two NULL pointer dereference crashes in padata_replace and in
padata_get_cpumask.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
kernel/padata.c | 24 +++++++-----------------
1 files changed, 7 insertions(+), 17 deletions(-)
diff --git a/kernel/padata.c b/kernel/padata.c
index 4287868..6a51945 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -417,7 +417,7 @@ static void padata_init_pqueues(struct parallel_data *pd)
}
num_cpus = cpumask_weight(pd->cpumask.pcpu);
- pd->max_seq_nr = (MAX_SEQ_NR / num_cpus) * num_cpus - 1;
+ pd->max_seq_nr = num_cpus ? (MAX_SEQ_NR / num_cpus) * num_cpus - 1 : 0;
}
/* Allocate and initialize the internal cpumask dependend resources. */
@@ -527,21 +527,19 @@ static void padata_replace(struct padata_instance *pinst,
rcu_assign_pointer(pinst->pd, pd_new);
synchronize_rcu();
- if (!pd_old)
- goto out;
- padata_flush_queues(pd_old);
if (!cpumask_equal(pd_old->cpumask.pcpu, pd_new->cpumask.pcpu))
notification_mask |= PADATA_CPU_PARALLEL;
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 (notification_mask)
blocking_notifier_call_chain(&pinst->cpumask_change_notifier,
notification_mask, pinst);
-out:
pinst->flags &= ~PADATA_RESET;
}
@@ -673,6 +671,7 @@ int __padata_set_cpumasks(struct padata_instance *pinst,
struct parallel_data *pd = NULL;
mutex_lock(&pinst->lock);
+ get_online_cpus();
valid = padata_validate_cpumask(pinst, pcpumask);
if (!valid) {
@@ -681,20 +680,16 @@ int __padata_set_cpumasks(struct padata_instance *pinst,
}
valid = padata_validate_cpumask(pinst, cbcpumask);
- if (!valid) {
+ if (!valid)
__padata_stop(pinst);
- goto out_replace;
- }
-
- get_online_cpus();
+out_replace:
pd = padata_alloc_pd(pinst, pcpumask, cbcpumask);
if (!pd) {
err = -ENOMEM;
goto out;
}
-out_replace:
cpumask_copy(pinst->cpumask.pcpu, pcpumask);
cpumask_copy(pinst->cpumask.cbcpu, cbcpumask);
@@ -705,7 +700,6 @@ out_replace:
out:
put_online_cpus();
-
mutex_unlock(&pinst->lock);
return err;
@@ -776,11 +770,8 @@ static int __padata_remove_cpu(struct padata_instance *pinst, int cpu)
if (cpumask_test_cpu(cpu, cpu_online_mask)) {
if (!padata_validate_cpumask(pinst, pinst->cpumask.pcpu) ||
- !padata_validate_cpumask(pinst, pinst->cpumask.cbcpu)) {
+ !padata_validate_cpumask(pinst, pinst->cpumask.cbcpu))
__padata_stop(pinst);
- padata_replace(pinst, pd);
- goto out;
- }
pd = padata_alloc_pd(pinst, pinst->cpumask.pcpu,
pinst->cpumask.cbcpu);
@@ -790,7 +781,6 @@ static int __padata_remove_cpu(struct padata_instance *pinst, int cpu)
padata_replace(pinst, pd);
}
-out:
return 0;
}
--
1.5.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 2/4] padata: Allocate cpumask dependend recources in any case
@ 2010-07-20 6:49 ` Steffen Klassert
0 siblings, 0 replies; 20+ messages in thread
From: Steffen Klassert @ 2010-07-20 6:49 UTC (permalink / raw)
To: Herbert Xu; +Cc: Dan Kruchinin, Andrew Morton, linux-crypto, linux-kernel
The cpumask separation work assumes the cpumask dependend recources
present regardless of valid or invalid cpumasks. With this patch
we allocate the cpumask dependend recources in any case. This fixes
two NULL pointer dereference crashes in padata_replace and in
padata_get_cpumask.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
kernel/padata.c | 24 +++++++-----------------
1 files changed, 7 insertions(+), 17 deletions(-)
diff --git a/kernel/padata.c b/kernel/padata.c
index 4287868..6a51945 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -417,7 +417,7 @@ static void padata_init_pqueues(struct parallel_data *pd)
}
num_cpus = cpumask_weight(pd->cpumask.pcpu);
- pd->max_seq_nr = (MAX_SEQ_NR / num_cpus) * num_cpus - 1;
+ pd->max_seq_nr = num_cpus ? (MAX_SEQ_NR / num_cpus) * num_cpus - 1 : 0;
}
/* Allocate and initialize the internal cpumask dependend resources. */
@@ -527,21 +527,19 @@ static void padata_replace(struct padata_instance *pinst,
rcu_assign_pointer(pinst->pd, pd_new);
synchronize_rcu();
- if (!pd_old)
- goto out;
- padata_flush_queues(pd_old);
if (!cpumask_equal(pd_old->cpumask.pcpu, pd_new->cpumask.pcpu))
notification_mask |= PADATA_CPU_PARALLEL;
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 (notification_mask)
blocking_notifier_call_chain(&pinst->cpumask_change_notifier,
notification_mask, pinst);
-out:
pinst->flags &= ~PADATA_RESET;
}
@@ -673,6 +671,7 @@ int __padata_set_cpumasks(struct padata_instance *pinst,
struct parallel_data *pd = NULL;
mutex_lock(&pinst->lock);
+ get_online_cpus();
valid = padata_validate_cpumask(pinst, pcpumask);
if (!valid) {
@@ -681,20 +680,16 @@ int __padata_set_cpumasks(struct padata_instance *pinst,
}
valid = padata_validate_cpumask(pinst, cbcpumask);
- if (!valid) {
+ if (!valid)
__padata_stop(pinst);
- goto out_replace;
- }
-
- get_online_cpus();
+out_replace:
pd = padata_alloc_pd(pinst, pcpumask, cbcpumask);
if (!pd) {
err = -ENOMEM;
goto out;
}
-out_replace:
cpumask_copy(pinst->cpumask.pcpu, pcpumask);
cpumask_copy(pinst->cpumask.cbcpu, cbcpumask);
@@ -705,7 +700,6 @@ out_replace:
out:
put_online_cpus();
-
mutex_unlock(&pinst->lock);
return err;
@@ -776,11 +770,8 @@ static int __padata_remove_cpu(struct padata_instance *pinst, int cpu)
if (cpumask_test_cpu(cpu, cpu_online_mask)) {
if (!padata_validate_cpumask(pinst, pinst->cpumask.pcpu) ||
- !padata_validate_cpumask(pinst, pinst->cpumask.cbcpu)) {
+ !padata_validate_cpumask(pinst, pinst->cpumask.cbcpu))
__padata_stop(pinst);
- padata_replace(pinst, pd);
- goto out;
- }
pd = padata_alloc_pd(pinst, pinst->cpumask.pcpu,
pinst->cpumask.cbcpu);
@@ -790,7 +781,6 @@ static int __padata_remove_cpu(struct padata_instance *pinst, int cpu)
padata_replace(pinst, pd);
}
-out:
return 0;
}
--
1.5.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/4] padata: Check for valid cpumasks
2010-07-20 6:47 ` Steffen Klassert
@ 2010-07-20 6:51 ` Steffen Klassert
-1 siblings, 0 replies; 20+ messages in thread
From: Steffen Klassert @ 2010-07-20 6:51 UTC (permalink / raw)
To: Herbert Xu; +Cc: Dan Kruchinin, Andrew Morton, linux-crypto, linux-kernel
Now that we allow to change the cpumasks from userspace, we have
to check for valid cpumasks in padata_do_parallel. This patch adds
the necessary check. This fixes a division by zero crash if the
parallel cpumask contains no active cpu.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
kernel/padata.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/padata.c b/kernel/padata.c
index 6a51945..7f895e2 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -114,7 +114,7 @@ int padata_do_parallel(struct padata_instance *pinst,
pd = rcu_dereference(pinst->pd);
err = -EINVAL;
- if (!(pinst->flags & PADATA_INIT))
+ if (!(pinst->flags & PADATA_INIT) || pinst->flags & PADATA_INVALID)
goto out;
if (!cpumask_test_cpu(cb_cpu, pd->cpumask.cbcpu))
--
1.5.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/4] padata: Check for valid cpumasks
@ 2010-07-20 6:51 ` Steffen Klassert
0 siblings, 0 replies; 20+ messages in thread
From: Steffen Klassert @ 2010-07-20 6:51 UTC (permalink / raw)
To: Herbert Xu; +Cc: Dan Kruchinin, Andrew Morton, linux-crypto, linux-kernel
Now that we allow to change the cpumasks from userspace, we have
to check for valid cpumasks in padata_do_parallel. This patch adds
the necessary check. This fixes a division by zero crash if the
parallel cpumask contains no active cpu.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
kernel/padata.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/padata.c b/kernel/padata.c
index 6a51945..7f895e2 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -114,7 +114,7 @@ int padata_do_parallel(struct padata_instance *pinst,
pd = rcu_dereference(pinst->pd);
err = -EINVAL;
- if (!(pinst->flags & PADATA_INIT))
+ if (!(pinst->flags & PADATA_INIT) || pinst->flags & PADATA_INVALID)
goto out;
if (!cpumask_test_cpu(cb_cpu, pd->cpumask.cbcpu))
--
1.5.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/4] crypto: pcrypt - Dont calulate a callback cpu on empty callback cpumask
2010-07-20 6:47 ` Steffen Klassert
@ 2010-07-20 6:52 ` Steffen Klassert
-1 siblings, 0 replies; 20+ messages in thread
From: Steffen Klassert @ 2010-07-20 6:52 UTC (permalink / raw)
To: Herbert Xu; +Cc: Dan Kruchinin, Andrew Morton, linux-crypto, linux-kernel
If the callback cpumask is empty, we crash with a division by zero
when we try to calculate a callback cpu. So we don't update the callback
cpu in pcrypt_do_parallel if the callback cpumask is empty.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
crypto/pcrypt.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
index 7153a50..794c172 100644
--- a/crypto/pcrypt.c
+++ b/crypto/pcrypt.c
@@ -82,6 +82,9 @@ static int pcrypt_do_parallel(struct padata_priv *padata, unsigned int *cb_cpu,
if (cpumask_test_cpu(cpu, cpumask->mask))
goto out;
+ if (!cpumask_weight(cpumask->mask))
+ goto out;
+
cpu_index = cpu % cpumask_weight(cpumask->mask);
cpu = cpumask_first(cpumask->mask);
--
1.5.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/4] crypto: pcrypt - Dont calulate a callback cpu on empty callback cpumask
@ 2010-07-20 6:52 ` Steffen Klassert
0 siblings, 0 replies; 20+ messages in thread
From: Steffen Klassert @ 2010-07-20 6:52 UTC (permalink / raw)
To: Herbert Xu; +Cc: Dan Kruchinin, Andrew Morton, linux-crypto, linux-kernel
If the callback cpumask is empty, we crash with a division by zero
when we try to calculate a callback cpu. So we don't update the callback
cpu in pcrypt_do_parallel if the callback cpumask is empty.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
crypto/pcrypt.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
index 7153a50..794c172 100644
--- a/crypto/pcrypt.c
+++ b/crypto/pcrypt.c
@@ -82,6 +82,9 @@ static int pcrypt_do_parallel(struct padata_priv *padata, unsigned int *cb_cpu,
if (cpumask_test_cpu(cpu, cpumask->mask))
goto out;
+ if (!cpumask_weight(cpumask->mask))
+ goto out;
+
cpu_index = cpu % cpumask_weight(cpumask->mask);
cpu = cpumask_first(cpumask->mask);
--
1.5.6.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 0/4] padata/pcrypt: fixes
2010-07-20 6:47 ` Steffen Klassert
@ 2010-07-26 6:16 ` Herbert Xu
-1 siblings, 0 replies; 20+ messages in thread
From: Herbert Xu @ 2010-07-26 6:16 UTC (permalink / raw)
To: Steffen Klassert; +Cc: Dan Kruchinin, Andrew Morton, linux-crypto, linux-kernel
On Tue, Jul 20, 2010 at 08:47:36AM +0200, Steffen Klassert wrote:
> This patchset contains the following fixes:
All applied. Thanks Steffen!
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 20+ messages in thread