Minki/linux
1
0
Fork 1
mirror of https://github.com/torvalds/linux.git synced 2024-11-10 22:21:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

TOMOYO: Fix lockdep warning.

Browse Source 
Currently TOMOYO holds SRCU lock upon open() and releases it upon close()
because list elements stored in the "struct tomoyo_io_buffer" instances are
accessed until close() is called. However, such SRCU usage causes lockdep to
complain about leaving the kernel with SRCU lock held.

This patch solves the warning by holding/releasing SRCU upon each
read()/write(). This patch is doing something similar to calling kfree()
without calling synchronize_srcu(), by selectively deferring kfree() by keeping
track of the "struct tomoyo_io_buffer" instances.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <jmorris@namei.org>
This commit is contained in:
Tetsuo Handa 2011-06-26 23:20:55 +09:00 committed by James Morris
parent 5625f2e326
commit 2e503bbb43
3 changed files with 276 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
security/tomoyo/common.c
View File

@ -1820,9 +1820,7 @@ static void tomoyo_read_self_domain(struct tomoyo_io_buffer *head)
* @type: Type of interface. * @type: Type of interface.
* @file: Pointer to "struct file". * @file: Pointer to "struct file".
* *
* Associates policy handler and returns 0 on success, -ENOMEM otherwise. * Returns 0 on success, negative value otherwise.
*
* Caller acquires tomoyo_read_lock().
*/ */
int tomoyo_open_control(const u8 type, struct file *file) int tomoyo_open_control(const u8 type, struct file *file)
{ {
@ -1921,9 +1919,6 @@ int tomoyo_open_control(const u8 type, struct file *file)
return -ENOMEM; return -ENOMEM;
} }
} }
if (type != TOMOYO_QUERY && type != TOMOYO_AUDIT)
head->reader_idx = tomoyo_read_lock();
file->private_data = head;
/* /*
* If the file is /sys/kernel/security/tomoyo/query , increment the * If the file is /sys/kernel/security/tomoyo/query , increment the
* observer counter. * observer counter.
@ -1932,6 +1927,8 @@ int tomoyo_open_control(const u8 type, struct file *file)
*/ */
if (type == TOMOYO_QUERY) if (type == TOMOYO_QUERY)
atomic_inc(&tomoyo_query_observers); atomic_inc(&tomoyo_query_observers);
file->private_data = head;
tomoyo_notify_gc(head, true);
return 0; return 0;
} }
@ -2000,13 +1997,12 @@ static inline bool tomoyo_has_more_namespace(struct tomoyo_io_buffer *head)
* @buffer_len: Size of @buffer. * @buffer_len: Size of @buffer.
* *
* Returns bytes read on success, negative value otherwise. * Returns bytes read on success, negative value otherwise.
*
* Caller holds tomoyo_read_lock().
*/ */
int tomoyo_read_control(struct tomoyo_io_buffer *head, char __user *buffer, int tomoyo_read_control(struct tomoyo_io_buffer *head, char __user *buffer,
const int buffer_len) const int buffer_len)
{ {
int len; int len;
int idx;
if (!head->read) if (!head->read)
return -ENOSYS; return -ENOSYS;
@ -2014,6 +2010,7 @@ int tomoyo_read_control(struct tomoyo_io_buffer *head, char __user *buffer,
return -EINTR; return -EINTR;
head->read_user_buf = buffer; head->read_user_buf = buffer;
head->read_user_buf_avail = buffer_len; head->read_user_buf_avail = buffer_len;
idx = tomoyo_read_lock();
if (tomoyo_flush(head)) if (tomoyo_flush(head))
/* Call the policy handler. */ /* Call the policy handler. */
do { do {
@ -2021,6 +2018,7 @@ int tomoyo_read_control(struct tomoyo_io_buffer *head, char __user *buffer,
head->read(head); head->read(head);
} while (tomoyo_flush(head) && } while (tomoyo_flush(head) &&
tomoyo_has_more_namespace(head)); tomoyo_has_more_namespace(head));
tomoyo_read_unlock(idx);
len = head->read_user_buf - buffer; len = head->read_user_buf - buffer;
mutex_unlock(&head->io_sem); mutex_unlock(&head->io_sem);
return len; return len;
@ -2071,8 +2069,6 @@ static int tomoyo_parse_policy(struct tomoyo_io_buffer *head, char *line)
* @buffer_len: Size of @buffer. * @buffer_len: Size of @buffer.
* *
* Returns @buffer_len on success, negative value otherwise. * Returns @buffer_len on success, negative value otherwise.
*
* Caller holds tomoyo_read_lock().
*/ */
int tomoyo_write_control(struct tomoyo_io_buffer *head, int tomoyo_write_control(struct tomoyo_io_buffer *head,
const char __user *buffer, const int buffer_len) const char __user *buffer, const int buffer_len)
@ -2080,12 +2076,14 @@ int tomoyo_write_control(struct tomoyo_io_buffer *head,
int error = buffer_len; int error = buffer_len;
size_t avail_len = buffer_len; size_t avail_len = buffer_len;
char *cp0 = head->write_buf; char *cp0 = head->write_buf;
int idx;
if (!head->write) if (!head->write)
return -ENOSYS; return -ENOSYS;
if (!access_ok(VERIFY_READ, buffer, buffer_len)) if (!access_ok(VERIFY_READ, buffer, buffer_len))
return -EFAULT; return -EFAULT;
if (mutex_lock_interruptible(&head->io_sem)) if (mutex_lock_interruptible(&head->io_sem))
return -EINTR; return -EINTR;
idx = tomoyo_read_lock();
/* Read a line and dispatch it to the policy handler. */ /* Read a line and dispatch it to the policy handler. */
while (avail_len > 0) { while (avail_len > 0) {
char c; char c;
@ -2148,6 +2146,7 @@ int tomoyo_write_control(struct tomoyo_io_buffer *head,
} }
} }
out: out:
tomoyo_read_unlock(idx);
mutex_unlock(&head->io_sem); mutex_unlock(&head->io_sem);
return error; return error;
} }
@ -2157,30 +2156,18 @@ out:
* *
* @head: Pointer to "struct tomoyo_io_buffer". * @head: Pointer to "struct tomoyo_io_buffer".
* *
* Releases memory and returns 0. * Returns 0.
*
* Caller looses tomoyo_read_lock().
*/ */
int tomoyo_close_control(struct tomoyo_io_buffer *head) int tomoyo_close_control(struct tomoyo_io_buffer *head)
{ {
const bool is_write = !!head->write_buf;
/* /*
* If the file is /sys/kernel/security/tomoyo/query , decrement the * If the file is /sys/kernel/security/tomoyo/query , decrement the
* observer counter. * observer counter.
*/ */
if (head->type == TOMOYO_QUERY) if (head->type == TOMOYO_QUERY &&
atomic_dec(&tomoyo_query_observers); atomic_dec_and_test(&tomoyo_query_observers))
else if (head->type != TOMOYO_AUDIT) wake_up_all(&tomoyo_answer_wait);
tomoyo_read_unlock(head->reader_idx); tomoyo_notify_gc(head, false);
/* Release memory used for policy I/O. */
kfree(head->read_buf);
head->read_buf = NULL;
kfree(head->write_buf);
head->write_buf = NULL;
kfree(head);
if (is_write)
tomoyo_run_gc();
return 0; return 0;
} }

8
security/tomoyo/common.h
View File

@ -441,8 +441,6 @@ struct tomoyo_io_buffer {
int (*poll) (struct file *file, poll_table *wait); int (*poll) (struct file *file, poll_table *wait);
/* Exclusive lock for this structure. */ /* Exclusive lock for this structure. */
struct mutex io_sem; struct mutex io_sem;
/* Index returned by tomoyo_read_lock(). */
int reader_idx;
char __user *read_user_buf; char __user *read_user_buf;
int read_user_buf_avail; int read_user_buf_avail;
struct { struct {
@ -480,6 +478,10 @@ struct tomoyo_io_buffer {
int writebuf_size; int writebuf_size;
/* Type of this interface. */ /* Type of this interface. */
u8 type; u8 type;
/* Users counter protected by tomoyo_io_buffer_list_lock. */
u8 users;
/* List for telling GC not to kfree() elements. */
struct list_head list;
}; };
/* /*
@ -651,7 +653,7 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm);
void tomoyo_print_ulong(char *buffer, const int buffer_len, void tomoyo_print_ulong(char *buffer, const int buffer_len,
const unsigned long value, const u8 type); const unsigned long value, const u8 type);
void tomoyo_put_name_union(struct tomoyo_name_union *ptr); void tomoyo_put_name_union(struct tomoyo_name_union *ptr);
void tomoyo_run_gc(void); void tomoyo_notify_gc(struct tomoyo_io_buffer *head, const bool is_register);
void tomoyo_memory_free(void *ptr); void tomoyo_memory_free(void *ptr);
int tomoyo_update_domain(struct tomoyo_acl_info *new_entry, const int size, int tomoyo_update_domain(struct tomoyo_acl_info *new_entry, const int size,
struct tomoyo_acl_param *param, struct tomoyo_acl_param *param,

278
security/tomoyo/gc.c
View File

@ -11,13 +11,123 @@
#include <linux/kthread.h> #include <linux/kthread.h>
#include <linux/slab.h> #include <linux/slab.h>
/* The list for "struct tomoyo_io_buffer". */
static LIST_HEAD(tomoyo_io_buffer_list);
/* Lock for protecting tomoyo_io_buffer_list. */
static DEFINE_SPINLOCK(tomoyo_io_buffer_list_lock);
/* Size of an element. */
static const u8 tomoyo_element_size[TOMOYO_MAX_POLICY] = {
[TOMOYO_ID_GROUP] = sizeof(struct tomoyo_group),
[TOMOYO_ID_PATH_GROUP] = sizeof(struct tomoyo_path_group),
[TOMOYO_ID_NUMBER_GROUP] = sizeof(struct tomoyo_number_group),
[TOMOYO_ID_AGGREGATOR] = sizeof(struct tomoyo_aggregator),
[TOMOYO_ID_TRANSITION_CONTROL] =
sizeof(struct tomoyo_transition_control),
[TOMOYO_ID_MANAGER] = sizeof(struct tomoyo_manager),
/* [TOMOYO_ID_NAME] = "struct tomoyo_name"->size, */
/* [TOMOYO_ID_ACL] =
tomoyo_acl_size["struct tomoyo_acl_info"->type], */
[TOMOYO_ID_DOMAIN] = sizeof(struct tomoyo_domain_info),
};
/* Size of a domain ACL element. */
static const u8 tomoyo_acl_size[] = {
[TOMOYO_TYPE_PATH_ACL] = sizeof(struct tomoyo_path_acl),
[TOMOYO_TYPE_PATH2_ACL] = sizeof(struct tomoyo_path2_acl),
[TOMOYO_TYPE_PATH_NUMBER_ACL] = sizeof(struct tomoyo_path_number_acl),
[TOMOYO_TYPE_MKDEV_ACL] = sizeof(struct tomoyo_mkdev_acl),
[TOMOYO_TYPE_MOUNT_ACL] = sizeof(struct tomoyo_mount_acl),
};
/**
* tomoyo_struct_used_by_io_buffer - Check whether the list element is used by /sys/kernel/security/tomoyo/ users or not.
*
* @element: Pointer to "struct list_head".
*
* Returns true if @element is used by /sys/kernel/security/tomoyo/ users,
* false otherwise.
*/
static bool tomoyo_struct_used_by_io_buffer(const struct list_head *element)
{
struct tomoyo_io_buffer *head;
bool in_use = false;
spin_lock(&tomoyo_io_buffer_list_lock);
list_for_each_entry(head, &tomoyo_io_buffer_list, list) {
head->users++;
spin_unlock(&tomoyo_io_buffer_list_lock);
if (mutex_lock_interruptible(&head->io_sem)) {
in_use = true;
goto out;
}
if (head->r.domain == element || head->r.group == element ||
head->r.acl == element || &head->w.domain->list == element)
in_use = true;
mutex_unlock(&head->io_sem);
out:
spin_lock(&tomoyo_io_buffer_list_lock);
head->users--;
if (in_use)
break;
}
spin_unlock(&tomoyo_io_buffer_list_lock);
return in_use;
}
/**
* tomoyo_name_used_by_io_buffer - Check whether the string is used by /sys/kernel/security/tomoyo/ users or not.
*
* @string: String to check.
* @size: Memory allocated for @string .
*
* Returns true if @string is used by /sys/kernel/security/tomoyo/ users,
* false otherwise.
*/
static bool tomoyo_name_used_by_io_buffer(const char *string,
const size_t size)
{
struct tomoyo_io_buffer *head;
bool in_use = false;
spin_lock(&tomoyo_io_buffer_list_lock);
list_for_each_entry(head, &tomoyo_io_buffer_list, list) {
int i;
head->users++;
spin_unlock(&tomoyo_io_buffer_list_lock);
if (mutex_lock_interruptible(&head->io_sem)) {
in_use = true;
goto out;
}
for (i = 0; i < TOMOYO_MAX_IO_READ_QUEUE; i++) {
const char *w = head->r.w[i];
if (w < string || w > string + size)
continue;
in_use = true;
break;
}
mutex_unlock(&head->io_sem);
out:
spin_lock(&tomoyo_io_buffer_list_lock);
head->users--;
if (in_use)
break;
}
spin_unlock(&tomoyo_io_buffer_list_lock);
return in_use;
}
/* Structure for garbage collection. */
struct tomoyo_gc { struct tomoyo_gc {
struct list_head list; struct list_head list;
enum tomoyo_policy_id type; enum tomoyo_policy_id type;
size_t size;
struct list_head *element; struct list_head *element;
}; };
static LIST_HEAD(tomoyo_gc_queue); /* List of entries to be deleted. */
static DEFINE_MUTEX(tomoyo_gc_mutex); static LIST_HEAD(tomoyo_gc_list);
/* Length of tomoyo_gc_list. */
static int tomoyo_gc_list_len;
/** /**
* tomoyo_add_to_gc - Add an entry to to be deleted list. * tomoyo_add_to_gc - Add an entry to to be deleted list.
@ -43,10 +153,42 @@ static bool tomoyo_add_to_gc(const int type, struct list_head *element)
if (!entry) if (!entry)
return false; return false;
entry->type = type; entry->type = type;
if (type == TOMOYO_ID_ACL)
entry->size = tomoyo_acl_size[
container_of(element,
typeof(struct tomoyo_acl_info),
list)->type];
else if (type == TOMOYO_ID_NAME)
entry->size = strlen(container_of(element,
typeof(struct tomoyo_name),
head.list)->entry.name) + 1;
else
entry->size = tomoyo_element_size[type];
entry->element = element; entry->element = element;
list_add(&entry->list, &tomoyo_gc_queue); list_add(&entry->list, &tomoyo_gc_list);
list_del_rcu(element); list_del_rcu(element);
return true; return tomoyo_gc_list_len++ < 128;
}
/**
* tomoyo_element_linked_by_gc - Validate next element of an entry.
*
* @element: Pointer to an element.
* @size: Size of @element in byte.
*
* Returns true if @element is linked by other elements in the garbage
* collector's queue, false otherwise.
*/
static bool tomoyo_element_linked_by_gc(const u8 *element, const size_t size)
{
struct tomoyo_gc *p;
list_for_each_entry(p, &tomoyo_gc_list, list) {
const u8 *ptr = (const u8 *) p->element->next;
if (ptr < element || element + size < ptr)
continue;
return true;
}
return false;
} }
/** /**
@ -151,6 +293,13 @@ static void tomoyo_del_acl(struct list_head *element)
} }
} }
/**
* tomoyo_del_domain - Delete members in "struct tomoyo_domain_info".
*
* @element: Pointer to "struct list_head".
*
* Returns true if deleted, false otherwise.
*/
static bool tomoyo_del_domain(struct list_head *element) static bool tomoyo_del_domain(struct list_head *element)
{ {
struct tomoyo_domain_info *domain = struct tomoyo_domain_info *domain =
@ -360,13 +509,44 @@ unlock:
mutex_unlock(&tomoyo_policy_lock); mutex_unlock(&tomoyo_policy_lock);
} }
static void tomoyo_kfree_entry(void) /**
* tomoyo_kfree_entry - Delete entries in tomoyo_gc_list.
*
* Returns true if some entries were kfree()d, false otherwise.
*/
static bool tomoyo_kfree_entry(void)
{ {
struct tomoyo_gc *p; struct tomoyo_gc *p;
struct tomoyo_gc *tmp; struct tomoyo_gc *tmp;
bool result = false;
list_for_each_entry_safe(p, tmp, &tomoyo_gc_queue, list) { list_for_each_entry_safe(p, tmp, &tomoyo_gc_list, list) {
struct list_head *element = p->element; struct list_head *element = p->element;
/*
* list_del_rcu() in tomoyo_add_to_gc() guarantees that the
* list element became no longer reachable from the list which
* the element was originally on (e.g. tomoyo_domain_list).
* Also, synchronize_srcu() in tomoyo_gc_thread() guarantees
* that the list element became no longer referenced by syscall
* users.
*
* However, there are three users which may still be using the
* list element. We need to defer until all of these users
* forget the list element.
*
* Firstly, defer until "struct tomoyo_io_buffer"->r.{domain,
* group,acl} and "struct tomoyo_io_buffer"->w.domain forget
* the list element.
*/
if (tomoyo_struct_used_by_io_buffer(element))
continue;
/*
* Secondly, defer until all other elements in the
* tomoyo_gc_list list forget the list element.
*/
if (tomoyo_element_linked_by_gc((const u8 *) element, p->size))
continue;
switch (p->type) { switch (p->type) {
case TOMOYO_ID_TRANSITION_CONTROL: case TOMOYO_ID_TRANSITION_CONTROL:
tomoyo_del_transition_control(element); tomoyo_del_transition_control(element);
@ -378,6 +558,14 @@ static void tomoyo_kfree_entry(void)
tomoyo_del_manager(element); tomoyo_del_manager(element);
break; break;
case TOMOYO_ID_NAME: case TOMOYO_ID_NAME:
/*
* Thirdly, defer until all "struct tomoyo_io_buffer"
* ->r.w[] forget the list element.
*/
if (tomoyo_name_used_by_io_buffer(
container_of(element, typeof(struct tomoyo_name),
head.list)->entry.name, p->size))
continue;
tomoyo_del_name(element); tomoyo_del_name(element);
break; break;
case TOMOYO_ID_ACL: case TOMOYO_ID_ACL:
@ -402,7 +590,10 @@ static void tomoyo_kfree_entry(void)
tomoyo_memory_free(element); tomoyo_memory_free(element);
list_del(&p->list); list_del(&p->list);
kfree(p); kfree(p);
tomoyo_gc_list_len--;
result = true;
} }
return result;
} }
/** /**
@ -418,25 +609,70 @@ static void tomoyo_kfree_entry(void)
*/ */
static int tomoyo_gc_thread(void *unused) static int tomoyo_gc_thread(void *unused)
{ {
/* Garbage collector thread is exclusive. */
static DEFINE_MUTEX(tomoyo_gc_mutex);
if (!mutex_trylock(&tomoyo_gc_mutex))
goto out;
daemonize("GC for TOMOYO"); daemonize("GC for TOMOYO");
if (mutex_trylock(&tomoyo_gc_mutex)) { do {
int i; tomoyo_collect_entry();
for (i = 0; i < 10; i++) { if (list_empty(&tomoyo_gc_list))
tomoyo_collect_entry(); break;
if (list_empty(&tomoyo_gc_queue)) synchronize_srcu(&tomoyo_ss);
break; } while (tomoyo_kfree_entry());
synchronize_srcu(&tomoyo_ss); {
tomoyo_kfree_entry(); struct tomoyo_io_buffer *head;
struct tomoyo_io_buffer *tmp;
spin_lock(&tomoyo_io_buffer_list_lock);
list_for_each_entry_safe(head, tmp, &tomoyo_io_buffer_list,
list) {
if (head->users)
continue;
list_del(&head->list);
kfree(head->read_buf);
kfree(head->write_buf);
kfree(head);
} }
mutex_unlock(&tomoyo_gc_mutex); spin_unlock(&tomoyo_io_buffer_list_lock);
} }
do_exit(0); mutex_unlock(&tomoyo_gc_mutex);
out:
/* This acts as do_exit(0). */
return 0;
} }
void tomoyo_run_gc(void) /**
* tomoyo_notify_gc - Register/unregister /sys/kernel/security/tomoyo/ users.
*
* @head: Pointer to "struct tomoyo_io_buffer".
* @is_register: True if register, false if unregister.
*
* Returns nothing.
*/
void tomoyo_notify_gc(struct tomoyo_io_buffer *head, const bool is_register)
{ {
struct task_struct *task = kthread_create(tomoyo_gc_thread, NULL, bool is_write = false;
"GC for TOMOYO");
if (!IS_ERR(task)) spin_lock(&tomoyo_io_buffer_list_lock);
wake_up_process(task); if (is_register) {
head->users = 1;
list_add(&head->list, &tomoyo_io_buffer_list);
} else {
is_write = head->write_buf != NULL;
if (!--head->users) {
list_del(&head->list);
kfree(head->read_buf);
kfree(head->write_buf);
kfree(head);
}
}
spin_unlock(&tomoyo_io_buffer_list_lock);
if (is_write) {
struct task_struct *task = kthread_create(tomoyo_gc_thread,
NULL,
"GC for TOMOYO");
if (!IS_ERR(task))
wake_up_process(task);
}
} }