Skip to content

Commit

Permalink
TOMOYO: Fix lockdep warning.
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Tetsuo Handa authored and James Morris committed Jun 28, 2011
1 parent 5625f2e commit 2e503bb
Show file tree
Hide file tree
Showing 3 changed files with 276 additions and 51 deletions.
41 changes: 14 additions & 27 deletions security/tomoyo/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -1820,9 +1820,7 @@ static void tomoyo_read_self_domain(struct tomoyo_io_buffer *head)
* @type: Type of interface.
* @file: Pointer to "struct file".
*
* Associates policy handler and returns 0 on success, -ENOMEM otherwise.
*
* Caller acquires tomoyo_read_lock().
* Returns 0 on success, negative value otherwise.
*/
int tomoyo_open_control(const u8 type, struct file *file)
{
Expand Down Expand Up @@ -1921,9 +1919,6 @@ int tomoyo_open_control(const u8 type, struct file *file)
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
* observer counter.
Expand All @@ -1932,6 +1927,8 @@ int tomoyo_open_control(const u8 type, struct file *file)
*/
if (type == TOMOYO_QUERY)
atomic_inc(&tomoyo_query_observers);
file->private_data = head;
tomoyo_notify_gc(head, true);
return 0;
}

Expand Down Expand Up @@ -2000,27 +1997,28 @@ static inline bool tomoyo_has_more_namespace(struct tomoyo_io_buffer *head)
* @buffer_len: Size of @buffer.
*
* 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,
const int buffer_len)
{
int len;
int idx;

if (!head->read)
return -ENOSYS;
if (mutex_lock_interruptible(&head->io_sem))
return -EINTR;
head->read_user_buf = buffer;
head->read_user_buf_avail = buffer_len;
idx = tomoyo_read_lock();
if (tomoyo_flush(head))
/* Call the policy handler. */
do {
tomoyo_set_namespace_cursor(head);
head->read(head);
} while (tomoyo_flush(head) &&
tomoyo_has_more_namespace(head));
tomoyo_read_unlock(idx);
len = head->read_user_buf - buffer;
mutex_unlock(&head->io_sem);
return len;
Expand Down Expand Up @@ -2071,21 +2069,21 @@ static int tomoyo_parse_policy(struct tomoyo_io_buffer *head, char *line)
* @buffer_len: Size of @buffer.
*
* Returns @buffer_len on success, negative value otherwise.
*
* Caller holds tomoyo_read_lock().
*/
int tomoyo_write_control(struct tomoyo_io_buffer *head,
const char __user *buffer, const int buffer_len)
{
int error = buffer_len;
size_t avail_len = buffer_len;
char *cp0 = head->write_buf;
int idx;
if (!head->write)
return -ENOSYS;
if (!access_ok(VERIFY_READ, buffer, buffer_len))
return -EFAULT;
if (mutex_lock_interruptible(&head->io_sem))
return -EINTR;
idx = tomoyo_read_lock();
/* Read a line and dispatch it to the policy handler. */
while (avail_len > 0) {
char c;
Expand Down Expand Up @@ -2148,6 +2146,7 @@ int tomoyo_write_control(struct tomoyo_io_buffer *head,
}
}
out:
tomoyo_read_unlock(idx);
mutex_unlock(&head->io_sem);
return error;
}
Expand All @@ -2157,30 +2156,18 @@ int tomoyo_write_control(struct tomoyo_io_buffer *head,
*
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Releases memory and returns 0.
*
* Caller looses tomoyo_read_lock().
* Returns 0.
*/
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
* observer counter.
*/
if (head->type == TOMOYO_QUERY)
atomic_dec(&tomoyo_query_observers);
else if (head->type != TOMOYO_AUDIT)
tomoyo_read_unlock(head->reader_idx);
/* 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();
if (head->type == TOMOYO_QUERY &&
atomic_dec_and_test(&tomoyo_query_observers))
wake_up_all(&tomoyo_answer_wait);
tomoyo_notify_gc(head, false);
return 0;
}

Expand Down
8 changes: 5 additions & 3 deletions security/tomoyo/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,6 @@ struct tomoyo_io_buffer {
int (*poll) (struct file *file, poll_table *wait);
/* Exclusive lock for this structure. */
struct mutex io_sem;
/* Index returned by tomoyo_read_lock(). */
int reader_idx;
char __user *read_user_buf;
int read_user_buf_avail;
struct {
Expand Down Expand Up @@ -480,6 +478,10 @@ struct tomoyo_io_buffer {
int writebuf_size;
/* Type of this interface. */
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;
};

/*
Expand Down Expand Up @@ -651,7 +653,7 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm);
void tomoyo_print_ulong(char *buffer, const int buffer_len,
const unsigned long value, const u8 type);
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);
int tomoyo_update_domain(struct tomoyo_acl_info *new_entry, const int size,
struct tomoyo_acl_param *param,
Expand Down
Loading

0 comments on commit 2e503bb

Please sign in to comment.