I'm looking at proto_unregister() in linux-2.6.13:
Il calls kmem_cache_destroy() while holding proto_list_lock:
void proto_unregister(struct proto *prot)
{
write_lock(&proto_list_lock);
if (prot->slab != NULL) {
kmem_cache_destroy(prot->slab);
However, kmem_cache_destroy() may sleep:
int kmem_cache_destroy (kmem_cache_t * cachep)
{
int i;
if (!cachep || in_interrupt())
BUG();
/* Don't let CPUs to come and go */
lock_cpu_hotplug();
/* Find the cache in the chain of caches. */
down(&cache_chain_sem);
^^^^^^^^^^^^^^^^^^^^^^^
Am I seeing it right?
Bye,
--
Daniele Orlandi
[NET]: proto_unregister: fix sleeping while atomic
proto_unregister holds a lock while calling kmem_cache_destroy, which can
sleep.
Noticed by Daniele Orlandi <[email protected]>.
Signed-off-by: Patrick McHardy <[email protected]>
---
commit 90fdf8bc78b52d40190766c5496a3d8c24903be8
tree 54048218d981ab263b37e0de09ad52f4bd49a000
parent 591bd554f58b7d363167760a606d2a84696772da
author Patrick McHardy <[email protected]> Wed, 07 Sep 2005 01:00:00 +0200
committer Patrick McHardy <[email protected]> Wed, 07 Sep 2005 01:00:00 +0200
net/core/sock.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/net/core/sock.c b/net/core/sock.c
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1528,8 +1528,6 @@ EXPORT_SYMBOL(proto_register);
void proto_unregister(struct proto *prot)
{
- write_lock(&proto_list_lock);
-
if (prot->slab != NULL) {
kmem_cache_destroy(prot->slab);
prot->slab = NULL;
@@ -1551,6 +1549,7 @@ void proto_unregister(struct proto *prot
prot->twsk_slab = NULL;
}
+ write_lock(&proto_list_lock);
list_del(&prot->node);
write_unlock(&proto_list_lock);
}
From: Patrick McHardy <[email protected]>
Date: Wed, 07 Sep 2005 01:02:01 +0200
> You're right, good catch. This patch fixes it by moving the lock
> down to the list-operation which it is supposed to protect.
I think we need to unlink from the list first if you're
going to do it this way. Otherwise someone can find the
protocol via lookup, and then bogusly try to use the SLAB
cache we're freeing up.
Or does something else prevent this?
[NET]: proto_unregister: fix sleeping while atomic
proto_unregister holds a lock while calling kmem_cache_destroy, which can
sleep.
Noticed by Daniele Orlandi <[email protected]>.
Signed-off-by: Patrick McHardy <[email protected]>
---
commit d68b08edb26dfb58d18ab6c555d011572f9115a6
tree 1d14cf91ca5db6878b6af3953f85a34a6fe12a91
parent 591bd554f58b7d363167760a606d2a84696772da
author Patrick McHardy <[email protected]> Wed, 07 Sep 2005 01:35:19 +0200
committer Patrick McHardy <[email protected]> Wed, 07 Sep 2005 01:35:19 +0200
net/core/sock.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/core/sock.c b/net/core/sock.c
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1529,6 +1529,8 @@ EXPORT_SYMBOL(proto_register);
void proto_unregister(struct proto *prot)
{
write_lock(&proto_list_lock);
+ list_del(&prot->node);
+ write_unlock(&proto_list_lock);
if (prot->slab != NULL) {
kmem_cache_destroy(prot->slab);
@@ -1550,9 +1552,6 @@ void proto_unregister(struct proto *prot
kfree(name);
prot->twsk_slab = NULL;
}
-
- list_del(&prot->node);
- write_unlock(&proto_list_lock);
}
EXPORT_SYMBOL(proto_unregister);
From: Patrick McHardy <[email protected]>
Date: Wed, 07 Sep 2005 01:42:48 +0200
> The only other user of proto_list besides proto_register, which
> doesn't care, are the seqfs functions. They use the slab pointer,
> but in a harmless way:
>
> proto->slab == NULL ? "no" : "yes",
>
> Anyway, I've moved it up to the top.
Ok thanks, patch applied.