* [PATCH] fix braindamage in audit_tree.c untag_chunk()
@ 2009-12-19 15:59 Al Viro
2009-12-19 16:03 ` [PATCH] fix more leaks in audit_tree.c tag_chunk() Al Viro
0 siblings, 1 reply; 2+ messages in thread
From: Al Viro @ 2009-12-19 15:59 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, eparis
... aka "Al had badly fscked up when writing that thing and nobody
noticed until Eric had fixed leaks that used to mask the breakage".
The function essentially creates a copy of old array sans one element
and replaces the references to elements of original (they are on cyclic
lists) with those to corresponding elements of new one. After that the
old one is fair game for freeing.
First of all, there's a dumb braino: when we get to list_replace_init we
use indices for wrong arrays - position in new one with the old array and
vice versa.
Another bug is more subtle - termination condition is wrong if the element
to be excluded happens to be the last one. We shouldn't go until we fill
the new array, we should go until we'd finished the old one. Otherwise
the element we are trying to kill will remain on the cyclic lists...
That crap used to be masked by several leaks, so it was not quite trivial
to hit. Eric had fixed some of those leaks a while ago and the shit had
hit the fan...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -277,7 +277,7 @@ static void untag_chunk(struct node *p)
owner->root = NULL;
}
- for (i = j = 0; i < size; i++, j++) {
+ for (i = j = 0; j <= size; i++, j++) {
struct audit_tree *s;
if (&chunk->owners[j] == p) {
list_del_init(&p->list);
@@ -290,7 +290,7 @@ static void untag_chunk(struct node *p)
if (!s) /* result of earlier fallback */
continue;
get_tree(s);
- list_replace_init(&chunk->owners[i].list, &new->owners[j].list);
+ list_replace_init(&chunk->owners[j].list, &new->owners[i].list);
}
list_replace_rcu(&chunk->hash, &new->hash);
^ permalink raw reply [flat|nested] 2+ messages in thread
* [PATCH] fix more leaks in audit_tree.c tag_chunk()
2009-12-19 15:59 [PATCH] fix braindamage in audit_tree.c untag_chunk() Al Viro
@ 2009-12-19 16:03 ` Al Viro
0 siblings, 0 replies; 2+ messages in thread
From: Al Viro @ 2009-12-19 16:03 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, eparis
Several leaks in audit_tree didn't get caught by commit
318b6d3d7ddbcad3d6867e630711b8a705d873d7, including the leak on
normal exit in case of multiple rules refering to the same chunk.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -373,15 +373,17 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
for (n = 0; n < old->count; n++) {
if (old->owners[n].owner == tree) {
spin_unlock(&hash_lock);
- put_inotify_watch(watch);
+ put_inotify_watch(&old->watch);
return 0;
}
}
spin_unlock(&hash_lock);
chunk = alloc_chunk(old->count + 1);
- if (!chunk)
+ if (!chunk) {
+ put_inotify_watch(&old->watch);
return -ENOMEM;
+ }
mutex_lock(&inode->inotify_mutex);
if (inotify_clone_watch(&old->watch, &chunk->watch) < 0) {
@@ -425,7 +427,8 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
spin_unlock(&hash_lock);
inotify_evict_watch(&old->watch);
mutex_unlock(&inode->inotify_mutex);
- put_inotify_watch(&old->watch);
+ put_inotify_watch(&old->watch); /* pair to inotify_find_watch */
+ put_inotify_watch(&old->watch); /* and kill it */
return 0;
}
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2009-12-19 16:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-19 15:59 [PATCH] fix braindamage in audit_tree.c untag_chunk() Al Viro
2009-12-19 16:03 ` [PATCH] fix more leaks in audit_tree.c tag_chunk() Al Viro
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox