Skip to content

Commit

Permalink
audit: Fix possible tagging failures
Browse files Browse the repository at this point in the history
Audit tree code is replacing marks attached to inodes in non-atomic way.
Thus fsnotify_find_mark() in tag_chunk() may find a mark that belongs to
a chunk that is no longer valid one and will soon be destroyed. Tags
added to such chunk will be simply lost.

Fix the problem by making sure old mark is marked as going away (through
fsnotify_detach_mark()) before dropping mark_mutex and thus in an atomic
way wrt tag_chunk(). Note that this does not fix the problem completely
as if tag_chunk() finds a mark that is going away, it fails with
-ENOENT. But at least the failure is not silent and currently there's no
way to search for another fsnotify mark attached to the inode. We'll fix
this problem in later patch.

Reviewed-by: Richard Guy Briggs <[email protected]>
Signed-off-by: Jan Kara <[email protected]>
Signed-off-by: Paul Moore <[email protected]>
  • Loading branch information
jankara authored and pcmoore committed Nov 12, 2018
1 parent a5789b0 commit b1e4603
Showing 1 changed file with 10 additions and 7 deletions.
17 changes: 10 additions & 7 deletions kernel/audit_tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,9 @@ static void untag_chunk(struct node *p)
list_del_init(&p->list);
list_del_rcu(&chunk->hash);
spin_unlock(&hash_lock);
fsnotify_detach_mark(entry);
mutex_unlock(&entry->group->mark_mutex);
fsnotify_destroy_mark(entry, audit_tree_group);
fsnotify_free_mark(entry);
goto out;
}

Expand Down Expand Up @@ -320,8 +321,9 @@ static void untag_chunk(struct node *p)
list_for_each_entry(owner, &new->trees, same_root)
owner->root = new;
spin_unlock(&hash_lock);
fsnotify_detach_mark(entry);
mutex_unlock(&entry->group->mark_mutex);
fsnotify_destroy_mark(entry, audit_tree_group);
fsnotify_free_mark(entry);
fsnotify_put_mark(&new->mark); /* drop initial reference */
goto out;

Expand Down Expand Up @@ -364,8 +366,9 @@ static int create_chunk(struct inode *inode, struct audit_tree *tree)
if (tree->goner) {
spin_unlock(&hash_lock);
chunk->dead = 1;
fsnotify_detach_mark(entry);
mutex_unlock(&audit_tree_group->mark_mutex);
fsnotify_destroy_mark(entry, audit_tree_group);
fsnotify_free_mark(entry);
fsnotify_put_mark(entry);
return 0;
}
Expand Down Expand Up @@ -446,10 +449,9 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
if (tree->goner) {
spin_unlock(&hash_lock);
chunk->dead = 1;
fsnotify_detach_mark(chunk_entry);
mutex_unlock(&audit_tree_group->mark_mutex);

fsnotify_destroy_mark(chunk_entry, audit_tree_group);

fsnotify_free_mark(chunk_entry);
fsnotify_put_mark(chunk_entry);
fsnotify_put_mark(old_entry);
return 0;
Expand Down Expand Up @@ -477,8 +479,9 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
list_add(&tree->same_root, &chunk->trees);
}
spin_unlock(&hash_lock);
fsnotify_detach_mark(old_entry);
mutex_unlock(&audit_tree_group->mark_mutex);
fsnotify_destroy_mark(old_entry, audit_tree_group);
fsnotify_free_mark(old_entry);
fsnotify_put_mark(chunk_entry); /* drop initial reference */
fsnotify_put_mark(old_entry); /* pair to fsnotify_find mark_entry */
return 0;
Expand Down

0 comments on commit b1e4603

Please sign in to comment.