2009-10-04 12:53:37

by Tetsuo Handa

[permalink] [raw]
Subject: [TOMOYO #16 01/25] LSM: Add security_path_chmod() and security_path_chown().

This patch allows pathname based LSM modules to check chmod()/chown()
operations. Since notify_change() does not receive "struct vfsmount *",
we add security_path_chmod() and security_path_chown() to the caller of
notify_change().

These hooks are used by TOMOYO.

Signed-off-by: Tetsuo Handa <[email protected]>
---
fs/open.c | 24 ++++++++++++++++++++----
include/linux/security.h | 30 ++++++++++++++++++++++++++++++
security/capability.c | 13 +++++++++++++
security/security.c | 15 +++++++++++++++
4 files changed, 78 insertions(+), 4 deletions(-)

--- security-testing-2.6.orig/fs/open.c
+++ security-testing-2.6/fs/open.c
@@ -616,6 +616,9 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd
err = mnt_want_write_file(file);
if (err)
goto out_putf;
+ err = security_path_chmod(dentry, file->f_vfsmnt, mode);
+ if (err)
+ goto out_drop_write;
mutex_lock(&inode->i_mutex);
if (mode == (mode_t) -1)
mode = inode->i_mode;
@@ -623,6 +626,7 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd
newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
err = notify_change(dentry, &newattrs);
mutex_unlock(&inode->i_mutex);
+out_drop_write:
mnt_drop_write(file->f_path.mnt);
out_putf:
fput(file);
@@ -645,6 +649,9 @@ SYSCALL_DEFINE3(fchmodat, int, dfd, cons
error = mnt_want_write(path.mnt);
if (error)
goto dput_and_out;
+ error = security_path_chmod(path.dentry, path.mnt, mode);
+ if (error)
+ goto out_drop_write;
mutex_lock(&inode->i_mutex);
if (mode == (mode_t) -1)
mode = inode->i_mode;
@@ -652,6 +659,7 @@ SYSCALL_DEFINE3(fchmodat, int, dfd, cons
newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
error = notify_change(path.dentry, &newattrs);
mutex_unlock(&inode->i_mutex);
+out_drop_write:
mnt_drop_write(path.mnt);
dput_and_out:
path_put(&path);
@@ -700,7 +708,9 @@ SYSCALL_DEFINE3(chown, const char __user
error = mnt_want_write(path.mnt);
if (error)
goto out_release;
- error = chown_common(path.dentry, user, group);
+ error = security_path_chown(&path, user, group);
+ if (!error)
+ error = chown_common(path.dentry, user, group);
mnt_drop_write(path.mnt);
out_release:
path_put(&path);
@@ -725,7 +735,9 @@ SYSCALL_DEFINE5(fchownat, int, dfd, cons
error = mnt_want_write(path.mnt);
if (error)
goto out_release;
- error = chown_common(path.dentry, user, group);
+ error = security_path_chown(&path, user, group);
+ if (!error)
+ error = chown_common(path.dentry, user, group);
mnt_drop_write(path.mnt);
out_release:
path_put(&path);
@@ -744,7 +756,9 @@ SYSCALL_DEFINE3(lchown, const char __use
error = mnt_want_write(path.mnt);
if (error)
goto out_release;
- error = chown_common(path.dentry, user, group);
+ error = security_path_chown(&path, user, group);
+ if (!error)
+ error = chown_common(path.dentry, user, group);
mnt_drop_write(path.mnt);
out_release:
path_put(&path);
@@ -767,7 +781,9 @@ SYSCALL_DEFINE3(fchown, unsigned int, fd
goto out_fput;
dentry = file->f_path.dentry;
audit_inode(NULL, dentry);
- error = chown_common(dentry, user, group);
+ error = security_path_chown(&file->f_path, user, group);
+ if (!error)
+ error = chown_common(dentry, user, group);
mnt_drop_write(file->f_path.mnt);
out_fput:
fput(file);
--- security-testing-2.6.orig/include/linux/security.h
+++ security-testing-2.6/include/linux/security.h
@@ -447,6 +447,18 @@ static inline void security_free_mnt_opt
* @new_dir contains the path structure for parent of the new link.
* @new_dentry contains the dentry structure of the new link.
* Return 0 if permission is granted.
+ * @path_chmod:
+ * Check for permission to change DAC's permission of a file or directory.
+ * @dentry contains the dentry structure.
+ * @mnt contains the vfsmnt structure.
+ * @mode contains DAC's mode.
+ * Return 0 if permission is granted.
+ * @path_chown:
+ * Check for permission to change owner/group of a file or directory.
+ * @path contains the path structure.
+ * @uid contains new owner's ID.
+ * @gid contains new group's ID.
+ * Return 0 if permission is granted.
* @inode_readlink:
* Check the permission to read the symbolic link.
* @dentry contains the dentry structure for the file link.
@@ -1488,6 +1500,9 @@ struct security_operations {
struct dentry *new_dentry);
int (*path_rename) (struct path *old_dir, struct dentry *old_dentry,
struct path *new_dir, struct dentry *new_dentry);
+ int (*path_chmod) (struct dentry *dentry, struct vfsmount *mnt,
+ mode_t mode);
+ int (*path_chown) (struct path *path, uid_t uid, gid_t gid);
#endif

int (*inode_alloc_security) (struct inode *inode);
@@ -2952,6 +2967,9 @@ int security_path_link(struct dentry *ol
struct dentry *new_dentry);
int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
struct path *new_dir, struct dentry *new_dentry);
+int security_path_chmod(struct dentry *dentry, struct vfsmount *mnt,
+ mode_t mode);
+int security_path_chown(struct path *path, uid_t uid, gid_t gid);
#else /* CONFIG_SECURITY_PATH */
static inline int security_path_unlink(struct path *dir, struct dentry *dentry)
{
@@ -3001,6 +3019,18 @@ static inline int security_path_rename(s
{
return 0;
}
+
+static inline int security_path_chmod(struct dentry *dentry,
+ struct vfsmount *mnt,
+ mode_t mode)
+{
+ return 0;
+}
+
+static inline int security_path_chown(struct path *path, uid_t uid, gid_t gid)
+{
+ return 0;
+}
#endif /* CONFIG_SECURITY_PATH */

#ifdef CONFIG_KEYS
--- security-testing-2.6.orig/security/capability.c
+++ security-testing-2.6/security/capability.c
@@ -308,6 +308,17 @@ static int cap_path_truncate(struct path
{
return 0;
}
+
+static int cap_path_chmod(struct dentry *dentry, struct vfsmount *mnt,
+ mode_t mode)
+{
+ return 0;
+}
+
+static int cap_path_chown(struct path *path, uid_t uid, gid_t gid)
+{
+ return 0;
+}
#endif

static int cap_file_permission(struct file *file, int mask)
@@ -977,6 +988,8 @@ void security_fixup_ops(struct security_
set_to_cap_if_null(ops, path_link);
set_to_cap_if_null(ops, path_rename);
set_to_cap_if_null(ops, path_truncate);
+ set_to_cap_if_null(ops, path_chmod);
+ set_to_cap_if_null(ops, path_chown);
#endif
set_to_cap_if_null(ops, file_permission);
set_to_cap_if_null(ops, file_alloc_security);
--- security-testing-2.6.orig/security/security.c
+++ security-testing-2.6/security/security.c
@@ -434,6 +434,21 @@ int security_path_truncate(struct path *
return 0;
return security_ops->path_truncate(path, length, time_attrs);
}
+
+int security_path_chmod(struct dentry *dentry, struct vfsmount *mnt,
+ mode_t mode)
+{
+ if (unlikely(IS_PRIVATE(dentry->d_inode)))
+ return 0;
+ return security_ops->path_chmod(dentry, mnt, mode);
+}
+
+int security_path_chown(struct path *path, uid_t uid, gid_t gid)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_chown(path, uid, gid);
+}
#endif

int security_inode_create(struct inode *dir, struct dentry *dentry, int mode)

--


2009-10-08 17:11:06

by John Johansen

[permalink] [raw]
Subject: Re: [TOMOYO #16 01/25] LSM: Add security_path_chmod() and security_path_chown().

Tetsuo Handa wrote:
> This patch allows pathname based LSM modules to check chmod()/chown()
> operations. Since notify_change() does not receive "struct vfsmount *",
> we add security_path_chmod() and security_path_chown() to the caller of
> notify_change().
>
> These hooks are used by TOMOYO.

This hooks would be useful for AppArmor as well.

2009-10-12 01:05:25

by James Morris

[permalink] [raw]
Subject: Re: [TOMOYO #16 01/25] LSM: Add security_path_chmod() and security_path_chown().

On Thu, 8 Oct 2009, John Johansen wrote:

> Tetsuo Handa wrote:
> > This patch allows pathname based LSM modules to check chmod()/chown()
> > operations. Since notify_change() does not receive "struct vfsmount *",
> > we add security_path_chmod() and security_path_chown() to the caller of
> > notify_change().
> >
> > These hooks are used by TOMOYO.
>
> This hooks would be useful for AppArmor as well.


I've applied the first three patches to

git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next

It would be good to see some more evidence of review for the remaining
patches.


--
James Morris
<[email protected]>

2009-10-13 11:35:23

by Tetsuo Handa

[permalink] [raw]
Subject: Re: [TOMOYO #16 01/25] LSM: Add security_path_chmod() andsecurity_path_chown().

James Morris wrote:
> On Thu, 8 Oct 2009, John Johansen wrote:
>
> > Tetsuo Handa wrote:
> > > This patch allows pathname based LSM modules to check chmod()/chown()
> > > operations. Since notify_change() does not receive "struct vfsmount *",
> > > we add security_path_chmod() and security_path_chown() to the caller of
> > > notify_change().
> > >
> > > These hooks are used by TOMOYO.
> >
> > This hooks would be useful for AppArmor as well.
>
>
> I've applied the first three patches to
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
>
Thank you.

> It would be good to see some more evidence of review for the remaining
> patches.
Yes. I'm ready to submit first three patches.

2009-10-13 11:38:17

by Tetsuo Handa

[permalink] [raw]
Subject: [PATCH] TOMOYO: Add recursive directory matching operator support.

[PATCH] TOMOYO: Add recursive directory matching operator support.

This patch introduces new operator /\{dir\}/ which matches
'/' + 'One or more repetitions of dir/' (e.g. /dir/ /dir/dir/ /dir/dir/dir/ ).

Signed-off-by: Tetsuo Handa <[email protected]>
---
security/tomoyo/common.c | 200 ++++++++++++++++++++++++++++-------------------
security/tomoyo/common.h | 4
2 files changed, 121 insertions(+), 83 deletions(-)

--- security-testing-2.6.orig/security/tomoyo/common.c
+++ security-testing-2.6/security/tomoyo/common.c
@@ -187,6 +187,8 @@ bool tomoyo_is_correct_path(const char *
const s8 pattern_type, const s8 end_type,
const char *function)
{
+ const char *const start = filename;
+ bool in_repetition = false;
bool contains_pattern = false;
unsigned char c;
unsigned char d;
@@ -212,9 +214,13 @@ bool tomoyo_is_correct_path(const char *
if (c == '/')
goto out;
}
- while ((c = *filename++) != '\0') {
+ while (1) {
+ c = *filename++;
+ if (!c)
+ break;
if (c == '\\') {
- switch ((c = *filename++)) {
+ c = *filename++;
+ switch (c) {
case '\\': /* "\\" */
continue;
case '$': /* "\$" */
@@ -231,6 +237,22 @@ bool tomoyo_is_correct_path(const char *
break; /* Must not contain pattern */
contains_pattern = true;
continue;
+ case '{': /* "/\{" */
+ if (filename - 3 < start ||
+ *(filename - 3) != '/')
+ break;
+ if (pattern_type == -1)
+ break; /* Must not contain pattern */
+ contains_pattern = true;
+ in_repetition = true;
+ continue;
+ case '}': /* "\}/" */
+ if (*filename != '/')
+ break;
+ if (!in_repetition)
+ break;
+ in_repetition = false;
+ continue;
case '0': /* "\ooo" */
case '1':
case '2':
@@ -246,6 +268,8 @@ bool tomoyo_is_correct_path(const char *
continue; /* pattern is not \000 */
}
goto out;
+ } else if (in_repetition && c == '/') {
+ goto out;
} else if (tomoyo_is_invalid(c)) {
goto out;
}
@@ -254,6 +278,8 @@ bool tomoyo_is_correct_path(const char *
if (!contains_pattern)
goto out;
}
+ if (in_repetition)
+ goto out;
return true;
out:
printk(KERN_DEBUG "%s: Invalid pathname '%s'\n", function,
@@ -360,33 +386,6 @@ struct tomoyo_domain_info *tomoyo_find_d
}

/**
- * tomoyo_path_depth - Evaluate the number of '/' in a string.
- *
- * @pathname: The string to evaluate.
- *
- * Returns path depth of the string.
- *
- * I score 2 for each of the '/' in the @pathname
- * and score 1 if the @pathname ends with '/'.
- */
-static int tomoyo_path_depth(const char *pathname)
-{
- int i = 0;
-
- if (pathname) {
- const char *ep = pathname + strlen(pathname);
- if (pathname < ep--) {
- if (*ep != '/')
- i++;
- while (pathname <= ep)
- if (*ep-- == '/')
- i += 2;
- }
- }
- return i;
-}
-
-/**
* tomoyo_const_part_length - Evaluate the initial length without a pattern in a token.
*
* @filename: The string to evaluate.
@@ -444,11 +443,10 @@ void tomoyo_fill_path_info(struct tomoyo
ptr->is_dir = len && (name[len - 1] == '/');
ptr->is_patterned = (ptr->const_len < len);
ptr->hash = full_name_hash(name, len);
- ptr->depth = tomoyo_path_depth(name);
}

/**
- * tomoyo_file_matches_to_pattern2 - Pattern matching without '/' character
+ * tomoyo_file_matches_pattern2 - Pattern matching without '/' character
* and "\-" pattern.
*
* @filename: The start of string to check.
@@ -458,10 +456,10 @@ void tomoyo_fill_path_info(struct tomoyo
*
* Returns true if @filename matches @pattern, false otherwise.
*/
-static bool tomoyo_file_matches_to_pattern2(const char *filename,
- const char *filename_end,
- const char *pattern,
- const char *pattern_end)
+static bool tomoyo_file_matches_pattern2(const char *filename,
+ const char *filename_end,
+ const char *pattern,
+ const char *pattern_end)
{
while (filename < filename_end && pattern < pattern_end) {
char c;
@@ -519,7 +517,7 @@ static bool tomoyo_file_matches_to_patte
case '*':
case '@':
for (i = 0; i <= filename_end - filename; i++) {
- if (tomoyo_file_matches_to_pattern2(
+ if (tomoyo_file_matches_pattern2(
filename + i, filename_end,
pattern + 1, pattern_end))
return true;
@@ -550,7 +548,7 @@ static bool tomoyo_file_matches_to_patte
j++;
}
for (i = 1; i <= j; i++) {
- if (tomoyo_file_matches_to_pattern2(
+ if (tomoyo_file_matches_pattern2(
filename + i, filename_end,
pattern + 1, pattern_end))
return true;
@@ -567,7 +565,7 @@ static bool tomoyo_file_matches_to_patte
}

/**
- * tomoyo_file_matches_to_pattern - Pattern matching without without '/' character.
+ * tomoyo_file_matches_pattern - Pattern matching without without '/' character.
*
* @filename: The start of string to check.
* @filename_end: The end of string to check.
@@ -576,7 +574,7 @@ static bool tomoyo_file_matches_to_patte
*
* Returns true if @filename matches @pattern, false otherwise.
*/
-static bool tomoyo_file_matches_to_pattern(const char *filename,
+static bool tomoyo_file_matches_pattern(const char *filename,
const char *filename_end,
const char *pattern,
const char *pattern_end)
@@ -589,10 +587,10 @@ static bool tomoyo_file_matches_to_patte
/* Split at "\-" pattern. */
if (*pattern++ != '\\' || *pattern++ != '-')
continue;
- result = tomoyo_file_matches_to_pattern2(filename,
- filename_end,
- pattern_start,
- pattern - 2);
+ result = tomoyo_file_matches_pattern2(filename,
+ filename_end,
+ pattern_start,
+ pattern - 2);
if (first)
result = !result;
if (result)
@@ -600,13 +598,79 @@ static bool tomoyo_file_matches_to_patte
first = false;
pattern_start = pattern;
}
- result = tomoyo_file_matches_to_pattern2(filename, filename_end,
- pattern_start, pattern_end);
+ result = tomoyo_file_matches_pattern2(filename, filename_end,
+ pattern_start, pattern_end);
return first ? result : !result;
}

/**
+ * tomoyo_path_matches_pattern2 - Do pathname pattern matching.
+ *
+ * @f: The start of string to check.
+ * @p: The start of pattern to compare.
+ *
+ * Returns true if @f matches @p, false otherwise.
+ */
+static bool tomoyo_path_matches_pattern2(const char *f, const char *p)
+{
+ const char *f_delimiter;
+ const char *p_delimiter;
+
+ while (*f && *p) {
+ f_delimiter = strchr(f, '/');
+ if (!f_delimiter)
+ f_delimiter = f + strlen(f);
+ p_delimiter = strchr(p, '/');
+ if (!p_delimiter)
+ p_delimiter = p + strlen(p);
+ if (*p == '\\' && *(p + 1) == '{')
+ goto recursive;
+ if (!tomoyo_file_matches_pattern(f, f_delimiter, p,
+ p_delimiter))
+ return false;
+ f = f_delimiter;
+ if (*f)
+ f++;
+ p = p_delimiter;
+ if (*p)
+ p++;
+ }
+ /* Ignore trailing "\*" and "\@" in @pattern. */
+ while (*p == '\\' &&
+ (*(p + 1) == '*' || *(p + 1) == '@'))
+ p += 2;
+ return !*f && !*p;
+ recursive:
+ /*
+ * The "\{" pattern is permitted only after '/' character.
+ * This guarantees that below "*(p - 1)" is safe.
+ * Also, the "\}" pattern is permitted only before '/' character
+ * so that "\{" + "\}" pair will not break the "\-" operator.
+ */
+ if (*(p - 1) != '/' || p_delimiter <= p + 3 || *p_delimiter != '/' ||
+ *(p_delimiter - 1) != '}' || *(p_delimiter - 2) != '\\')
+ return false; /* Bad pattern. */
+ do {
+ /* Compare current component with pattern. */
+ if (!tomoyo_file_matches_pattern(f, f_delimiter, p + 2,
+ p_delimiter - 2))
+ break;
+ /* Proceed to next component. */
+ f = f_delimiter;
+ if (!*f)
+ break;
+ f++;
+ /* Continue comparison. */
+ if (tomoyo_path_matches_pattern2(f, p_delimiter + 1))
+ return true;
+ f_delimiter = strchr(f, '/');
+ } while (f_delimiter);
+ return false; /* Not matched. */
+}
+
+/**
* tomoyo_path_matches_pattern - Check whether the given filename matches the given pattern.
+ *
* @filename: The filename to check.
* @pattern: The pattern to compare.
*
@@ -615,24 +679,24 @@ static bool tomoyo_file_matches_to_patte
* The following patterns are available.
* \\ \ itself.
* \ooo Octal representation of a byte.
- * \* More than or equals to 0 character other than '/'.
- * \@ More than or equals to 0 character other than '/' or '.'.
+ * \* Zero or more repetitions of characters other than '/'.
+ * \@ Zero or more repetitions of characters other than '/' or '.'.
* \? 1 byte character other than '/'.
- * \$ More than or equals to 1 decimal digit.
+ * \$ One or more repetitions of decimal digits.
* \+ 1 decimal digit.
- * \X More than or equals to 1 hexadecimal digit.
+ * \X One or more repetitions of hexadecimal digits.
* \x 1 hexadecimal digit.
- * \A More than or equals to 1 alphabet character.
+ * \A One or more repetitions of alphabet characters.
* \a 1 alphabet character.
+ *
* \- Subtraction operator.
+ *
+ * /\{dir\}/ '/' + 'One or more repetitions of dir/' (e.g. /dir/ /dir/dir/
+ * /dir/dir/dir/ ).
*/
bool tomoyo_path_matches_pattern(const struct tomoyo_path_info *filename,
const struct tomoyo_path_info *pattern)
{
- /*
- if (!filename || !pattern)
- return false;
- */
const char *f = filename->name;
const char *p = pattern->name;
const int len = pattern->const_len;
@@ -640,37 +704,15 @@ bool tomoyo_path_matches_pattern(const s
/* If @pattern doesn't contain pattern, I can use strcmp(). */
if (!pattern->is_patterned)
return !tomoyo_pathcmp(filename, pattern);
- /* Dont compare if the number of '/' differs. */
- if (filename->depth != pattern->depth)
+ /* Don't compare directory and non-directory. */
+ if (filename->is_dir != pattern->is_dir)
return false;
/* Compare the initial length without patterns. */
if (strncmp(f, p, len))
return false;
f += len;
p += len;
- /* Main loop. Compare each directory component. */
- while (*f && *p) {
- const char *f_delimiter = strchr(f, '/');
- const char *p_delimiter = strchr(p, '/');
- if (!f_delimiter)
- f_delimiter = f + strlen(f);
- if (!p_delimiter)
- p_delimiter = p + strlen(p);
- if (!tomoyo_file_matches_to_pattern(f, f_delimiter,
- p, p_delimiter))
- return false;
- f = f_delimiter;
- if (*f)
- f++;
- p = p_delimiter;
- if (*p)
- p++;
- }
- /* Ignore trailing "\*" and "\@" in @pattern. */
- while (*p == '\\' &&
- (*(p + 1) == '*' || *(p + 1) == '@'))
- p += 2;
- return !*f && !*p;
+ return tomoyo_path_matches_pattern2(f, p);
}

/**
--- security-testing-2.6.orig/security/tomoyo/common.h
+++ security-testing-2.6/security/tomoyo/common.h
@@ -56,9 +56,6 @@ struct tomoyo_page_buffer {
* (5) "is_patterned" is a bool which is true if "name" contains wildcard
* characters, false otherwise. This allows TOMOYO to use "hash" and
* strcmp() for string comparison if "is_patterned" is false.
- * (6) "depth" is calculated using the number of "/" characters in "name".
- * This allows TOMOYO to avoid comparing two pathnames which never match
- * (e.g. whether "/var/www/html/index.html" matches "/tmp/sh-thd-\$").
*/
struct tomoyo_path_info {
const char *name;
@@ -66,7 +63,6 @@ struct tomoyo_path_info {
u16 const_len; /* = tomoyo_const_part_length(name) */
bool is_dir; /* = tomoyo_strendswith(name, "/") */
bool is_patterned; /* = tomoyo_path_contains_pattern(name) */
- u16 depth; /* = tomoyo_path_depth(name) */
};

/*

2009-10-13 11:39:59

by Tetsuo Handa

[permalink] [raw]
Subject: [PATCH] TOMOYO: Use RCU primitives for list operation

[PATCH] TOMOYO: Use RCU primitives for list operation

Remove down_read()/up_read() by replacing with RCU primitives.
SRCU based garbage collector will be added in the future.

Signed-off-by: Tetsuo Handa <[email protected]>
---
security/tomoyo/common.c | 52 ++++++++++-----------------------------------
security/tomoyo/common.h | 14 ++++++------
security/tomoyo/domain.c | 38 ++++++++++----------------------
security/tomoyo/file.c | 50 ++++++++++++++-----------------------------
security/tomoyo/realpath.c | 4 ---
5 files changed, 49 insertions(+), 109 deletions(-)

--- security-testing-2.6.orig/security/tomoyo/common.c
+++ security-testing-2.6/security/tomoyo/common.c
@@ -365,9 +365,6 @@ bool tomoyo_is_domain_def(const unsigned
*
* @domainname: The domainname to find.
*
- * Caller must call down_read(&tomoyo_domain_list_lock); or
- * down_write(&tomoyo_domain_list_lock); .
- *
* Returns pointer to "struct tomoyo_domain_info" if found, NULL otherwise.
*/
struct tomoyo_domain_info *tomoyo_find_domain(const char *domainname)
@@ -377,7 +374,7 @@ struct tomoyo_domain_info *tomoyo_find_d

name.name = domainname;
tomoyo_fill_path_info(&name);
- list_for_each_entry(domain, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
if (!domain->is_deleted &&
!tomoyo_pathcmp(&name, domain->domainname))
return domain;
@@ -837,8 +834,7 @@ bool tomoyo_domain_quota_is_ok(struct to

if (!domain)
return true;
- down_read(&tomoyo_domain_acl_info_list_lock);
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
if (ptr->type & TOMOYO_ACL_DELETED)
continue;
switch (tomoyo_acl_type2(ptr)) {
@@ -891,7 +887,6 @@ bool tomoyo_domain_quota_is_ok(struct to
break;
}
}
- up_read(&tomoyo_domain_acl_info_list_lock);
if (count < tomoyo_check_flags(domain, TOMOYO_MAX_ACCEPT_ENTRY))
return true;
if (!domain->quota_warned) {
@@ -1143,7 +1138,7 @@ static int tomoyo_update_manager_entry(c
if (!saved_manager)
return -ENOMEM;
down_write(&tomoyo_policy_manager_list_lock);
- list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_policy_manager_list, list) {
if (ptr->manager != saved_manager)
continue;
ptr->is_deleted = is_delete;
@@ -1159,7 +1154,7 @@ static int tomoyo_update_manager_entry(c
goto out;
new_entry->manager = saved_manager;
new_entry->is_domain = is_domain;
- list_add_tail(&new_entry->list, &tomoyo_policy_manager_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_policy_manager_list);
error = 0;
out:
up_write(&tomoyo_policy_manager_list_lock);
@@ -1199,7 +1194,6 @@ static int tomoyo_read_manager_policy(st

if (head->read_eof)
return 0;
- down_read(&tomoyo_policy_manager_list_lock);
list_for_each_cookie(pos, head->read_var2,
&tomoyo_policy_manager_list) {
struct tomoyo_policy_manager_entry *ptr;
@@ -1211,7 +1205,6 @@ static int tomoyo_read_manager_policy(st
if (!done)
break;
}
- up_read(&tomoyo_policy_manager_list_lock);
head->read_eof = done;
return 0;
}
@@ -1234,29 +1227,25 @@ static bool tomoyo_is_policy_manager(voi
return true;
if (!tomoyo_manage_by_non_root && (task->cred->uid || task->cred->euid))
return false;
- down_read(&tomoyo_policy_manager_list_lock);
- list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_policy_manager_list, list) {
if (!ptr->is_deleted && ptr->is_domain
&& !tomoyo_pathcmp(domainname, ptr->manager)) {
found = true;
break;
}
}
- up_read(&tomoyo_policy_manager_list_lock);
if (found)
return true;
exe = tomoyo_get_exe();
if (!exe)
return false;
- down_read(&tomoyo_policy_manager_list_lock);
- list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_policy_manager_list, list) {
if (!ptr->is_deleted && !ptr->is_domain
&& !strcmp(exe, ptr->manager->name)) {
found = true;
break;
}
}
- up_read(&tomoyo_policy_manager_list_lock);
if (!found) { /* Reduce error messages. */
static pid_t last_pid;
const pid_t pid = current->pid;
@@ -1292,11 +1281,8 @@ static bool tomoyo_is_select_one(struct
domain = tomoyo_real_domain(p);
read_unlock(&tasklist_lock);
} else if (!strncmp(data, "domain=", 7)) {
- if (tomoyo_is_domain_def(data + 7)) {
- down_read(&tomoyo_domain_list_lock);
+ if (tomoyo_is_domain_def(data + 7))
domain = tomoyo_find_domain(data + 7);
- up_read(&tomoyo_domain_list_lock);
- }
} else
return false;
head->write_var1 = domain;
@@ -1310,13 +1296,11 @@ static bool tomoyo_is_select_one(struct
if (domain) {
struct tomoyo_domain_info *d;
head->read_var1 = NULL;
- down_read(&tomoyo_domain_list_lock);
- list_for_each_entry(d, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(d, &tomoyo_domain_list, list) {
if (d == domain)
break;
head->read_var1 = &d->list;
}
- up_read(&tomoyo_domain_list_lock);
head->read_var2 = NULL;
head->read_bit = 0;
head->read_step = 0;
@@ -1342,7 +1326,7 @@ static int tomoyo_delete_domain(char *do
tomoyo_fill_path_info(&name);
down_write(&tomoyo_domain_list_lock);
/* Is there an active domain? */
- list_for_each_entry(domain, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
/* Never delete tomoyo_kernel_domain */
if (domain == &tomoyo_kernel_domain)
continue;
@@ -1384,11 +1368,9 @@ static int tomoyo_write_domain_policy(st
domain = NULL;
if (is_delete)
tomoyo_delete_domain(data);
- else if (is_select) {
- down_read(&tomoyo_domain_list_lock);
+ else if (is_select)
domain = tomoyo_find_domain(data);
- up_read(&tomoyo_domain_list_lock);
- } else
+ else
domain = tomoyo_find_or_assign_new_domain(data, 0);
head->write_var1 = domain;
return 0;
@@ -1544,7 +1526,6 @@ static int tomoyo_read_domain_policy(str
return 0;
if (head->read_step == 0)
head->read_step = 1;
- down_read(&tomoyo_domain_list_lock);
list_for_each_cookie(dpos, head->read_var1, &tomoyo_domain_list) {
struct tomoyo_domain_info *domain;
const char *quota_exceeded = "";
@@ -1577,7 +1558,6 @@ acl_loop:
if (head->read_step == 3)
goto tail_mark;
/* Print ACL entries in the domain. */
- down_read(&tomoyo_domain_acl_info_list_lock);
list_for_each_cookie(apos, head->read_var2,
&domain->acl_info_list) {
struct tomoyo_acl_info *ptr
@@ -1587,7 +1567,6 @@ acl_loop:
if (!done)
break;
}
- up_read(&tomoyo_domain_acl_info_list_lock);
if (!done)
break;
head->read_step = 3;
@@ -1599,7 +1578,6 @@ tail_mark:
if (head->read_single_domain)
break;
}
- up_read(&tomoyo_domain_list_lock);
head->read_eof = done;
return 0;
}
@@ -1626,9 +1604,7 @@ static int tomoyo_write_domain_profile(s
if (!cp)
return -EINVAL;
*cp = '\0';
- down_read(&tomoyo_domain_list_lock);
domain = tomoyo_find_domain(cp + 1);
- up_read(&tomoyo_domain_list_lock);
if (strict_strtoul(data, 10, &profile))
return -EINVAL;
if (domain && profile < TOMOYO_MAX_PROFILES
@@ -1658,7 +1634,6 @@ static int tomoyo_read_domain_profile(st

if (head->read_eof)
return 0;
- down_read(&tomoyo_domain_list_lock);
list_for_each_cookie(pos, head->read_var1, &tomoyo_domain_list) {
struct tomoyo_domain_info *domain;
domain = list_entry(pos, struct tomoyo_domain_info, list);
@@ -1669,7 +1644,6 @@ static int tomoyo_read_domain_profile(st
if (!done)
break;
}
- up_read(&tomoyo_domain_list_lock);
head->read_eof = done;
return 0;
}
@@ -1889,15 +1863,13 @@ void tomoyo_load_policy(const char *file
tomoyo_policy_loaded = true;
{ /* Check all profiles currently assigned to domains are defined. */
struct tomoyo_domain_info *domain;
- down_read(&tomoyo_domain_list_lock);
- list_for_each_entry(domain, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
const u8 profile = domain->profile;
if (tomoyo_profile_ptr[profile])
continue;
panic("Profile %u (used by '%s') not defined.\n",
profile, domain->domainname->name);
}
- up_read(&tomoyo_domain_list_lock);
}
}

--- security-testing-2.6.orig/security/tomoyo/common.h
+++ security-testing-2.6/security/tomoyo/common.h
@@ -442,16 +442,16 @@ extern struct tomoyo_domain_info tomoyo_
* @cookie: the &struct list_head to use as a cookie.
* @head: the head for your list.
*
- * Same with list_for_each() except that this primitive uses @cookie
+ * Same with list_for_each_rcu() except that this primitive uses @cookie
* so that we can continue iteration.
* @cookie must be NULL when iteration starts, and @cookie will become
* NULL when iteration finishes.
*/
-#define list_for_each_cookie(pos, cookie, head) \
- for (({ if (!cookie) \
- cookie = head; }), \
- pos = (cookie)->next; \
- prefetch(pos->next), pos != (head) || ((cookie) = NULL); \
- (cookie) = pos, pos = pos->next)
+#define list_for_each_cookie(pos, cookie, head) \
+ for (({ if (!cookie) \
+ cookie = head; }), \
+ pos = rcu_dereference((cookie)->next); \
+ prefetch(pos->next), pos != (head) || ((cookie) = NULL); \
+ (cookie) = pos, pos = rcu_dereference(pos->next))

#endif /* !defined(_SECURITY_TOMOYO_COMMON_H) */
--- security-testing-2.6.orig/security/tomoyo/domain.c
+++ security-testing-2.6/security/tomoyo/domain.c
@@ -246,7 +246,7 @@ static int tomoyo_update_domain_initiali
if (!saved_program)
return -ENOMEM;
down_write(&tomoyo_domain_initializer_list_lock);
- list_for_each_entry(ptr, &tomoyo_domain_initializer_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_domain_initializer_list, list) {
if (ptr->is_not != is_not ||
ptr->domainname != saved_domainname ||
ptr->program != saved_program)
@@ -266,7 +266,7 @@ static int tomoyo_update_domain_initiali
new_entry->program = saved_program;
new_entry->is_not = is_not;
new_entry->is_last_name = is_last_name;
- list_add_tail(&new_entry->list, &tomoyo_domain_initializer_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_domain_initializer_list);
error = 0;
out:
up_write(&tomoyo_domain_initializer_list_lock);
@@ -285,7 +285,6 @@ bool tomoyo_read_domain_initializer_poli
struct list_head *pos;
bool done = true;

- down_read(&tomoyo_domain_initializer_list_lock);
list_for_each_cookie(pos, head->read_var2,
&tomoyo_domain_initializer_list) {
const char *no;
@@ -308,7 +307,6 @@ bool tomoyo_read_domain_initializer_poli
if (!done)
break;
}
- up_read(&tomoyo_domain_initializer_list_lock);
return done;
}

@@ -355,8 +353,7 @@ static bool tomoyo_is_domain_initializer
struct tomoyo_domain_initializer_entry *ptr;
bool flag = false;

- down_read(&tomoyo_domain_initializer_list_lock);
- list_for_each_entry(ptr, &tomoyo_domain_initializer_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_domain_initializer_list, list) {
if (ptr->is_deleted)
continue;
if (ptr->domainname) {
@@ -376,7 +373,6 @@ static bool tomoyo_is_domain_initializer
}
flag = true;
}
- up_read(&tomoyo_domain_initializer_list_lock);
return flag;
}

@@ -459,7 +455,7 @@ static int tomoyo_update_domain_keeper_e
if (!saved_domainname)
return -ENOMEM;
down_write(&tomoyo_domain_keeper_list_lock);
- list_for_each_entry(ptr, &tomoyo_domain_keeper_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_domain_keeper_list, list) {
if (ptr->is_not != is_not ||
ptr->domainname != saved_domainname ||
ptr->program != saved_program)
@@ -479,7 +475,7 @@ static int tomoyo_update_domain_keeper_e
new_entry->program = saved_program;
new_entry->is_not = is_not;
new_entry->is_last_name = is_last_name;
- list_add_tail(&new_entry->list, &tomoyo_domain_keeper_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_domain_keeper_list);
error = 0;
out:
up_write(&tomoyo_domain_keeper_list_lock);
@@ -519,7 +515,6 @@ bool tomoyo_read_domain_keeper_policy(st
struct list_head *pos;
bool done = true;

- down_read(&tomoyo_domain_keeper_list_lock);
list_for_each_cookie(pos, head->read_var2,
&tomoyo_domain_keeper_list) {
struct tomoyo_domain_keeper_entry *ptr;
@@ -542,7 +537,6 @@ bool tomoyo_read_domain_keeper_policy(st
if (!done)
break;
}
- up_read(&tomoyo_domain_keeper_list_lock);
return done;
}

@@ -563,8 +557,7 @@ static bool tomoyo_is_domain_keeper(cons
struct tomoyo_domain_keeper_entry *ptr;
bool flag = false;

- down_read(&tomoyo_domain_keeper_list_lock);
- list_for_each_entry(ptr, &tomoyo_domain_keeper_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_domain_keeper_list, list) {
if (ptr->is_deleted)
continue;
if (!ptr->is_last_name) {
@@ -582,7 +575,6 @@ static bool tomoyo_is_domain_keeper(cons
}
flag = true;
}
- up_read(&tomoyo_domain_keeper_list_lock);
return flag;
}

@@ -646,7 +638,7 @@ static int tomoyo_update_alias_entry(con
if (!saved_original_name || !saved_aliased_name)
return -ENOMEM;
down_write(&tomoyo_alias_list_lock);
- list_for_each_entry(ptr, &tomoyo_alias_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_alias_list, list) {
if (ptr->original_name != saved_original_name ||
ptr->aliased_name != saved_aliased_name)
continue;
@@ -663,7 +655,7 @@ static int tomoyo_update_alias_entry(con
goto out;
new_entry->original_name = saved_original_name;
new_entry->aliased_name = saved_aliased_name;
- list_add_tail(&new_entry->list, &tomoyo_alias_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_alias_list);
error = 0;
out:
up_write(&tomoyo_alias_list_lock);
@@ -682,7 +674,6 @@ bool tomoyo_read_alias_policy(struct tom
struct list_head *pos;
bool done = true;

- down_read(&tomoyo_alias_list_lock);
list_for_each_cookie(pos, head->read_var2, &tomoyo_alias_list) {
struct tomoyo_alias_entry *ptr;

@@ -695,7 +686,6 @@ bool tomoyo_read_alias_policy(struct tom
if (!done)
break;
}
- up_read(&tomoyo_alias_list_lock);
return done;
}

@@ -742,7 +732,7 @@ struct tomoyo_domain_info *tomoyo_find_o
if (!saved_domainname)
goto out;
/* Can I reuse memory of deleted domain? */
- list_for_each_entry(domain, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
struct task_struct *p;
struct tomoyo_acl_info *ptr;
bool flag;
@@ -760,7 +750,7 @@ struct tomoyo_domain_info *tomoyo_find_o
read_unlock(&tasklist_lock);
if (flag)
continue;
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
ptr->type |= TOMOYO_ACL_DELETED;
}
tomoyo_set_domain_flag(domain, true, domain->flags);
@@ -776,7 +766,7 @@ struct tomoyo_domain_info *tomoyo_find_o
INIT_LIST_HEAD(&domain->acl_info_list);
domain->domainname = saved_domainname;
domain->profile = profile;
- list_add_tail(&domain->list, &tomoyo_domain_list);
+ list_add_tail_rcu(&domain->list, &tomoyo_domain_list);
}
out:
up_write(&tomoyo_domain_list_lock);
@@ -849,8 +839,7 @@ int tomoyo_find_next_domain(struct linux
if (tomoyo_pathcmp(&r, &s)) {
struct tomoyo_alias_entry *ptr;
/* Is this program allowed to be called via symbolic links? */
- down_read(&tomoyo_alias_list_lock);
- list_for_each_entry(ptr, &tomoyo_alias_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_alias_list, list) {
if (ptr->is_deleted ||
tomoyo_pathcmp(&r, ptr->original_name) ||
tomoyo_pathcmp(&s, ptr->aliased_name))
@@ -861,7 +850,6 @@ int tomoyo_find_next_domain(struct linux
tomoyo_fill_path_info(&r);
break;
}
- up_read(&tomoyo_alias_list_lock);
}

/* Check execute permission. */
@@ -892,9 +880,7 @@ int tomoyo_find_next_domain(struct linux
}
if (domain || strlen(new_domain_name) >= TOMOYO_MAX_PATHNAME_LEN)
goto done;
- down_read(&tomoyo_domain_list_lock);
domain = tomoyo_find_domain(new_domain_name);
- up_read(&tomoyo_domain_list_lock);
if (domain)
goto done;
if (is_enforce)
--- security-testing-2.6.orig/security/tomoyo/file.c
+++ security-testing-2.6/security/tomoyo/file.c
@@ -220,7 +220,7 @@ static int tomoyo_update_globally_readab
if (!saved_filename)
return -ENOMEM;
down_write(&tomoyo_globally_readable_list_lock);
- list_for_each_entry(ptr, &tomoyo_globally_readable_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_globally_readable_list, list) {
if (ptr->filename != saved_filename)
continue;
ptr->is_deleted = is_delete;
@@ -235,7 +235,7 @@ static int tomoyo_update_globally_readab
if (!new_entry)
goto out;
new_entry->filename = saved_filename;
- list_add_tail(&new_entry->list, &tomoyo_globally_readable_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_globally_readable_list);
error = 0;
out:
up_write(&tomoyo_globally_readable_list_lock);
@@ -254,15 +254,13 @@ static bool tomoyo_is_globally_readable_
{
struct tomoyo_globally_readable_file_entry *ptr;
bool found = false;
- down_read(&tomoyo_globally_readable_list_lock);
- list_for_each_entry(ptr, &tomoyo_globally_readable_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_globally_readable_list, list) {
if (!ptr->is_deleted &&
tomoyo_path_matches_pattern(filename, ptr->filename)) {
found = true;
break;
}
}
- up_read(&tomoyo_globally_readable_list_lock);
return found;
}

@@ -291,7 +289,6 @@ bool tomoyo_read_globally_readable_polic
struct list_head *pos;
bool done = true;

- down_read(&tomoyo_globally_readable_list_lock);
list_for_each_cookie(pos, head->read_var2,
&tomoyo_globally_readable_list) {
struct tomoyo_globally_readable_file_entry *ptr;
@@ -305,7 +302,6 @@ bool tomoyo_read_globally_readable_polic
if (!done)
break;
}
- up_read(&tomoyo_globally_readable_list_lock);
return done;
}

@@ -363,7 +359,7 @@ static int tomoyo_update_file_pattern_en
if (!saved_pattern)
return -ENOMEM;
down_write(&tomoyo_pattern_list_lock);
- list_for_each_entry(ptr, &tomoyo_pattern_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_pattern_list, list) {
if (saved_pattern != ptr->pattern)
continue;
ptr->is_deleted = is_delete;
@@ -378,7 +374,7 @@ static int tomoyo_update_file_pattern_en
if (!new_entry)
goto out;
new_entry->pattern = saved_pattern;
- list_add_tail(&new_entry->list, &tomoyo_pattern_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_pattern_list);
error = 0;
out:
up_write(&tomoyo_pattern_list_lock);
@@ -398,8 +394,7 @@ tomoyo_get_file_pattern(const struct tom
struct tomoyo_pattern_entry *ptr;
const struct tomoyo_path_info *pattern = NULL;

- down_read(&tomoyo_pattern_list_lock);
- list_for_each_entry(ptr, &tomoyo_pattern_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_pattern_list, list) {
if (ptr->is_deleted)
continue;
if (!tomoyo_path_matches_pattern(filename, ptr->pattern))
@@ -412,7 +407,6 @@ tomoyo_get_file_pattern(const struct tom
break;
}
}
- up_read(&tomoyo_pattern_list_lock);
if (pattern)
filename = pattern;
return filename;
@@ -443,7 +437,6 @@ bool tomoyo_read_file_pattern(struct tom
struct list_head *pos;
bool done = true;

- down_read(&tomoyo_pattern_list_lock);
list_for_each_cookie(pos, head->read_var2, &tomoyo_pattern_list) {
struct tomoyo_pattern_entry *ptr;
ptr = list_entry(pos, struct tomoyo_pattern_entry, list);
@@ -454,7 +447,6 @@ bool tomoyo_read_file_pattern(struct tom
if (!done)
break;
}
- up_read(&tomoyo_pattern_list_lock);
return done;
}

@@ -511,7 +503,7 @@ static int tomoyo_update_no_rewrite_entr
if (!saved_pattern)
return -ENOMEM;
down_write(&tomoyo_no_rewrite_list_lock);
- list_for_each_entry(ptr, &tomoyo_no_rewrite_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_no_rewrite_list, list) {
if (ptr->pattern != saved_pattern)
continue;
ptr->is_deleted = is_delete;
@@ -526,7 +518,7 @@ static int tomoyo_update_no_rewrite_entr
if (!new_entry)
goto out;
new_entry->pattern = saved_pattern;
- list_add_tail(&new_entry->list, &tomoyo_no_rewrite_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_no_rewrite_list);
error = 0;
out:
up_write(&tomoyo_no_rewrite_list_lock);
@@ -546,8 +538,7 @@ static bool tomoyo_is_no_rewrite_file(co
struct tomoyo_no_rewrite_entry *ptr;
bool found = false;

- down_read(&tomoyo_no_rewrite_list_lock);
- list_for_each_entry(ptr, &tomoyo_no_rewrite_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_no_rewrite_list, list) {
if (ptr->is_deleted)
continue;
if (!tomoyo_path_matches_pattern(filename, ptr->pattern))
@@ -555,7 +546,6 @@ static bool tomoyo_is_no_rewrite_file(co
found = true;
break;
}
- up_read(&tomoyo_no_rewrite_list_lock);
return found;
}

@@ -584,7 +574,6 @@ bool tomoyo_read_no_rewrite_policy(struc
struct list_head *pos;
bool done = true;

- down_read(&tomoyo_no_rewrite_list_lock);
list_for_each_cookie(pos, head->read_var2, &tomoyo_no_rewrite_list) {
struct tomoyo_no_rewrite_entry *ptr;
ptr = list_entry(pos, struct tomoyo_no_rewrite_entry, list);
@@ -595,7 +584,6 @@ bool tomoyo_read_no_rewrite_policy(struc
if (!done)
break;
}
- up_read(&tomoyo_no_rewrite_list_lock);
return done;
}

@@ -661,8 +649,7 @@ static int tomoyo_check_single_path_acl2
struct tomoyo_acl_info *ptr;
int error = -EPERM;

- down_read(&tomoyo_domain_acl_info_list_lock);
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
struct tomoyo_single_path_acl_record *acl;
if (tomoyo_acl_type2(ptr) != TOMOYO_TYPE_SINGLE_PATH_ACL)
continue;
@@ -680,7 +667,6 @@ static int tomoyo_check_single_path_acl2
error = 0;
break;
}
- up_read(&tomoyo_domain_acl_info_list_lock);
return error;
}

@@ -848,7 +834,7 @@ static int tomoyo_update_single_path_acl
down_write(&tomoyo_domain_acl_info_list_lock);
if (is_delete)
goto delete;
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
if (tomoyo_acl_type1(ptr) != TOMOYO_TYPE_SINGLE_PATH_ACL)
continue;
acl = container_of(ptr, struct tomoyo_single_path_acl_record,
@@ -875,12 +861,12 @@ static int tomoyo_update_single_path_acl
if (perm == (1 << TOMOYO_TYPE_READ_WRITE_ACL))
acl->perm |= rw_mask;
acl->filename = saved_filename;
- list_add_tail(&acl->head.list, &domain->acl_info_list);
+ list_add_tail_rcu(&acl->head.list, &domain->acl_info_list);
error = 0;
goto out;
delete:
error = -ENOENT;
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
if (tomoyo_acl_type2(ptr) != TOMOYO_TYPE_SINGLE_PATH_ACL)
continue;
acl = container_of(ptr, struct tomoyo_single_path_acl_record,
@@ -937,7 +923,7 @@ static int tomoyo_update_double_path_acl
down_write(&tomoyo_domain_acl_info_list_lock);
if (is_delete)
goto delete;
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
if (tomoyo_acl_type1(ptr) != TOMOYO_TYPE_DOUBLE_PATH_ACL)
continue;
acl = container_of(ptr, struct tomoyo_double_path_acl_record,
@@ -960,12 +946,12 @@ static int tomoyo_update_double_path_acl
acl->perm = perm;
acl->filename1 = saved_filename1;
acl->filename2 = saved_filename2;
- list_add_tail(&acl->head.list, &domain->acl_info_list);
+ list_add_tail_rcu(&acl->head.list, &domain->acl_info_list);
error = 0;
goto out;
delete:
error = -ENOENT;
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
if (tomoyo_acl_type2(ptr) != TOMOYO_TYPE_DOUBLE_PATH_ACL)
continue;
acl = container_of(ptr, struct tomoyo_double_path_acl_record,
@@ -1025,8 +1011,7 @@ static int tomoyo_check_double_path_acl(

if (!tomoyo_check_flags(domain, TOMOYO_MAC_FOR_FILE))
return 0;
- down_read(&tomoyo_domain_acl_info_list_lock);
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
struct tomoyo_double_path_acl_record *acl;
if (tomoyo_acl_type2(ptr) != TOMOYO_TYPE_DOUBLE_PATH_ACL)
continue;
@@ -1041,7 +1026,6 @@ static int tomoyo_check_double_path_acl(
error = 0;
break;
}
- up_read(&tomoyo_domain_acl_info_list_lock);
return error;
}

--- security-testing-2.6.orig/security/tomoyo/realpath.c
+++ security-testing-2.6/security/tomoyo/realpath.c
@@ -387,11 +387,9 @@ void __init tomoyo_realpath_init(void)
INIT_LIST_HEAD(&tomoyo_name_list[i]);
INIT_LIST_HEAD(&tomoyo_kernel_domain.acl_info_list);
tomoyo_kernel_domain.domainname = tomoyo_save_name(TOMOYO_ROOT_NAME);
- list_add_tail(&tomoyo_kernel_domain.list, &tomoyo_domain_list);
- down_read(&tomoyo_domain_list_lock);
+ list_add_tail_rcu(&tomoyo_kernel_domain.list, &tomoyo_domain_list);
if (tomoyo_find_domain(TOMOYO_ROOT_NAME) != &tomoyo_kernel_domain)
panic("Can't register tomoyo_kernel_domain");
- up_read(&tomoyo_domain_list_lock);
}

/* Memory allocated for temporary purpose. */

2009-10-13 11:41:48

by Tetsuo Handa

[permalink] [raw]
Subject: [PATCH] TOMOYO: Bring memory allocation to outside semaphore

[PATCH] TOMOYO: Bring memory allocation to outside semaphore

This patch brings kmalloc(GFP_KERNEL) to outside rw_semaphores.

Since readers no longer need to call down_read()/up_read() and writers no
longer sleep inside down_write()/up_write(), this patch also replaces various
rw_semaphore with single tomoyo_policy_lock mutex.

Signed-off-by: Tetsuo Handa <[email protected]>
---
security/tomoyo/common.c | 87 +++++++++----------------
security/tomoyo/common.h | 11 +--
security/tomoyo/domain.c | 137 ++++++++++++++++-----------------------
security/tomoyo/file.c | 124 ++++++++++++++++++++----------------
security/tomoyo/realpath.c | 155 +++++++++++++++------------------------------
security/tomoyo/realpath.h | 6 -
6 files changed, 216 insertions(+), 304 deletions(-)

--- security-testing-2.6.orig/security/tomoyo/common.c
+++ security-testing-2.6/security/tomoyo/common.c
@@ -908,25 +908,28 @@ bool tomoyo_domain_quota_is_ok(struct to
static struct tomoyo_profile *tomoyo_find_or_assign_new_profile(const unsigned
int profile)
{
- static DEFINE_MUTEX(lock);
- struct tomoyo_profile *ptr = NULL;
- int i;
+ struct tomoyo_profile *ptr;
+ struct tomoyo_profile *entry;

if (profile >= TOMOYO_MAX_PROFILES)
return NULL;
- mutex_lock(&lock);
ptr = tomoyo_profile_ptr[profile];
if (ptr)
- goto ok;
- ptr = tomoyo_alloc_element(sizeof(*ptr));
- if (!ptr)
- goto ok;
- for (i = 0; i < TOMOYO_MAX_CONTROL_INDEX; i++)
- ptr->value[i] = tomoyo_control_array[i].current_value;
- mb(); /* Avoid out-of-order execution. */
- tomoyo_profile_ptr[profile] = ptr;
- ok:
- mutex_unlock(&lock);
+ return ptr;
+ entry = kmalloc(sizeof(*ptr), GFP_KERNEL);
+ mutex_lock(&tomoyo_policy_lock);
+ ptr = tomoyo_profile_ptr[profile];
+ if (!ptr && tomoyo_memory_ok(entry)) {
+ int i;
+ ptr = entry;
+ for (i = 0; i < TOMOYO_MAX_CONTROL_INDEX; i++)
+ ptr->value[i] = tomoyo_control_array[i].current_value;
+ mb(); /* Avoid out-of-order execution. */
+ tomoyo_profile_ptr[profile] = ptr;
+ entry = NULL;
+ }
+ mutex_unlock(&tomoyo_policy_lock);
+ kfree(entry);
return ptr;
}

@@ -1107,7 +1110,6 @@ struct tomoyo_policy_manager_entry {
* # cat /sys/kernel/security/tomoyo/manager
*/
static LIST_HEAD(tomoyo_policy_manager_list);
-static DECLARE_RWSEM(tomoyo_policy_manager_list_lock);

/**
* tomoyo_update_manager_entry - Add a manager entry.
@@ -1120,7 +1122,7 @@ static DECLARE_RWSEM(tomoyo_policy_manag
static int tomoyo_update_manager_entry(const char *manager,
const bool is_delete)
{
- struct tomoyo_policy_manager_entry *new_entry;
+ struct tomoyo_policy_manager_entry *entry;
struct tomoyo_policy_manager_entry *ptr;
const struct tomoyo_path_info *saved_manager;
int error = -ENOMEM;
@@ -1137,7 +1139,8 @@ static int tomoyo_update_manager_entry(c
saved_manager = tomoyo_save_name(manager);
if (!saved_manager)
return -ENOMEM;
- down_write(&tomoyo_policy_manager_list_lock);
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ mutex_lock(&tomoyo_policy_lock);
list_for_each_entry_rcu(ptr, &tomoyo_policy_manager_list, list) {
if (ptr->manager != saved_manager)
continue;
@@ -1149,15 +1152,16 @@ static int tomoyo_update_manager_entry(c
error = -ENOENT;
goto out;
}
- new_entry = tomoyo_alloc_element(sizeof(*new_entry));
- if (!new_entry)
- goto out;
- new_entry->manager = saved_manager;
- new_entry->is_domain = is_domain;
- list_add_tail_rcu(&new_entry->list, &tomoyo_policy_manager_list);
- error = 0;
+ if (tomoyo_memory_ok(entry)) {
+ entry->manager = saved_manager;
+ entry->is_domain = is_domain;
+ list_add_tail_rcu(&entry->list, &tomoyo_policy_manager_list);
+ entry = NULL;
+ error = 0;
+ }
out:
- up_write(&tomoyo_policy_manager_list_lock);
+ mutex_unlock(&tomoyo_policy_lock);
+ kfree(entry);
return error;
}

@@ -1324,7 +1328,7 @@ static int tomoyo_delete_domain(char *do

name.name = domainname;
tomoyo_fill_path_info(&name);
- down_write(&tomoyo_domain_list_lock);
+ mutex_lock(&tomoyo_policy_lock);
/* Is there an active domain? */
list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
/* Never delete tomoyo_kernel_domain */
@@ -1336,7 +1340,7 @@ static int tomoyo_delete_domain(char *do
domain->is_deleted = true;
break;
}
- up_write(&tomoyo_domain_list_lock);
+ mutex_unlock(&tomoyo_policy_lock);
return 0;
}

@@ -2133,35 +2137,6 @@ static int tomoyo_close_control(struct f
}

/**
- * tomoyo_alloc_acl_element - Allocate permanent memory for ACL entry.
- *
- * @acl_type: Type of ACL entry.
- *
- * Returns pointer to the ACL entry on success, NULL otherwise.
- */
-void *tomoyo_alloc_acl_element(const u8 acl_type)
-{
- int len;
- struct tomoyo_acl_info *ptr;
-
- switch (acl_type) {
- case TOMOYO_TYPE_SINGLE_PATH_ACL:
- len = sizeof(struct tomoyo_single_path_acl_record);
- break;
- case TOMOYO_TYPE_DOUBLE_PATH_ACL:
- len = sizeof(struct tomoyo_double_path_acl_record);
- break;
- default:
- return NULL;
- }
- ptr = tomoyo_alloc_element(len);
- if (!ptr)
- return NULL;
- ptr->type = acl_type;
- return ptr;
-}
-
-/**
* tomoyo_open - open() for /sys/kernel/security/tomoyo/ interface.
*
* @inode: Pointer to "struct inode".
--- security-testing-2.6.orig/security/tomoyo/common.h
+++ security-testing-2.6/security/tomoyo/common.h
@@ -307,6 +307,8 @@ bool tomoyo_is_correct_path(const char *
const char *function);
/* Check whether the token can be a domainname. */
bool tomoyo_is_domain_def(const unsigned char *buffer);
+/* Check memory quota. */
+bool tomoyo_memory_ok(void *ptr);
/* Check whether the given filename matches the given pattern. */
bool tomoyo_path_matches_pattern(const struct tomoyo_path_info *filename,
const struct tomoyo_path_info *pattern);
@@ -370,8 +372,6 @@ struct tomoyo_domain_info *tomoyo_find_o
/* Check mode for specified functionality. */
unsigned int tomoyo_check_flags(const struct tomoyo_domain_info *domain,
const u8 index);
-/* Allocate memory for structures. */
-void *tomoyo_alloc_acl_element(const u8 acl_type);
/* Fill in "struct tomoyo_path_info" members. */
void tomoyo_fill_path_info(struct tomoyo_path_info *ptr);
/* Run policy loader when /sbin/init starts. */
@@ -423,12 +423,11 @@ static inline bool tomoyo_is_invalid(con
return c && (c <= ' ' || c >= 127);
}

+/* Lock for modifying policy. */
+extern struct mutex tomoyo_policy_lock;
+
/* The list for "struct tomoyo_domain_info". */
extern struct list_head tomoyo_domain_list;
-extern struct rw_semaphore tomoyo_domain_list_lock;
-
-/* Lock for domain->acl_info_list. */
-extern struct rw_semaphore tomoyo_domain_acl_info_list_lock;

/* Has /sbin/init started? */
extern bool tomoyo_policy_loaded;
--- security-testing-2.6.orig/security/tomoyo/domain.c
+++ security-testing-2.6/security/tomoyo/domain.c
@@ -58,7 +58,8 @@ struct tomoyo_domain_info tomoyo_kernel_
* exceptions.
*/
LIST_HEAD(tomoyo_domain_list);
-DECLARE_RWSEM(tomoyo_domain_list_lock);
+/* Lock for modifying policy. */
+DEFINE_MUTEX(tomoyo_policy_lock);

/*
* tomoyo_domain_initializer_entry is a structure which is used for holding
@@ -206,7 +207,6 @@ const char *tomoyo_get_last_name(const s
* unless executed from "<kernel> /etc/rc.d/init.d/httpd" domain.
*/
static LIST_HEAD(tomoyo_domain_initializer_list);
-static DECLARE_RWSEM(tomoyo_domain_initializer_list_lock);

/**
* tomoyo_update_domain_initializer_entry - Update "struct tomoyo_domain_initializer_entry" list.
@@ -223,7 +223,7 @@ static int tomoyo_update_domain_initiali
const bool is_not,
const bool is_delete)
{
- struct tomoyo_domain_initializer_entry *new_entry;
+ struct tomoyo_domain_initializer_entry *entry = NULL;
struct tomoyo_domain_initializer_entry *ptr;
const struct tomoyo_path_info *saved_program;
const struct tomoyo_path_info *saved_domainname = NULL;
@@ -245,7 +245,9 @@ static int tomoyo_update_domain_initiali
saved_program = tomoyo_save_name(program);
if (!saved_program)
return -ENOMEM;
- down_write(&tomoyo_domain_initializer_list_lock);
+ if (!is_delete)
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ mutex_lock(&tomoyo_policy_lock);
list_for_each_entry_rcu(ptr, &tomoyo_domain_initializer_list, list) {
if (ptr->is_not != is_not ||
ptr->domainname != saved_domainname ||
@@ -259,17 +261,19 @@ static int tomoyo_update_domain_initiali
error = -ENOENT;
goto out;
}
- new_entry = tomoyo_alloc_element(sizeof(*new_entry));
- if (!new_entry)
- goto out;
- new_entry->domainname = saved_domainname;
- new_entry->program = saved_program;
- new_entry->is_not = is_not;
- new_entry->is_last_name = is_last_name;
- list_add_tail_rcu(&new_entry->list, &tomoyo_domain_initializer_list);
- error = 0;
+ if (tomoyo_memory_ok(entry)) {
+ entry->domainname = saved_domainname;
+ entry->program = saved_program;
+ entry->is_not = is_not;
+ entry->is_last_name = is_last_name;
+ list_add_tail_rcu(&entry->list,
+ &tomoyo_domain_initializer_list);
+ entry = NULL;
+ error = 0;
+ }
out:
- up_write(&tomoyo_domain_initializer_list_lock);
+ mutex_unlock(&tomoyo_policy_lock);
+ kfree(entry);
return error;
}

@@ -415,7 +419,6 @@ static bool tomoyo_is_domain_initializer
* explicitly specified by "initialize_domain".
*/
static LIST_HEAD(tomoyo_domain_keeper_list);
-static DECLARE_RWSEM(tomoyo_domain_keeper_list_lock);

/**
* tomoyo_update_domain_keeper_entry - Update "struct tomoyo_domain_keeper_entry" list.
@@ -432,7 +435,7 @@ static int tomoyo_update_domain_keeper_e
const bool is_not,
const bool is_delete)
{
- struct tomoyo_domain_keeper_entry *new_entry;
+ struct tomoyo_domain_keeper_entry *entry = NULL;
struct tomoyo_domain_keeper_entry *ptr;
const struct tomoyo_path_info *saved_domainname;
const struct tomoyo_path_info *saved_program = NULL;
@@ -454,7 +457,9 @@ static int tomoyo_update_domain_keeper_e
saved_domainname = tomoyo_save_name(domainname);
if (!saved_domainname)
return -ENOMEM;
- down_write(&tomoyo_domain_keeper_list_lock);
+ if (!is_delete)
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ mutex_lock(&tomoyo_policy_lock);
list_for_each_entry_rcu(ptr, &tomoyo_domain_keeper_list, list) {
if (ptr->is_not != is_not ||
ptr->domainname != saved_domainname ||
@@ -468,17 +473,18 @@ static int tomoyo_update_domain_keeper_e
error = -ENOENT;
goto out;
}
- new_entry = tomoyo_alloc_element(sizeof(*new_entry));
- if (!new_entry)
- goto out;
- new_entry->domainname = saved_domainname;
- new_entry->program = saved_program;
- new_entry->is_not = is_not;
- new_entry->is_last_name = is_last_name;
- list_add_tail_rcu(&new_entry->list, &tomoyo_domain_keeper_list);
- error = 0;
+ if (tomoyo_memory_ok(entry)) {
+ entry->domainname = saved_domainname;
+ entry->program = saved_program;
+ entry->is_not = is_not;
+ entry->is_last_name = is_last_name;
+ list_add_tail_rcu(&entry->list, &tomoyo_domain_keeper_list);
+ entry = NULL;
+ error = 0;
+ }
out:
- up_write(&tomoyo_domain_keeper_list_lock);
+ mutex_unlock(&tomoyo_policy_lock);
+ kfree(entry);
return error;
}

@@ -609,7 +615,6 @@ static bool tomoyo_is_domain_keeper(cons
* execve() succeeds is calculated using /bin/cat rather than /bin/busybox .
*/
static LIST_HEAD(tomoyo_alias_list);
-static DECLARE_RWSEM(tomoyo_alias_list_lock);

/**
* tomoyo_update_alias_entry - Update "struct tomoyo_alias_entry" list.
@@ -624,7 +629,7 @@ static int tomoyo_update_alias_entry(con
const char *aliased_name,
const bool is_delete)
{
- struct tomoyo_alias_entry *new_entry;
+ struct tomoyo_alias_entry *entry = NULL;
struct tomoyo_alias_entry *ptr;
const struct tomoyo_path_info *saved_original_name;
const struct tomoyo_path_info *saved_aliased_name;
@@ -637,7 +642,9 @@ static int tomoyo_update_alias_entry(con
saved_aliased_name = tomoyo_save_name(aliased_name);
if (!saved_original_name || !saved_aliased_name)
return -ENOMEM;
- down_write(&tomoyo_alias_list_lock);
+ if (!is_delete)
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ mutex_lock(&tomoyo_policy_lock);
list_for_each_entry_rcu(ptr, &tomoyo_alias_list, list) {
if (ptr->original_name != saved_original_name ||
ptr->aliased_name != saved_aliased_name)
@@ -650,15 +657,16 @@ static int tomoyo_update_alias_entry(con
error = -ENOENT;
goto out;
}
- new_entry = tomoyo_alloc_element(sizeof(*new_entry));
- if (!new_entry)
- goto out;
- new_entry->original_name = saved_original_name;
- new_entry->aliased_name = saved_aliased_name;
- list_add_tail_rcu(&new_entry->list, &tomoyo_alias_list);
- error = 0;
+ if (tomoyo_memory_ok(entry)) {
+ entry->original_name = saved_original_name;
+ entry->aliased_name = saved_aliased_name;
+ list_add_tail_rcu(&entry->list, &tomoyo_alias_list);
+ entry = NULL;
+ error = 0;
+ }
out:
- up_write(&tomoyo_alias_list_lock);
+ mutex_unlock(&tomoyo_policy_lock);
+ kfree(entry);
return error;
}

@@ -719,57 +727,28 @@ struct tomoyo_domain_info *tomoyo_find_o
domainname,
const u8 profile)
{
- struct tomoyo_domain_info *domain = NULL;
+ struct tomoyo_domain_info *entry;
+ struct tomoyo_domain_info *domain;
const struct tomoyo_path_info *saved_domainname;

- down_write(&tomoyo_domain_list_lock);
- domain = tomoyo_find_domain(domainname);
- if (domain)
- goto out;
if (!tomoyo_is_correct_domain(domainname, __func__))
- goto out;
+ return NULL;
saved_domainname = tomoyo_save_name(domainname);
if (!saved_domainname)
- goto out;
- /* Can I reuse memory of deleted domain? */
- list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
- struct task_struct *p;
- struct tomoyo_acl_info *ptr;
- bool flag;
- if (!domain->is_deleted ||
- domain->domainname != saved_domainname)
- continue;
- flag = false;
- read_lock(&tasklist_lock);
- for_each_process(p) {
- if (tomoyo_real_domain(p) != domain)
- continue;
- flag = true;
- break;
- }
- read_unlock(&tasklist_lock);
- if (flag)
- continue;
- list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
- ptr->type |= TOMOYO_ACL_DELETED;
- }
- tomoyo_set_domain_flag(domain, true, domain->flags);
- domain->profile = profile;
- domain->quota_warned = false;
- mb(); /* Avoid out-of-order execution. */
- domain->is_deleted = false;
- goto out;
- }
- /* No memory reusable. Create using new memory. */
- domain = tomoyo_alloc_element(sizeof(*domain));
- if (domain) {
+ return NULL;
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ mutex_lock(&tomoyo_policy_lock);
+ domain = tomoyo_find_domain(domainname);
+ if (!domain && tomoyo_memory_ok(entry)) {
+ domain = entry;
INIT_LIST_HEAD(&domain->acl_info_list);
domain->domainname = saved_domainname;
domain->profile = profile;
list_add_tail_rcu(&domain->list, &tomoyo_domain_list);
+ entry = NULL;
}
- out:
- up_write(&tomoyo_domain_list_lock);
+ mutex_unlock(&tomoyo_policy_lock);
+ kfree(entry);
return domain;
}

--- security-testing-2.6.orig/security/tomoyo/file.c
+++ security-testing-2.6/security/tomoyo/file.c
@@ -159,9 +159,6 @@ static struct tomoyo_path_info *tomoyo_g
return NULL;
}

-/* Lock for domain->acl_info_list. */
-DECLARE_RWSEM(tomoyo_domain_acl_info_list_lock);
-
static int tomoyo_update_double_path_acl(const u8 type, const char *filename1,
const char *filename2,
struct tomoyo_domain_info *
@@ -196,7 +193,6 @@ static int tomoyo_update_single_path_acl
* belongs to.
*/
static LIST_HEAD(tomoyo_globally_readable_list);
-static DECLARE_RWSEM(tomoyo_globally_readable_list_lock);

/**
* tomoyo_update_globally_readable_entry - Update "struct tomoyo_globally_readable_file_entry" list.
@@ -209,7 +205,7 @@ static DECLARE_RWSEM(tomoyo_globally_rea
static int tomoyo_update_globally_readable_entry(const char *filename,
const bool is_delete)
{
- struct tomoyo_globally_readable_file_entry *new_entry;
+ struct tomoyo_globally_readable_file_entry *entry = NULL;
struct tomoyo_globally_readable_file_entry *ptr;
const struct tomoyo_path_info *saved_filename;
int error = -ENOMEM;
@@ -219,7 +215,9 @@ static int tomoyo_update_globally_readab
saved_filename = tomoyo_save_name(filename);
if (!saved_filename)
return -ENOMEM;
- down_write(&tomoyo_globally_readable_list_lock);
+ if (!is_delete)
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ mutex_lock(&tomoyo_policy_lock);
list_for_each_entry_rcu(ptr, &tomoyo_globally_readable_list, list) {
if (ptr->filename != saved_filename)
continue;
@@ -231,14 +229,15 @@ static int tomoyo_update_globally_readab
error = -ENOENT;
goto out;
}
- new_entry = tomoyo_alloc_element(sizeof(*new_entry));
- if (!new_entry)
- goto out;
- new_entry->filename = saved_filename;
- list_add_tail_rcu(&new_entry->list, &tomoyo_globally_readable_list);
- error = 0;
+ if (tomoyo_memory_ok(entry)) {
+ entry->filename = saved_filename;
+ list_add_tail_rcu(&entry->list, &tomoyo_globally_readable_list);
+ entry = NULL;
+ error = 0;
+ }
out:
- up_write(&tomoyo_globally_readable_list_lock);
+ mutex_unlock(&tomoyo_policy_lock);
+ kfree(entry);
return error;
}

@@ -335,7 +334,6 @@ bool tomoyo_read_globally_readable_polic
* current process from accessing other process's information.
*/
static LIST_HEAD(tomoyo_pattern_list);
-static DECLARE_RWSEM(tomoyo_pattern_list_lock);

/**
* tomoyo_update_file_pattern_entry - Update "struct tomoyo_pattern_entry" list.
@@ -348,7 +346,7 @@ static DECLARE_RWSEM(tomoyo_pattern_list
static int tomoyo_update_file_pattern_entry(const char *pattern,
const bool is_delete)
{
- struct tomoyo_pattern_entry *new_entry;
+ struct tomoyo_pattern_entry *entry = NULL;
struct tomoyo_pattern_entry *ptr;
const struct tomoyo_path_info *saved_pattern;
int error = -ENOMEM;
@@ -358,7 +356,9 @@ static int tomoyo_update_file_pattern_en
saved_pattern = tomoyo_save_name(pattern);
if (!saved_pattern)
return -ENOMEM;
- down_write(&tomoyo_pattern_list_lock);
+ if (!is_delete)
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ mutex_lock(&tomoyo_policy_lock);
list_for_each_entry_rcu(ptr, &tomoyo_pattern_list, list) {
if (saved_pattern != ptr->pattern)
continue;
@@ -370,14 +370,15 @@ static int tomoyo_update_file_pattern_en
error = -ENOENT;
goto out;
}
- new_entry = tomoyo_alloc_element(sizeof(*new_entry));
- if (!new_entry)
- goto out;
- new_entry->pattern = saved_pattern;
- list_add_tail_rcu(&new_entry->list, &tomoyo_pattern_list);
- error = 0;
+ if (tomoyo_memory_ok(entry)) {
+ entry->pattern = saved_pattern;
+ list_add_tail_rcu(&entry->list, &tomoyo_pattern_list);
+ entry = NULL;
+ error = 0;
+ }
out:
- up_write(&tomoyo_pattern_list_lock);
+ mutex_unlock(&tomoyo_policy_lock);
+ kfree(entry);
return error;
}

@@ -480,7 +481,6 @@ bool tomoyo_read_file_pattern(struct tom
* need to worry whether the file is already unlink()ed or not.
*/
static LIST_HEAD(tomoyo_no_rewrite_list);
-static DECLARE_RWSEM(tomoyo_no_rewrite_list_lock);

/**
* tomoyo_update_no_rewrite_entry - Update "struct tomoyo_no_rewrite_entry" list.
@@ -493,7 +493,8 @@ static DECLARE_RWSEM(tomoyo_no_rewrite_l
static int tomoyo_update_no_rewrite_entry(const char *pattern,
const bool is_delete)
{
- struct tomoyo_no_rewrite_entry *new_entry, *ptr;
+ struct tomoyo_no_rewrite_entry *entry = NULL;
+ struct tomoyo_no_rewrite_entry *ptr;
const struct tomoyo_path_info *saved_pattern;
int error = -ENOMEM;

@@ -502,7 +503,9 @@ static int tomoyo_update_no_rewrite_entr
saved_pattern = tomoyo_save_name(pattern);
if (!saved_pattern)
return -ENOMEM;
- down_write(&tomoyo_no_rewrite_list_lock);
+ if (!is_delete)
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ mutex_lock(&tomoyo_policy_lock);
list_for_each_entry_rcu(ptr, &tomoyo_no_rewrite_list, list) {
if (ptr->pattern != saved_pattern)
continue;
@@ -514,14 +517,15 @@ static int tomoyo_update_no_rewrite_entr
error = -ENOENT;
goto out;
}
- new_entry = tomoyo_alloc_element(sizeof(*new_entry));
- if (!new_entry)
- goto out;
- new_entry->pattern = saved_pattern;
- list_add_tail_rcu(&new_entry->list, &tomoyo_no_rewrite_list);
- error = 0;
+ if (tomoyo_memory_ok(entry)) {
+ entry->pattern = saved_pattern;
+ list_add_tail_rcu(&entry->list, &tomoyo_no_rewrite_list);
+ entry = NULL;
+ error = 0;
+ }
out:
- up_write(&tomoyo_no_rewrite_list_lock);
+ mutex_unlock(&tomoyo_policy_lock);
+ kfree(entry);
return error;
}

@@ -821,6 +825,7 @@ static int tomoyo_update_single_path_acl
const struct tomoyo_path_info *saved_filename;
struct tomoyo_acl_info *ptr;
struct tomoyo_single_path_acl_record *acl;
+ struct tomoyo_single_path_acl_record *entry = NULL;
int error = -ENOMEM;
const u16 perm = 1 << type;

@@ -831,7 +836,9 @@ static int tomoyo_update_single_path_acl
saved_filename = tomoyo_save_name(filename);
if (!saved_filename)
return -ENOMEM;
- down_write(&tomoyo_domain_acl_info_list_lock);
+ if (!is_delete)
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ mutex_lock(&tomoyo_policy_lock);
if (is_delete)
goto delete;
list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
@@ -854,15 +861,17 @@ static int tomoyo_update_single_path_acl
goto out;
}
/* Not found. Append it to the tail. */
- acl = tomoyo_alloc_acl_element(TOMOYO_TYPE_SINGLE_PATH_ACL);
- if (!acl)
- goto out;
- acl->perm = perm;
- if (perm == (1 << TOMOYO_TYPE_READ_WRITE_ACL))
- acl->perm |= rw_mask;
- acl->filename = saved_filename;
- list_add_tail_rcu(&acl->head.list, &domain->acl_info_list);
- error = 0;
+ if (tomoyo_memory_ok(entry)) {
+ acl = entry;
+ acl->head.type = TOMOYO_TYPE_SINGLE_PATH_ACL;
+ acl->perm = perm;
+ if (perm == (1 << TOMOYO_TYPE_READ_WRITE_ACL))
+ acl->perm |= rw_mask;
+ acl->filename = saved_filename;
+ list_add_tail_rcu(&acl->head.list, &domain->acl_info_list);
+ entry = NULL;
+ error = 0;
+ }
goto out;
delete:
error = -ENOENT;
@@ -884,7 +893,8 @@ static int tomoyo_update_single_path_acl
break;
}
out:
- up_write(&tomoyo_domain_acl_info_list_lock);
+ mutex_unlock(&tomoyo_policy_lock);
+ kfree(entry);
return error;
}

@@ -908,6 +918,7 @@ static int tomoyo_update_double_path_acl
const struct tomoyo_path_info *saved_filename2;
struct tomoyo_acl_info *ptr;
struct tomoyo_double_path_acl_record *acl;
+ struct tomoyo_double_path_acl_record *entry = NULL;
int error = -ENOMEM;
const u8 perm = 1 << type;

@@ -920,7 +931,9 @@ static int tomoyo_update_double_path_acl
saved_filename2 = tomoyo_save_name(filename2);
if (!saved_filename1 || !saved_filename2)
return -ENOMEM;
- down_write(&tomoyo_domain_acl_info_list_lock);
+ if (!is_delete)
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ mutex_lock(&tomoyo_policy_lock);
if (is_delete)
goto delete;
list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
@@ -940,14 +953,16 @@ static int tomoyo_update_double_path_acl
goto out;
}
/* Not found. Append it to the tail. */
- acl = tomoyo_alloc_acl_element(TOMOYO_TYPE_DOUBLE_PATH_ACL);
- if (!acl)
- goto out;
- acl->perm = perm;
- acl->filename1 = saved_filename1;
- acl->filename2 = saved_filename2;
- list_add_tail_rcu(&acl->head.list, &domain->acl_info_list);
- error = 0;
+ if (tomoyo_memory_ok(entry)) {
+ acl = entry;
+ acl->head.type = TOMOYO_TYPE_DOUBLE_PATH_ACL;
+ acl->perm = perm;
+ acl->filename1 = saved_filename1;
+ acl->filename2 = saved_filename2;
+ list_add_tail_rcu(&acl->head.list, &domain->acl_info_list);
+ entry = NULL;
+ error = 0;
+ }
goto out;
delete:
error = -ENOENT;
@@ -966,7 +981,8 @@ static int tomoyo_update_double_path_acl
break;
}
out:
- up_write(&tomoyo_domain_acl_info_list_lock);
+ mutex_unlock(&tomoyo_policy_lock);
+ kfree(entry);
return error;
}

--- security-testing-2.6.orig/security/tomoyo/realpath.c
+++ security-testing-2.6/security/tomoyo/realpath.c
@@ -17,6 +17,25 @@
#include "realpath.h"

/**
+ * tomoyo_warn_oom - Print wraning message if memory allocation failed.
+ *
+ * @function: Function name.
+ */
+static void tomoyo_warn_oom(const char *function)
+{
+ /* Reduce error messages. */
+ static pid_t tomoyo_last_pid;
+ const pid_t pid = current->pid;
+ if (tomoyo_last_pid != pid) {
+ printk(KERN_WARNING "ERROR: Out of memory at %s.\n",
+ function);
+ tomoyo_last_pid = pid;
+ }
+ if (!tomoyo_policy_loaded)
+ panic("MAC Initialization failed.\n");
+}
+
+/**
* tomoyo_encode: Convert binary string to ascii string.
*
* @buffer: Buffer for ASCII string.
@@ -200,57 +219,26 @@ static unsigned int tomoyo_allocated_mem
static unsigned int tomoyo_quota_for_elements;

/**
- * tomoyo_alloc_element - Allocate permanent memory for structures.
+ * tomoyo_memory_ok - Check memory quota.
*
- * @size: Size in bytes.
+ * @ptr: Pointer to allocated memory.
*
- * Returns pointer to allocated memory on success, NULL otherwise.
+ * Returns true if @ptr is not NULL and quota not exceeded, false otherwise.
*
- * Memory has to be zeroed.
- * The RAM is chunked, so NEVER try to kfree() the returned pointer.
+ * Caller holds to tomoyo_policy_lock mutex.
*/
-void *tomoyo_alloc_element(const unsigned int size)
+bool tomoyo_memory_ok(void *ptr)
{
- static char *buf;
- static DEFINE_MUTEX(lock);
- static unsigned int buf_used_len = PATH_MAX;
- char *ptr = NULL;
- /*Assumes sizeof(void *) >= sizeof(long) is true. */
- const unsigned int word_aligned_size
- = roundup(size, max(sizeof(void *), sizeof(long)));
- if (word_aligned_size > PATH_MAX)
- return NULL;
- mutex_lock(&lock);
- if (buf_used_len + word_aligned_size > PATH_MAX) {
- if (!tomoyo_quota_for_elements ||
- tomoyo_allocated_memory_for_elements
- + PATH_MAX <= tomoyo_quota_for_elements)
- ptr = kzalloc(PATH_MAX, GFP_KERNEL);
- if (!ptr) {
- printk(KERN_WARNING "ERROR: Out of memory "
- "for tomoyo_alloc_element().\n");
- if (!tomoyo_policy_loaded)
- panic("MAC Initialization failed.\n");
- } else {
- buf = ptr;
- tomoyo_allocated_memory_for_elements += PATH_MAX;
- buf_used_len = word_aligned_size;
- ptr = buf;
- }
- } else if (word_aligned_size) {
- int i;
- ptr = buf + buf_used_len;
- buf_used_len += word_aligned_size;
- for (i = 0; i < word_aligned_size; i++) {
- if (!ptr[i])
- continue;
- printk(KERN_ERR "WARNING: Reserved memory was tainted! "
- "The system might go wrong.\n");
- ptr[i] = '\0';
- }
+ const unsigned int s = ptr ? ksize(ptr) : 0;
+ if (ptr && (!tomoyo_quota_for_elements ||
+ tomoyo_allocated_memory_for_elements + s
+ <= tomoyo_quota_for_elements)) {
+ tomoyo_allocated_memory_for_elements += s;
+ memset(ptr, 0, s);
+ return true;
}
- mutex_unlock(&lock);
- return ptr;
+ tomoyo_warn_oom(__func__);
+ return false;
}

/* Memory allocated for string data in bytes. */
@@ -280,13 +268,6 @@ struct tomoyo_name_entry {
struct tomoyo_path_info entry;
};

-/* Structure for available memory region. */
-struct tomoyo_free_memory_block_list {
- struct list_head list;
- char *ptr; /* Pointer to a free area. */
- int len; /* Length of the area. */
-};
-
/*
* tomoyo_name_list is used for holding string data used by TOMOYO.
* Since same string data is likely used for multiple times (e.g.
@@ -294,6 +275,7 @@ struct tomoyo_free_memory_block_list {
* "const struct tomoyo_path_info *".
*/
static struct list_head tomoyo_name_list[TOMOYO_MAX_HASH];
+static DEFINE_MUTEX(tomoyo_name_list_lock);

/**
* tomoyo_save_name - Allocate permanent memory for string data.
@@ -306,72 +288,39 @@ static struct list_head tomoyo_name_list
*/
const struct tomoyo_path_info *tomoyo_save_name(const char *name)
{
- static LIST_HEAD(fmb_list);
- static DEFINE_MUTEX(lock);
struct tomoyo_name_entry *ptr;
unsigned int hash;
- /* fmb contains available size in bytes.
- fmb is removed from the fmb_list when fmb->len becomes 0. */
- struct tomoyo_free_memory_block_list *fmb;
int len;
- char *cp;
+ int allocated_len;

if (!name)
return NULL;
len = strlen(name) + 1;
- if (len > TOMOYO_MAX_PATHNAME_LEN) {
- printk(KERN_WARNING "ERROR: Name too long "
- "for tomoyo_save_name().\n");
- return NULL;
- }
hash = full_name_hash((const unsigned char *) name, len - 1);
- mutex_lock(&lock);
+ mutex_lock(&tomoyo_name_list_lock);
list_for_each_entry(ptr, &tomoyo_name_list[hash % TOMOYO_MAX_HASH],
- list) {
- if (hash == ptr->entry.hash && !strcmp(name, ptr->entry.name))
- goto out;
- }
- list_for_each_entry(fmb, &fmb_list, list) {
- if (len <= fmb->len)
- goto ready;
- }
- if (!tomoyo_quota_for_savename ||
- tomoyo_allocated_memory_for_savename + PATH_MAX
- <= tomoyo_quota_for_savename)
- cp = kzalloc(PATH_MAX, GFP_KERNEL);
- else
- cp = NULL;
- fmb = kzalloc(sizeof(*fmb), GFP_KERNEL);
- if (!cp || !fmb) {
- kfree(cp);
- kfree(fmb);
- printk(KERN_WARNING "ERROR: Out of memory "
- "for tomoyo_save_name().\n");
- if (!tomoyo_policy_loaded)
- panic("MAC Initialization failed.\n");
- ptr = NULL;
+ list) {
+ if (hash != ptr->entry.hash || strcmp(name, ptr->entry.name))
+ continue;
goto out;
}
- tomoyo_allocated_memory_for_savename += PATH_MAX;
- list_add(&fmb->list, &fmb_list);
- fmb->ptr = cp;
- fmb->len = PATH_MAX;
- ready:
- ptr = tomoyo_alloc_element(sizeof(*ptr));
- if (!ptr)
+ ptr = kzalloc(sizeof(*ptr) + len, GFP_KERNEL);
+ allocated_len = ptr ? ksize(ptr) : 0;
+ if (!ptr || (tomoyo_quota_for_savename &&
+ tomoyo_allocated_memory_for_savename + allocated_len
+ > tomoyo_quota_for_savename)) {
+ kfree(ptr);
+ ptr = NULL;
+ tomoyo_warn_oom(__func__);
goto out;
- ptr->entry.name = fmb->ptr;
- memmove(fmb->ptr, name, len);
+ }
+ tomoyo_allocated_memory_for_savename += allocated_len;
+ ptr->entry.name = ((char *) ptr) + sizeof(*ptr);
+ memmove((char *) ptr->entry.name, name, len);
tomoyo_fill_path_info(&ptr->entry);
- fmb->ptr += len;
- fmb->len -= len;
list_add_tail(&ptr->list, &tomoyo_name_list[hash % TOMOYO_MAX_HASH]);
- if (fmb->len == 0) {
- list_del(&fmb->list);
- kfree(fmb);
- }
out:
- mutex_unlock(&lock);
+ mutex_unlock(&tomoyo_name_list_lock);
return ptr ? &ptr->entry : NULL;
}

--- security-testing-2.6.orig/security/tomoyo/realpath.h
+++ security-testing-2.6/security/tomoyo/realpath.h
@@ -37,12 +37,6 @@ char *tomoyo_realpath_nofollow(const cha
char *tomoyo_realpath_from_path(struct path *path);

/*
- * Allocate memory for ACL entry.
- * The RAM is chunked, so NEVER try to kfree() the returned pointer.
- */
-void *tomoyo_alloc_element(const unsigned int size);
-
-/*
* Keep the given name on the RAM.
* The RAM is shared, so NEVER try to modify or kfree() the returned name.
*/

2009-10-29 05:12:09

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [TOMOYO #16 01/25] LSM: Add security_path_chmod() and security_path_chown().

Quoting Tetsuo Handa ([email protected]):
> This patch allows pathname based LSM modules to check chmod()/chown()
> operations. Since notify_change() does not receive "struct vfsmount *",
> we add security_path_chmod() and security_path_chown() to the caller of
> notify_change().
>
> These hooks are used by TOMOYO.
>
> Signed-off-by: Tetsuo Handa <[email protected]>
> ---
> fs/open.c | 24 ++++++++++++++++++++----
> include/linux/security.h | 30 ++++++++++++++++++++++++++++++
> security/capability.c | 13 +++++++++++++
> security/security.c | 15 +++++++++++++++
> 4 files changed, 78 insertions(+), 4 deletions(-)
>
> --- security-testing-2.6.orig/fs/open.c
> +++ security-testing-2.6/fs/open.c
> @@ -616,6 +616,9 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd
> err = mnt_want_write_file(file);
> if (err)
> goto out_putf;
> + err = security_path_chmod(dentry, file->f_vfsmnt, mode);
> + if (err)
> + goto out_drop_write;
> mutex_lock(&inode->i_mutex);

Isn't doing the security check before the mutex_lock racy?

Any reason not to move it into the lock? (since you had
considered putting a path hook in notify_change() I assume
not?)

-serge

2009-10-29 05:40:25

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH] TOMOYO: Use RCU primitives for list operation

Quoting Tetsuo Handa ([email protected]):
> [PATCH] TOMOYO: Use RCU primitives for list operation
>
> Remove down_read()/up_read() by replacing with RCU primitives.
> SRCU based garbage collector will be added in the future.
>
> Signed-off-by: Tetsuo Handa <[email protected]>
> ---
> security/tomoyo/common.c | 52 ++++++++++-----------------------------------
> security/tomoyo/common.h | 14 ++++++------
> security/tomoyo/domain.c | 38 ++++++++++----------------------
> security/tomoyo/file.c | 50 ++++++++++++++-----------------------------
> security/tomoyo/realpath.c | 4 ---
> 5 files changed, 49 insertions(+), 109 deletions(-)
>
> --- security-testing-2.6.orig/security/tomoyo/common.c
> +++ security-testing-2.6/security/tomoyo/common.c
> @@ -365,9 +365,6 @@ bool tomoyo_is_domain_def(const unsigned
> *
> * @domainname: The domainname to find.
> *
> - * Caller must call down_read(&tomoyo_domain_list_lock); or
> - * down_write(&tomoyo_domain_list_lock); .
> - *
> * Returns pointer to "struct tomoyo_domain_info" if found, NULL otherwise.
> */
> struct tomoyo_domain_info *tomoyo_find_domain(const char *domainname)
> @@ -377,7 +374,7 @@ struct tomoyo_domain_info *tomoyo_find_d
>
> name.name = domainname;
> tomoyo_fill_path_info(&name);
> - list_for_each_entry(domain, &tomoyo_domain_list, list) {
> + list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
> if (!domain->is_deleted &&
> !tomoyo_pathcmp(&name, domain->domainname))
> return domain;
> @@ -837,8 +834,7 @@ bool tomoyo_domain_quota_is_ok(struct to
>
> if (!domain)
> return true;
> - down_read(&tomoyo_domain_acl_info_list_lock);
> - list_for_each_entry(ptr, &domain->acl_info_list, list) {
> + list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
> if (ptr->type & TOMOYO_ACL_DELETED)
> continue;
> switch (tomoyo_acl_type2(ptr)) {

You are removing the down_read()s, but not replacing them with
rcu_read_lock()s. I assume this is based on the same discussions
you had with Paul awhile ago about the safety of walking the list
bc you only append to the end (which I trust must have concluded
in your favor)?

If you'll be adding gc eventually anyway, is it really worthwhile
to 'violate the rules' now by calling list_for_each_entry_rcu()
without being inside rcu_read_lock() now? I fear it'll only serve
to confuse readers, especially those looking for rcu users to serve
as examples.

-serge

2009-10-29 15:56:51

by Tetsuo Handa

[permalink] [raw]
Subject: Re: [TOMOYO #16 01/25] LSM: Add security_path_chmod() andsecurity_path_chown().

Hello.

Serge E. Hallyn wrote:
> Quoting Tetsuo Handa ([email protected]):
> > This patch allows pathname based LSM modules to check chmod()/chown()
> > operations. Since notify_change() does not receive "struct vfsmount *",
> > we add security_path_chmod() and security_path_chown() to the caller of
> > notify_change().
> >
> > These hooks are used by TOMOYO.
> >
> > Signed-off-by: Tetsuo Handa <[email protected]>
> > ---
> > fs/open.c | 24 ++++++++++++++++++++----
> > include/linux/security.h | 30 ++++++++++++++++++++++++++++++
> > security/capability.c | 13 +++++++++++++
> > security/security.c | 15 +++++++++++++++
> > 4 files changed, 78 insertions(+), 4 deletions(-)
> >
> > --- security-testing-2.6.orig/fs/open.c
> > +++ security-testing-2.6/fs/open.c
> > @@ -616,6 +616,9 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd
> > err = mnt_want_write_file(file);
> > if (err)
> > goto out_putf;
> > + err = security_path_chmod(dentry, file->f_vfsmnt, mode);
> > + if (err)
> > + goto out_drop_write;
> > mutex_lock(&inode->i_mutex);
>
> Isn't doing the security check before the mutex_lock racy?
>
> Any reason not to move it into the lock? (since you had
> considered putting a path hook in notify_change() I assume
> not?)
>
None for security_path_chmod(). None for security_path_chown() if we are
allowed to pass "struct vfsmount" to chown_common().
> -serge
>
Al, is it OK to pass "struct vfsmount" to chown_common() so that
security_path_chown() is called after mutex_lock()?
----------
[PATCH] LSM: Move security_path_chmod()/security_path_chown() to after mutex_lock().

We should call security_path_chmod()/security_path_chown() after mutex_lock()
in order to avoid races.

Signed-off-by: Tetsuo Handa <[email protected]>
---
fs/open.c | 36 +++++++++++++++---------------------
1 file changed, 15 insertions(+), 21 deletions(-)

--- security-testing-2.6.orig/fs/open.c
+++ security-testing-2.6/fs/open.c
@@ -619,17 +619,17 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd
err = mnt_want_write_file(file);
if (err)
goto out_putf;
+ mutex_lock(&inode->i_mutex);
err = security_path_chmod(dentry, file->f_vfsmnt, mode);
if (err)
- goto out_drop_write;
- mutex_lock(&inode->i_mutex);
+ goto out_unlock;
if (mode == (mode_t) -1)
mode = inode->i_mode;
newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
err = notify_change(dentry, &newattrs);
+out_unlock:
mutex_unlock(&inode->i_mutex);
-out_drop_write:
mnt_drop_write(file->f_path.mnt);
out_putf:
fput(file);
@@ -652,17 +652,17 @@ SYSCALL_DEFINE3(fchmodat, int, dfd, cons
error = mnt_want_write(path.mnt);
if (error)
goto dput_and_out;
+ mutex_lock(&inode->i_mutex);
error = security_path_chmod(path.dentry, path.mnt, mode);
if (error)
- goto out_drop_write;
- mutex_lock(&inode->i_mutex);
+ goto out_unlock;
if (mode == (mode_t) -1)
mode = inode->i_mode;
newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
error = notify_change(path.dentry, &newattrs);
+out_unlock:
mutex_unlock(&inode->i_mutex);
-out_drop_write:
mnt_drop_write(path.mnt);
dput_and_out:
path_put(&path);
@@ -675,9 +675,9 @@ SYSCALL_DEFINE2(chmod, const char __user
return sys_fchmodat(AT_FDCWD, filename, mode);
}

-static int chown_common(struct dentry * dentry, uid_t user, gid_t group)
+static int chown_common(struct path *path, uid_t user, gid_t group)
{
- struct inode *inode = dentry->d_inode;
+ struct inode *inode = path->dentry->d_inode;
int error;
struct iattr newattrs;

@@ -694,7 +694,9 @@ static int chown_common(struct dentry *
newattrs.ia_valid |=
ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV;
mutex_lock(&inode->i_mutex);
- error = notify_change(dentry, &newattrs);
+ error = security_path_chown(path, user, group);
+ if (!error)
+ error = notify_change(path->dentry, &newattrs);
mutex_unlock(&inode->i_mutex);

return error;
@@ -711,9 +713,7 @@ SYSCALL_DEFINE3(chown, const char __user
error = mnt_want_write(path.mnt);
if (error)
goto out_release;
- error = security_path_chown(&path, user, group);
- if (!error)
- error = chown_common(path.dentry, user, group);
+ error = chown_common(&path, user, group);
mnt_drop_write(path.mnt);
out_release:
path_put(&path);
@@ -738,9 +738,7 @@ SYSCALL_DEFINE5(fchownat, int, dfd, cons
error = mnt_want_write(path.mnt);
if (error)
goto out_release;
- error = security_path_chown(&path, user, group);
- if (!error)
- error = chown_common(path.dentry, user, group);
+ error = chown_common(&path, user, group);
mnt_drop_write(path.mnt);
out_release:
path_put(&path);
@@ -759,9 +757,7 @@ SYSCALL_DEFINE3(lchown, const char __use
error = mnt_want_write(path.mnt);
if (error)
goto out_release;
- error = security_path_chown(&path, user, group);
- if (!error)
- error = chown_common(path.dentry, user, group);
+ error = chown_common(&path, user, group);
mnt_drop_write(path.mnt);
out_release:
path_put(&path);
@@ -784,9 +780,7 @@ SYSCALL_DEFINE3(fchown, unsigned int, fd
goto out_fput;
dentry = file->f_path.dentry;
audit_inode(NULL, dentry);
- error = security_path_chown(&file->f_path, user, group);
- if (!error)
- error = chown_common(dentry, user, group);
+ error = chown_common(&file->f_path, user, group);
mnt_drop_write(file->f_path.mnt);
out_fput:
fput(file);

2009-11-22 02:49:52

by Tetsuo Handa

[permalink] [raw]
Subject: [PATCH] LSM: Move security_path_chmod()/security_path_chown() to after mutex_lock().

[PATCH] LSM: Move security_path_chmod()/security_path_chown() to after mutex_lock().

We should call security_path_chmod()/security_path_chown() after mutex_lock()
in order to avoid races.

Signed-off-by: Tetsuo Handa <[email protected]>
---
fs/open.c | 36 +++++++++++++++---------------------
1 file changed, 15 insertions(+), 21 deletions(-)

--- security-testing-2.6.orig/fs/open.c
+++ security-testing-2.6/fs/open.c
@@ -619,17 +619,17 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd
err = mnt_want_write_file(file);
if (err)
goto out_putf;
+ mutex_lock(&inode->i_mutex);
err = security_path_chmod(dentry, file->f_vfsmnt, mode);
if (err)
- goto out_drop_write;
- mutex_lock(&inode->i_mutex);
+ goto out_unlock;
if (mode == (mode_t) -1)
mode = inode->i_mode;
newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
err = notify_change(dentry, &newattrs);
+out_unlock:
mutex_unlock(&inode->i_mutex);
-out_drop_write:
mnt_drop_write(file->f_path.mnt);
out_putf:
fput(file);
@@ -652,17 +652,17 @@ SYSCALL_DEFINE3(fchmodat, int, dfd, cons
error = mnt_want_write(path.mnt);
if (error)
goto dput_and_out;
+ mutex_lock(&inode->i_mutex);
error = security_path_chmod(path.dentry, path.mnt, mode);
if (error)
- goto out_drop_write;
- mutex_lock(&inode->i_mutex);
+ goto out_unlock;
if (mode == (mode_t) -1)
mode = inode->i_mode;
newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
error = notify_change(path.dentry, &newattrs);
+out_unlock:
mutex_unlock(&inode->i_mutex);
-out_drop_write:
mnt_drop_write(path.mnt);
dput_and_out:
path_put(&path);
@@ -675,9 +675,9 @@ SYSCALL_DEFINE2(chmod, const char __user
return sys_fchmodat(AT_FDCWD, filename, mode);
}

-static int chown_common(struct dentry * dentry, uid_t user, gid_t group)
+static int chown_common(struct path *path, uid_t user, gid_t group)
{
- struct inode *inode = dentry->d_inode;
+ struct inode *inode = path->dentry->d_inode;
int error;
struct iattr newattrs;

@@ -694,7 +694,9 @@ static int chown_common(struct dentry *
newattrs.ia_valid |=
ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV;
mutex_lock(&inode->i_mutex);
- error = notify_change(dentry, &newattrs);
+ error = security_path_chown(path, user, group);
+ if (!error)
+ error = notify_change(path->dentry, &newattrs);
mutex_unlock(&inode->i_mutex);

return error;
@@ -711,9 +713,7 @@ SYSCALL_DEFINE3(chown, const char __user
error = mnt_want_write(path.mnt);
if (error)
goto out_release;
- error = security_path_chown(&path, user, group);
- if (!error)
- error = chown_common(path.dentry, user, group);
+ error = chown_common(&path, user, group);
mnt_drop_write(path.mnt);
out_release:
path_put(&path);
@@ -738,9 +738,7 @@ SYSCALL_DEFINE5(fchownat, int, dfd, cons
error = mnt_want_write(path.mnt);
if (error)
goto out_release;
- error = security_path_chown(&path, user, group);
- if (!error)
- error = chown_common(path.dentry, user, group);
+ error = chown_common(&path, user, group);
mnt_drop_write(path.mnt);
out_release:
path_put(&path);
@@ -759,9 +757,7 @@ SYSCALL_DEFINE3(lchown, const char __use
error = mnt_want_write(path.mnt);
if (error)
goto out_release;
- error = security_path_chown(&path, user, group);
- if (!error)
- error = chown_common(path.dentry, user, group);
+ error = chown_common(&path, user, group);
mnt_drop_write(path.mnt);
out_release:
path_put(&path);
@@ -784,9 +780,7 @@ SYSCALL_DEFINE3(fchown, unsigned int, fd
goto out_fput;
dentry = file->f_path.dentry;
audit_inode(NULL, dentry);
- error = security_path_chown(&file->f_path, user, group);
- if (!error)
- error = chown_common(dentry, user, group);
+ error = chown_common(&file->f_path, user, group);
mnt_drop_write(file->f_path.mnt);
out_fput:
fput(file);

2009-11-23 10:09:50

by John Johansen

[permalink] [raw]
Subject: Re: [PATCH] LSM: Move security_path_chmod()/security_path_chown() to after mutex_lock().

Tetsuo Handa wrote:
> [PATCH] LSM: Move security_path_chmod()/security_path_chown() to after mutex_lock().
>
> We should call security_path_chmod()/security_path_chown() after mutex_lock()
> in order to avoid races.
>
> Signed-off-by: Tetsuo Handa <[email protected]>

Acked-by: John Johansen <[email protected]>

> ---
> fs/open.c | 36 +++++++++++++++---------------------
> 1 file changed, 15 insertions(+), 21 deletions(-)
>
> --- security-testing-2.6.orig/fs/open.c
> +++ security-testing-2.6/fs/open.c
> @@ -619,17 +619,17 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd
> err = mnt_want_write_file(file);
> if (err)
> goto out_putf;
> + mutex_lock(&inode->i_mutex);
> err = security_path_chmod(dentry, file->f_vfsmnt, mode);
> if (err)
> - goto out_drop_write;
> - mutex_lock(&inode->i_mutex);
> + goto out_unlock;
> if (mode == (mode_t) -1)
> mode = inode->i_mode;
> newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
> newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
> err = notify_change(dentry, &newattrs);
> +out_unlock:
> mutex_unlock(&inode->i_mutex);
> -out_drop_write:
> mnt_drop_write(file->f_path.mnt);
> out_putf:
> fput(file);
> @@ -652,17 +652,17 @@ SYSCALL_DEFINE3(fchmodat, int, dfd, cons
> error = mnt_want_write(path.mnt);
> if (error)
> goto dput_and_out;
> + mutex_lock(&inode->i_mutex);
> error = security_path_chmod(path.dentry, path.mnt, mode);
> if (error)
> - goto out_drop_write;
> - mutex_lock(&inode->i_mutex);
> + goto out_unlock;
> if (mode == (mode_t) -1)
> mode = inode->i_mode;
> newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
> newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
> error = notify_change(path.dentry, &newattrs);
> +out_unlock:
> mutex_unlock(&inode->i_mutex);
> -out_drop_write:
> mnt_drop_write(path.mnt);
> dput_and_out:
> path_put(&path);
> @@ -675,9 +675,9 @@ SYSCALL_DEFINE2(chmod, const char __user
> return sys_fchmodat(AT_FDCWD, filename, mode);
> }
>
> -static int chown_common(struct dentry * dentry, uid_t user, gid_t group)
> +static int chown_common(struct path *path, uid_t user, gid_t group)
> {
> - struct inode *inode = dentry->d_inode;
> + struct inode *inode = path->dentry->d_inode;
> int error;
> struct iattr newattrs;
>
> @@ -694,7 +694,9 @@ static int chown_common(struct dentry *
> newattrs.ia_valid |=
> ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV;
> mutex_lock(&inode->i_mutex);
> - error = notify_change(dentry, &newattrs);
> + error = security_path_chown(path, user, group);
> + if (!error)
> + error = notify_change(path->dentry, &newattrs);
> mutex_unlock(&inode->i_mutex);
>
> return error;
> @@ -711,9 +713,7 @@ SYSCALL_DEFINE3(chown, const char __user
> error = mnt_want_write(path.mnt);
> if (error)
> goto out_release;
> - error = security_path_chown(&path, user, group);
> - if (!error)
> - error = chown_common(path.dentry, user, group);
> + error = chown_common(&path, user, group);
> mnt_drop_write(path.mnt);
> out_release:
> path_put(&path);
> @@ -738,9 +738,7 @@ SYSCALL_DEFINE5(fchownat, int, dfd, cons
> error = mnt_want_write(path.mnt);
> if (error)
> goto out_release;
> - error = security_path_chown(&path, user, group);
> - if (!error)
> - error = chown_common(path.dentry, user, group);
> + error = chown_common(&path, user, group);
> mnt_drop_write(path.mnt);
> out_release:
> path_put(&path);
> @@ -759,9 +757,7 @@ SYSCALL_DEFINE3(lchown, const char __use
> error = mnt_want_write(path.mnt);
> if (error)
> goto out_release;
> - error = security_path_chown(&path, user, group);
> - if (!error)
> - error = chown_common(path.dentry, user, group);
> + error = chown_common(&path, user, group);
> mnt_drop_write(path.mnt);
> out_release:
> path_put(&path);
> @@ -784,9 +780,7 @@ SYSCALL_DEFINE3(fchown, unsigned int, fd
> goto out_fput;
> dentry = file->f_path.dentry;
> audit_inode(NULL, dentry);
> - error = security_path_chown(&file->f_path, user, group);
> - if (!error)
> - error = chown_common(dentry, user, group);
> + error = chown_common(&file->f_path, user, group);
> mnt_drop_write(file->f_path.mnt);
> out_fput:
> fput(file);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2009-11-23 21:50:44

by James Morris

[permalink] [raw]
Subject: Re: [PATCH] LSM: Move security_path_chmod()/security_path_chown() to after mutex_lock().

On Mon, 23 Nov 2009, John Johansen wrote:

> Tetsuo Handa wrote:
> > [PATCH] LSM: Move security_path_chmod()/security_path_chown() to after mutex_lock().
> >
> > We should call security_path_chmod()/security_path_chown() after mutex_lock()
> > in order to avoid races.
> >
> > Signed-off-by: Tetsuo Handa <[email protected]>
>
> Acked-by: John Johansen <[email protected]>


Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next


--
James Morris
<[email protected]>

2009-12-04 12:34:14

by Tetsuo Handa

[permalink] [raw]
Subject: Re: [PATCH] TOMOYO: Use RCU primitives for list operation

Hello.

Serge E. Hallyn wrote:
> If you'll be adding gc eventually anyway, is it really worthwhile
> to 'violate the rules' now by calling list_for_each_entry_rcu()
> without being inside rcu_read_lock() now? I fear it'll only serve
> to confuse readers, especially those looking for rcu users to serve
> as examples.

You are right. I modified to call srcu_read_lock().

I don't modify tomoyo_file_permission() because it will be removed by
applying commit: c656ae95d1c5c8ed5763356263ace2d03087efec
(security/tomoyo: Remove now unnecessary handling of security_sysctl.).

Regards.
----------
[PATCH] TOMOYO: Use RCU primitives for list operation

Replace list operation with RCU primitives and replace
down_read()/up_read() with srcu_read_lock()/srcu_read_unlock().

Signed-off-by: Tetsuo Handa <[email protected]>
---
security/tomoyo/common.c | 90 ++++++++++++++++++++----------------
security/tomoyo/common.h | 28 ++++++++---
security/tomoyo/domain.c | 63 +++++++++++++++----------
security/tomoyo/file.c | 110 +++++++++++++++++++++++++++++++--------------
security/tomoyo/realpath.c | 8 ++-
security/tomoyo/tomoyo.c | 20 ++++++--
6 files changed, 207 insertions(+), 112 deletions(-)

--- security-testing-2.6.orig/security/tomoyo/common.c
+++ security-testing-2.6/security/tomoyo/common.c
@@ -365,10 +365,9 @@ bool tomoyo_is_domain_def(const unsigned
*
* @domainname: The domainname to find.
*
- * Caller must call down_read(&tomoyo_domain_list_lock); or
- * down_write(&tomoyo_domain_list_lock); .
- *
* Returns pointer to "struct tomoyo_domain_info" if found, NULL otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
struct tomoyo_domain_info *tomoyo_find_domain(const char *domainname)
{
@@ -377,7 +376,7 @@ struct tomoyo_domain_info *tomoyo_find_d

name.name = domainname;
tomoyo_fill_path_info(&name);
- list_for_each_entry(domain, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
if (!domain->is_deleted &&
!tomoyo_pathcmp(&name, domain->domainname))
return domain;
@@ -829,6 +828,8 @@ bool tomoyo_verbose_mode(const struct to
* @domain: Pointer to "struct tomoyo_domain_info".
*
* Returns true if the domain is not exceeded quota, false otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
bool tomoyo_domain_quota_is_ok(struct tomoyo_domain_info * const domain)
{
@@ -837,8 +838,7 @@ bool tomoyo_domain_quota_is_ok(struct to

if (!domain)
return true;
- down_read(&tomoyo_domain_acl_info_list_lock);
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
if (ptr->type & TOMOYO_ACL_DELETED)
continue;
switch (tomoyo_acl_type2(ptr)) {
@@ -891,7 +891,6 @@ bool tomoyo_domain_quota_is_ok(struct to
break;
}
}
- up_read(&tomoyo_domain_acl_info_list_lock);
if (count < tomoyo_check_flags(domain, TOMOYO_MAX_ACCEPT_ENTRY))
return true;
if (!domain->quota_warned) {
@@ -1121,6 +1120,8 @@ static DECLARE_RWSEM(tomoyo_policy_manag
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_update_manager_entry(const char *manager,
const bool is_delete)
@@ -1143,7 +1144,7 @@ static int tomoyo_update_manager_entry(c
if (!saved_manager)
return -ENOMEM;
down_write(&tomoyo_policy_manager_list_lock);
- list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_policy_manager_list, list) {
if (ptr->manager != saved_manager)
continue;
ptr->is_deleted = is_delete;
@@ -1159,7 +1160,7 @@ static int tomoyo_update_manager_entry(c
goto out;
new_entry->manager = saved_manager;
new_entry->is_domain = is_domain;
- list_add_tail(&new_entry->list, &tomoyo_policy_manager_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_policy_manager_list);
error = 0;
out:
up_write(&tomoyo_policy_manager_list_lock);
@@ -1172,6 +1173,8 @@ static int tomoyo_update_manager_entry(c
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_write_manager_policy(struct tomoyo_io_buffer *head)
{
@@ -1191,6 +1194,8 @@ static int tomoyo_write_manager_policy(s
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Returns 0.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_read_manager_policy(struct tomoyo_io_buffer *head)
{
@@ -1199,7 +1204,6 @@ static int tomoyo_read_manager_policy(st

if (head->read_eof)
return 0;
- down_read(&tomoyo_policy_manager_list_lock);
list_for_each_cookie(pos, head->read_var2,
&tomoyo_policy_manager_list) {
struct tomoyo_policy_manager_entry *ptr;
@@ -1211,7 +1215,6 @@ static int tomoyo_read_manager_policy(st
if (!done)
break;
}
- up_read(&tomoyo_policy_manager_list_lock);
head->read_eof = done;
return 0;
}
@@ -1221,6 +1224,8 @@ static int tomoyo_read_manager_policy(st
*
* Returns true if the current process is permitted to modify policy
* via /sys/kernel/security/tomoyo/ interface.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static bool tomoyo_is_policy_manager(void)
{
@@ -1234,29 +1239,25 @@ static bool tomoyo_is_policy_manager(voi
return true;
if (!tomoyo_manage_by_non_root && (task->cred->uid || task->cred->euid))
return false;
- down_read(&tomoyo_policy_manager_list_lock);
- list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_policy_manager_list, list) {
if (!ptr->is_deleted && ptr->is_domain
&& !tomoyo_pathcmp(domainname, ptr->manager)) {
found = true;
break;
}
}
- up_read(&tomoyo_policy_manager_list_lock);
if (found)
return true;
exe = tomoyo_get_exe();
if (!exe)
return false;
- down_read(&tomoyo_policy_manager_list_lock);
- list_for_each_entry(ptr, &tomoyo_policy_manager_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_policy_manager_list, list) {
if (!ptr->is_deleted && !ptr->is_domain
&& !strcmp(exe, ptr->manager->name)) {
found = true;
break;
}
}
- up_read(&tomoyo_policy_manager_list_lock);
if (!found) { /* Reduce error messages. */
static pid_t last_pid;
const pid_t pid = current->pid;
@@ -1277,6 +1278,8 @@ static bool tomoyo_is_policy_manager(voi
* @data: String to parse.
*
* Returns true on success, false otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static bool tomoyo_is_select_one(struct tomoyo_io_buffer *head,
const char *data)
@@ -1292,11 +1295,8 @@ static bool tomoyo_is_select_one(struct
domain = tomoyo_real_domain(p);
read_unlock(&tasklist_lock);
} else if (!strncmp(data, "domain=", 7)) {
- if (tomoyo_is_domain_def(data + 7)) {
- down_read(&tomoyo_domain_list_lock);
+ if (tomoyo_is_domain_def(data + 7))
domain = tomoyo_find_domain(data + 7);
- up_read(&tomoyo_domain_list_lock);
- }
} else
return false;
head->write_var1 = domain;
@@ -1310,13 +1310,11 @@ static bool tomoyo_is_select_one(struct
if (domain) {
struct tomoyo_domain_info *d;
head->read_var1 = NULL;
- down_read(&tomoyo_domain_list_lock);
- list_for_each_entry(d, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(d, &tomoyo_domain_list, list) {
if (d == domain)
break;
head->read_var1 = &d->list;
}
- up_read(&tomoyo_domain_list_lock);
head->read_var2 = NULL;
head->read_bit = 0;
head->read_step = 0;
@@ -1332,6 +1330,8 @@ static bool tomoyo_is_select_one(struct
* @domainname: The name of domain.
*
* Returns 0.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_delete_domain(char *domainname)
{
@@ -1342,7 +1342,7 @@ static int tomoyo_delete_domain(char *do
tomoyo_fill_path_info(&name);
down_write(&tomoyo_domain_list_lock);
/* Is there an active domain? */
- list_for_each_entry(domain, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
/* Never delete tomoyo_kernel_domain */
if (domain == &tomoyo_kernel_domain)
continue;
@@ -1362,6 +1362,8 @@ static int tomoyo_delete_domain(char *do
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_write_domain_policy(struct tomoyo_io_buffer *head)
{
@@ -1384,11 +1386,9 @@ static int tomoyo_write_domain_policy(st
domain = NULL;
if (is_delete)
tomoyo_delete_domain(data);
- else if (is_select) {
- down_read(&tomoyo_domain_list_lock);
+ else if (is_select)
domain = tomoyo_find_domain(data);
- up_read(&tomoyo_domain_list_lock);
- } else
+ else
domain = tomoyo_find_or_assign_new_domain(data, 0);
head->write_var1 = domain;
return 0;
@@ -1533,6 +1533,8 @@ static bool tomoyo_print_entry(struct to
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Returns 0.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_read_domain_policy(struct tomoyo_io_buffer *head)
{
@@ -1544,7 +1546,6 @@ static int tomoyo_read_domain_policy(str
return 0;
if (head->read_step == 0)
head->read_step = 1;
- down_read(&tomoyo_domain_list_lock);
list_for_each_cookie(dpos, head->read_var1, &tomoyo_domain_list) {
struct tomoyo_domain_info *domain;
const char *quota_exceeded = "";
@@ -1577,7 +1578,6 @@ acl_loop:
if (head->read_step == 3)
goto tail_mark;
/* Print ACL entries in the domain. */
- down_read(&tomoyo_domain_acl_info_list_lock);
list_for_each_cookie(apos, head->read_var2,
&domain->acl_info_list) {
struct tomoyo_acl_info *ptr
@@ -1587,7 +1587,6 @@ acl_loop:
if (!done)
break;
}
- up_read(&tomoyo_domain_acl_info_list_lock);
if (!done)
break;
head->read_step = 3;
@@ -1599,7 +1598,6 @@ tail_mark:
if (head->read_single_domain)
break;
}
- up_read(&tomoyo_domain_list_lock);
head->read_eof = done;
return 0;
}
@@ -1615,6 +1613,8 @@ tail_mark:
*
* ( echo "select " $domainname; echo "use_profile " $profile ) |
* /usr/lib/ccs/loadpolicy -d
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_write_domain_profile(struct tomoyo_io_buffer *head)
{
@@ -1626,9 +1626,7 @@ static int tomoyo_write_domain_profile(s
if (!cp)
return -EINVAL;
*cp = '\0';
- down_read(&tomoyo_domain_list_lock);
domain = tomoyo_find_domain(cp + 1);
- up_read(&tomoyo_domain_list_lock);
if (strict_strtoul(data, 10, &profile))
return -EINVAL;
if (domain && profile < TOMOYO_MAX_PROFILES
@@ -1650,6 +1648,8 @@ static int tomoyo_write_domain_profile(s
* awk ' { if ( domainname == "" ) { if ( $1 == "<kernel>" )
* domainname = $0; } else if ( $1 == "use_profile" ) {
* print $2 " " domainname; domainname = ""; } } ; '
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_read_domain_profile(struct tomoyo_io_buffer *head)
{
@@ -1658,7 +1658,6 @@ static int tomoyo_read_domain_profile(st

if (head->read_eof)
return 0;
- down_read(&tomoyo_domain_list_lock);
list_for_each_cookie(pos, head->read_var1, &tomoyo_domain_list) {
struct tomoyo_domain_info *domain;
domain = list_entry(pos, struct tomoyo_domain_info, list);
@@ -1669,7 +1668,6 @@ static int tomoyo_read_domain_profile(st
if (!done)
break;
}
- up_read(&tomoyo_domain_list_lock);
head->read_eof = done;
return 0;
}
@@ -1726,6 +1724,8 @@ static int tomoyo_read_pid(struct tomoyo
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_write_exception_policy(struct tomoyo_io_buffer *head)
{
@@ -1760,6 +1760,8 @@ static int tomoyo_write_exception_policy
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Returns 0 on success, -EINVAL otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_read_exception_policy(struct tomoyo_io_buffer *head)
{
@@ -1889,15 +1891,13 @@ void tomoyo_load_policy(const char *file
tomoyo_policy_loaded = true;
{ /* Check all profiles currently assigned to domains are defined. */
struct tomoyo_domain_info *domain;
- down_read(&tomoyo_domain_list_lock);
- list_for_each_entry(domain, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
const u8 profile = domain->profile;
if (tomoyo_profile_ptr[profile])
continue;
panic("Profile %u (used by '%s') not defined.\n",
profile, domain->domainname->name);
}
- up_read(&tomoyo_domain_list_lock);
}
}

@@ -1945,6 +1945,8 @@ static int tomoyo_read_self_domain(struc
* @file: Pointer to "struct file".
*
* Associates policy handler and returns 0 on success, -ENOMEM otherwise.
+ *
+ * Caller acquires tomoyo_read_lock().
*/
static int tomoyo_open_control(const u8 type, struct file *file)
{
@@ -2030,6 +2032,7 @@ static int tomoyo_open_control(const u8
return -ENOMEM;
}
}
+ head->reader_idx = tomoyo_read_lock();
file->private_data = head;
/*
* Call the handler now if the file is
@@ -2051,6 +2054,8 @@ static int tomoyo_open_control(const u8
* @buffer_len: Size of @buffer.
*
* Returns bytes read on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_read_control(struct file *file, char __user *buffer,
const int buffer_len)
@@ -2094,6 +2099,8 @@ static int tomoyo_read_control(struct fi
* @buffer_len: Size of @buffer.
*
* Returns @buffer_len on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_write_control(struct file *file, const char __user *buffer,
const int buffer_len)
@@ -2144,11 +2151,14 @@ static int tomoyo_write_control(struct f
* @file: Pointer to "struct file".
*
* Releases memory and returns 0.
+ *
+ * Caller looses tomoyo_read_lock().
*/
static int tomoyo_close_control(struct file *file)
{
struct tomoyo_io_buffer *head = file->private_data;

+ tomoyo_read_unlock(head->reader_idx);
/* Release memory used for policy I/O. */
tomoyo_free(head->read_buf);
head->read_buf = NULL;
--- security-testing-2.6.orig/security/tomoyo/common.h
+++ security-testing-2.6/security/tomoyo/common.h
@@ -265,6 +265,8 @@ struct tomoyo_io_buffer {
int (*write) (struct tomoyo_io_buffer *);
/* Exclusive lock for this structure. */
struct mutex io_sem;
+ /* Index returned by tomoyo_read_lock(). */
+ int reader_idx;
/* The position currently reading from. */
struct list_head *read_var1;
/* Extra variables for reading. */
@@ -442,16 +444,28 @@ extern struct tomoyo_domain_info tomoyo_
* @cookie: the &struct list_head to use as a cookie.
* @head: the head for your list.
*
- * Same with list_for_each() except that this primitive uses @cookie
+ * Same with list_for_each_rcu() except that this primitive uses @cookie
* so that we can continue iteration.
* @cookie must be NULL when iteration starts, and @cookie will become
* NULL when iteration finishes.
*/
-#define list_for_each_cookie(pos, cookie, head) \
- for (({ if (!cookie) \
- cookie = head; }), \
- pos = (cookie)->next; \
- prefetch(pos->next), pos != (head) || ((cookie) = NULL); \
- (cookie) = pos, pos = pos->next)
+#define list_for_each_cookie(pos, cookie, head) \
+ for (({ if (!cookie) \
+ cookie = head; }), \
+ pos = rcu_dereference((cookie)->next); \
+ prefetch(pos->next), pos != (head) || ((cookie) = NULL); \
+ (cookie) = pos, pos = rcu_dereference(pos->next))
+
+extern struct srcu_struct tomoyo_ss;
+
+static inline int tomoyo_read_lock(void)
+{
+ return srcu_read_lock(&tomoyo_ss);
+}
+
+static inline void tomoyo_read_unlock(int idx)
+{
+ srcu_read_unlock(&tomoyo_ss, idx);
+}

#endif /* !defined(_SECURITY_TOMOYO_COMMON_H) */
--- security-testing-2.6.orig/security/tomoyo/domain.c
+++ security-testing-2.6/security/tomoyo/domain.c
@@ -217,6 +217,8 @@ static DECLARE_RWSEM(tomoyo_domain_initi
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_update_domain_initializer_entry(const char *domainname,
const char *program,
@@ -246,7 +248,7 @@ static int tomoyo_update_domain_initiali
if (!saved_program)
return -ENOMEM;
down_write(&tomoyo_domain_initializer_list_lock);
- list_for_each_entry(ptr, &tomoyo_domain_initializer_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_domain_initializer_list, list) {
if (ptr->is_not != is_not ||
ptr->domainname != saved_domainname ||
ptr->program != saved_program)
@@ -266,7 +268,7 @@ static int tomoyo_update_domain_initiali
new_entry->program = saved_program;
new_entry->is_not = is_not;
new_entry->is_last_name = is_last_name;
- list_add_tail(&new_entry->list, &tomoyo_domain_initializer_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_domain_initializer_list);
error = 0;
out:
up_write(&tomoyo_domain_initializer_list_lock);
@@ -279,13 +281,14 @@ static int tomoyo_update_domain_initiali
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Returns true on success, false otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
bool tomoyo_read_domain_initializer_policy(struct tomoyo_io_buffer *head)
{
struct list_head *pos;
bool done = true;

- down_read(&tomoyo_domain_initializer_list_lock);
list_for_each_cookie(pos, head->read_var2,
&tomoyo_domain_initializer_list) {
const char *no;
@@ -308,7 +311,6 @@ bool tomoyo_read_domain_initializer_poli
if (!done)
break;
}
- up_read(&tomoyo_domain_initializer_list_lock);
return done;
}

@@ -320,6 +322,8 @@ bool tomoyo_read_domain_initializer_poli
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
int tomoyo_write_domain_initializer_policy(char *data, const bool is_not,
const bool is_delete)
@@ -345,6 +349,8 @@ int tomoyo_write_domain_initializer_poli
*
* Returns true if executing @program reinitializes domain transition,
* false otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static bool tomoyo_is_domain_initializer(const struct tomoyo_path_info *
domainname,
@@ -355,8 +361,7 @@ static bool tomoyo_is_domain_initializer
struct tomoyo_domain_initializer_entry *ptr;
bool flag = false;

- down_read(&tomoyo_domain_initializer_list_lock);
- list_for_each_entry(ptr, &tomoyo_domain_initializer_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_domain_initializer_list, list) {
if (ptr->is_deleted)
continue;
if (ptr->domainname) {
@@ -376,7 +381,6 @@ static bool tomoyo_is_domain_initializer
}
flag = true;
}
- up_read(&tomoyo_domain_initializer_list_lock);
return flag;
}

@@ -430,6 +434,8 @@ static DECLARE_RWSEM(tomoyo_domain_keepe
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_update_domain_keeper_entry(const char *domainname,
const char *program,
@@ -459,7 +465,7 @@ static int tomoyo_update_domain_keeper_e
if (!saved_domainname)
return -ENOMEM;
down_write(&tomoyo_domain_keeper_list_lock);
- list_for_each_entry(ptr, &tomoyo_domain_keeper_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_domain_keeper_list, list) {
if (ptr->is_not != is_not ||
ptr->domainname != saved_domainname ||
ptr->program != saved_program)
@@ -479,7 +485,7 @@ static int tomoyo_update_domain_keeper_e
new_entry->program = saved_program;
new_entry->is_not = is_not;
new_entry->is_last_name = is_last_name;
- list_add_tail(&new_entry->list, &tomoyo_domain_keeper_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_domain_keeper_list);
error = 0;
out:
up_write(&tomoyo_domain_keeper_list_lock);
@@ -493,6 +499,7 @@ static int tomoyo_update_domain_keeper_e
* @is_not: True if it is "no_keep_domain" entry.
* @is_delete: True if it is a delete request.
*
+ * Caller holds tomoyo_read_lock().
*/
int tomoyo_write_domain_keeper_policy(char *data, const bool is_not,
const bool is_delete)
@@ -513,13 +520,14 @@ int tomoyo_write_domain_keeper_policy(ch
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Returns true on success, false otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
bool tomoyo_read_domain_keeper_policy(struct tomoyo_io_buffer *head)
{
struct list_head *pos;
bool done = true;

- down_read(&tomoyo_domain_keeper_list_lock);
list_for_each_cookie(pos, head->read_var2,
&tomoyo_domain_keeper_list) {
struct tomoyo_domain_keeper_entry *ptr;
@@ -542,7 +550,6 @@ bool tomoyo_read_domain_keeper_policy(st
if (!done)
break;
}
- up_read(&tomoyo_domain_keeper_list_lock);
return done;
}

@@ -555,6 +562,8 @@ bool tomoyo_read_domain_keeper_policy(st
*
* Returns true if executing @program supresses domain transition,
* false otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static bool tomoyo_is_domain_keeper(const struct tomoyo_path_info *domainname,
const struct tomoyo_path_info *program,
@@ -563,8 +572,7 @@ static bool tomoyo_is_domain_keeper(cons
struct tomoyo_domain_keeper_entry *ptr;
bool flag = false;

- down_read(&tomoyo_domain_keeper_list_lock);
- list_for_each_entry(ptr, &tomoyo_domain_keeper_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_domain_keeper_list, list) {
if (ptr->is_deleted)
continue;
if (!ptr->is_last_name) {
@@ -582,7 +590,6 @@ static bool tomoyo_is_domain_keeper(cons
}
flag = true;
}
- up_read(&tomoyo_domain_keeper_list_lock);
return flag;
}

@@ -627,6 +634,8 @@ static DECLARE_RWSEM(tomoyo_alias_list_l
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_update_alias_entry(const char *original_name,
const char *aliased_name,
@@ -646,7 +655,7 @@ static int tomoyo_update_alias_entry(con
if (!saved_original_name || !saved_aliased_name)
return -ENOMEM;
down_write(&tomoyo_alias_list_lock);
- list_for_each_entry(ptr, &tomoyo_alias_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_alias_list, list) {
if (ptr->original_name != saved_original_name ||
ptr->aliased_name != saved_aliased_name)
continue;
@@ -663,7 +672,7 @@ static int tomoyo_update_alias_entry(con
goto out;
new_entry->original_name = saved_original_name;
new_entry->aliased_name = saved_aliased_name;
- list_add_tail(&new_entry->list, &tomoyo_alias_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_alias_list);
error = 0;
out:
up_write(&tomoyo_alias_list_lock);
@@ -676,13 +685,14 @@ static int tomoyo_update_alias_entry(con
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Returns true on success, false otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
bool tomoyo_read_alias_policy(struct tomoyo_io_buffer *head)
{
struct list_head *pos;
bool done = true;

- down_read(&tomoyo_alias_list_lock);
list_for_each_cookie(pos, head->read_var2, &tomoyo_alias_list) {
struct tomoyo_alias_entry *ptr;

@@ -695,7 +705,6 @@ bool tomoyo_read_alias_policy(struct tom
if (!done)
break;
}
- up_read(&tomoyo_alias_list_lock);
return done;
}

@@ -706,6 +715,8 @@ bool tomoyo_read_alias_policy(struct tom
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
int tomoyo_write_alias_policy(char *data, const bool is_delete)
{
@@ -724,6 +735,8 @@ int tomoyo_write_alias_policy(char *data
* @profile: Profile number to assign if the domain was newly created.
*
* Returns pointer to "struct tomoyo_domain_info" on success, NULL otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
struct tomoyo_domain_info *tomoyo_find_or_assign_new_domain(const char *
domainname,
@@ -742,7 +755,7 @@ struct tomoyo_domain_info *tomoyo_find_o
if (!saved_domainname)
goto out;
/* Can I reuse memory of deleted domain? */
- list_for_each_entry(domain, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
struct task_struct *p;
struct tomoyo_acl_info *ptr;
bool flag;
@@ -760,7 +773,7 @@ struct tomoyo_domain_info *tomoyo_find_o
read_unlock(&tasklist_lock);
if (flag)
continue;
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
ptr->type |= TOMOYO_ACL_DELETED;
}
tomoyo_set_domain_flag(domain, true, domain->flags);
@@ -776,7 +789,7 @@ struct tomoyo_domain_info *tomoyo_find_o
INIT_LIST_HEAD(&domain->acl_info_list);
domain->domainname = saved_domainname;
domain->profile = profile;
- list_add_tail(&domain->list, &tomoyo_domain_list);
+ list_add_tail_rcu(&domain->list, &tomoyo_domain_list);
}
out:
up_write(&tomoyo_domain_list_lock);
@@ -789,6 +802,8 @@ struct tomoyo_domain_info *tomoyo_find_o
* @bprm: Pointer to "struct linux_binprm".
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
int tomoyo_find_next_domain(struct linux_binprm *bprm)
{
@@ -849,8 +864,7 @@ int tomoyo_find_next_domain(struct linux
if (tomoyo_pathcmp(&r, &s)) {
struct tomoyo_alias_entry *ptr;
/* Is this program allowed to be called via symbolic links? */
- down_read(&tomoyo_alias_list_lock);
- list_for_each_entry(ptr, &tomoyo_alias_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_alias_list, list) {
if (ptr->is_deleted ||
tomoyo_pathcmp(&r, ptr->original_name) ||
tomoyo_pathcmp(&s, ptr->aliased_name))
@@ -861,7 +875,6 @@ int tomoyo_find_next_domain(struct linux
tomoyo_fill_path_info(&r);
break;
}
- up_read(&tomoyo_alias_list_lock);
}

/* Check execute permission. */
@@ -892,9 +905,7 @@ int tomoyo_find_next_domain(struct linux
}
if (domain || strlen(new_domain_name) >= TOMOYO_MAX_PATHNAME_LEN)
goto done;
- down_read(&tomoyo_domain_list_lock);
domain = tomoyo_find_domain(new_domain_name);
- up_read(&tomoyo_domain_list_lock);
if (domain)
goto done;
if (is_enforce)
--- security-testing-2.6.orig/security/tomoyo/file.c
+++ security-testing-2.6/security/tomoyo/file.c
@@ -205,6 +205,8 @@ static DECLARE_RWSEM(tomoyo_globally_rea
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_update_globally_readable_entry(const char *filename,
const bool is_delete)
@@ -220,7 +222,7 @@ static int tomoyo_update_globally_readab
if (!saved_filename)
return -ENOMEM;
down_write(&tomoyo_globally_readable_list_lock);
- list_for_each_entry(ptr, &tomoyo_globally_readable_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_globally_readable_list, list) {
if (ptr->filename != saved_filename)
continue;
ptr->is_deleted = is_delete;
@@ -235,7 +237,7 @@ static int tomoyo_update_globally_readab
if (!new_entry)
goto out;
new_entry->filename = saved_filename;
- list_add_tail(&new_entry->list, &tomoyo_globally_readable_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_globally_readable_list);
error = 0;
out:
up_write(&tomoyo_globally_readable_list_lock);
@@ -248,21 +250,22 @@ static int tomoyo_update_globally_readab
* @filename: The filename to check.
*
* Returns true if any domain can open @filename for reading, false otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static bool tomoyo_is_globally_readable_file(const struct tomoyo_path_info *
filename)
{
struct tomoyo_globally_readable_file_entry *ptr;
bool found = false;
- down_read(&tomoyo_globally_readable_list_lock);
- list_for_each_entry(ptr, &tomoyo_globally_readable_list, list) {
+
+ list_for_each_entry_rcu(ptr, &tomoyo_globally_readable_list, list) {
if (!ptr->is_deleted &&
tomoyo_path_matches_pattern(filename, ptr->filename)) {
found = true;
break;
}
}
- up_read(&tomoyo_globally_readable_list_lock);
return found;
}

@@ -273,6 +276,8 @@ static bool tomoyo_is_globally_readable_
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
int tomoyo_write_globally_readable_policy(char *data, const bool is_delete)
{
@@ -285,13 +290,14 @@ int tomoyo_write_globally_readable_polic
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Returns true on success, false otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
bool tomoyo_read_globally_readable_policy(struct tomoyo_io_buffer *head)
{
struct list_head *pos;
bool done = true;

- down_read(&tomoyo_globally_readable_list_lock);
list_for_each_cookie(pos, head->read_var2,
&tomoyo_globally_readable_list) {
struct tomoyo_globally_readable_file_entry *ptr;
@@ -305,7 +311,6 @@ bool tomoyo_read_globally_readable_polic
if (!done)
break;
}
- up_read(&tomoyo_globally_readable_list_lock);
return done;
}

@@ -348,6 +353,8 @@ static DECLARE_RWSEM(tomoyo_pattern_list
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_update_file_pattern_entry(const char *pattern,
const bool is_delete)
@@ -363,7 +370,7 @@ static int tomoyo_update_file_pattern_en
if (!saved_pattern)
return -ENOMEM;
down_write(&tomoyo_pattern_list_lock);
- list_for_each_entry(ptr, &tomoyo_pattern_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_pattern_list, list) {
if (saved_pattern != ptr->pattern)
continue;
ptr->is_deleted = is_delete;
@@ -378,7 +385,7 @@ static int tomoyo_update_file_pattern_en
if (!new_entry)
goto out;
new_entry->pattern = saved_pattern;
- list_add_tail(&new_entry->list, &tomoyo_pattern_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_pattern_list);
error = 0;
out:
up_write(&tomoyo_pattern_list_lock);
@@ -391,6 +398,8 @@ static int tomoyo_update_file_pattern_en
* @filename: The filename to find patterned pathname.
*
* Returns pointer to pathname pattern if matched, @filename otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static const struct tomoyo_path_info *
tomoyo_get_file_pattern(const struct tomoyo_path_info *filename)
@@ -398,8 +407,7 @@ tomoyo_get_file_pattern(const struct tom
struct tomoyo_pattern_entry *ptr;
const struct tomoyo_path_info *pattern = NULL;

- down_read(&tomoyo_pattern_list_lock);
- list_for_each_entry(ptr, &tomoyo_pattern_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_pattern_list, list) {
if (ptr->is_deleted)
continue;
if (!tomoyo_path_matches_pattern(filename, ptr->pattern))
@@ -412,7 +420,6 @@ tomoyo_get_file_pattern(const struct tom
break;
}
}
- up_read(&tomoyo_pattern_list_lock);
if (pattern)
filename = pattern;
return filename;
@@ -425,6 +432,8 @@ tomoyo_get_file_pattern(const struct tom
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
int tomoyo_write_pattern_policy(char *data, const bool is_delete)
{
@@ -437,13 +446,14 @@ int tomoyo_write_pattern_policy(char *da
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Returns true on success, false otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
bool tomoyo_read_file_pattern(struct tomoyo_io_buffer *head)
{
struct list_head *pos;
bool done = true;

- down_read(&tomoyo_pattern_list_lock);
list_for_each_cookie(pos, head->read_var2, &tomoyo_pattern_list) {
struct tomoyo_pattern_entry *ptr;
ptr = list_entry(pos, struct tomoyo_pattern_entry, list);
@@ -454,7 +464,6 @@ bool tomoyo_read_file_pattern(struct tom
if (!done)
break;
}
- up_read(&tomoyo_pattern_list_lock);
return done;
}

@@ -497,6 +506,8 @@ static DECLARE_RWSEM(tomoyo_no_rewrite_l
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_update_no_rewrite_entry(const char *pattern,
const bool is_delete)
@@ -511,7 +522,7 @@ static int tomoyo_update_no_rewrite_entr
if (!saved_pattern)
return -ENOMEM;
down_write(&tomoyo_no_rewrite_list_lock);
- list_for_each_entry(ptr, &tomoyo_no_rewrite_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_no_rewrite_list, list) {
if (ptr->pattern != saved_pattern)
continue;
ptr->is_deleted = is_delete;
@@ -526,7 +537,7 @@ static int tomoyo_update_no_rewrite_entr
if (!new_entry)
goto out;
new_entry->pattern = saved_pattern;
- list_add_tail(&new_entry->list, &tomoyo_no_rewrite_list);
+ list_add_tail_rcu(&new_entry->list, &tomoyo_no_rewrite_list);
error = 0;
out:
up_write(&tomoyo_no_rewrite_list_lock);
@@ -540,14 +551,15 @@ static int tomoyo_update_no_rewrite_entr
*
* Returns true if @filename is specified by "deny_rewrite" directive,
* false otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static bool tomoyo_is_no_rewrite_file(const struct tomoyo_path_info *filename)
{
struct tomoyo_no_rewrite_entry *ptr;
bool found = false;

- down_read(&tomoyo_no_rewrite_list_lock);
- list_for_each_entry(ptr, &tomoyo_no_rewrite_list, list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_no_rewrite_list, list) {
if (ptr->is_deleted)
continue;
if (!tomoyo_path_matches_pattern(filename, ptr->pattern))
@@ -555,7 +567,6 @@ static bool tomoyo_is_no_rewrite_file(co
found = true;
break;
}
- up_read(&tomoyo_no_rewrite_list_lock);
return found;
}

@@ -566,6 +577,8 @@ static bool tomoyo_is_no_rewrite_file(co
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
int tomoyo_write_no_rewrite_policy(char *data, const bool is_delete)
{
@@ -578,13 +591,14 @@ int tomoyo_write_no_rewrite_policy(char
* @head: Pointer to "struct tomoyo_io_buffer".
*
* Returns true on success, false otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
bool tomoyo_read_no_rewrite_policy(struct tomoyo_io_buffer *head)
{
struct list_head *pos;
bool done = true;

- down_read(&tomoyo_no_rewrite_list_lock);
list_for_each_cookie(pos, head->read_var2, &tomoyo_no_rewrite_list) {
struct tomoyo_no_rewrite_entry *ptr;
ptr = list_entry(pos, struct tomoyo_no_rewrite_entry, list);
@@ -595,7 +609,6 @@ bool tomoyo_read_no_rewrite_policy(struc
if (!done)
break;
}
- up_read(&tomoyo_no_rewrite_list_lock);
return done;
}

@@ -613,6 +626,8 @@ bool tomoyo_read_no_rewrite_policy(struc
* Current policy syntax uses "allow_read/write" instead of "6",
* "allow_read" instead of "4", "allow_write" instead of "2",
* "allow_execute" instead of "1".
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_update_file_acl(const char *filename, u8 perm,
struct tomoyo_domain_info * const domain,
@@ -650,6 +665,8 @@ static int tomoyo_update_file_acl(const
* @may_use_pattern: True if patterned ACL is permitted.
*
* Returns 0 on success, -EPERM otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_check_single_path_acl2(const struct tomoyo_domain_info *
domain,
@@ -661,8 +678,7 @@ static int tomoyo_check_single_path_acl2
struct tomoyo_acl_info *ptr;
int error = -EPERM;

- down_read(&tomoyo_domain_acl_info_list_lock);
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
struct tomoyo_single_path_acl_record *acl;
if (tomoyo_acl_type2(ptr) != TOMOYO_TYPE_SINGLE_PATH_ACL)
continue;
@@ -680,7 +696,6 @@ static int tomoyo_check_single_path_acl2
error = 0;
break;
}
- up_read(&tomoyo_domain_acl_info_list_lock);
return error;
}

@@ -692,6 +707,8 @@ static int tomoyo_check_single_path_acl2
* @operation: Mode ("read" or "write" or "read/write" or "execute").
*
* Returns 0 on success, -EPERM otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_check_file_acl(const struct tomoyo_domain_info *domain,
const struct tomoyo_path_info *filename,
@@ -725,6 +742,8 @@ static int tomoyo_check_file_acl(const s
* @mode: Access control mode.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_check_file_perm2(struct tomoyo_domain_info * const domain,
const struct tomoyo_path_info *filename,
@@ -778,6 +797,8 @@ static int tomoyo_check_file_perm2(struc
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
int tomoyo_write_file_policy(char *data, struct tomoyo_domain_info *domain,
const bool is_delete)
@@ -825,6 +846,8 @@ int tomoyo_write_file_policy(char *data,
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_update_single_path_acl(const u8 type, const char *filename,
struct tomoyo_domain_info *
@@ -848,7 +871,7 @@ static int tomoyo_update_single_path_acl
down_write(&tomoyo_domain_acl_info_list_lock);
if (is_delete)
goto delete;
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
if (tomoyo_acl_type1(ptr) != TOMOYO_TYPE_SINGLE_PATH_ACL)
continue;
acl = container_of(ptr, struct tomoyo_single_path_acl_record,
@@ -875,12 +898,12 @@ static int tomoyo_update_single_path_acl
if (perm == (1 << TOMOYO_TYPE_READ_WRITE_ACL))
acl->perm |= rw_mask;
acl->filename = saved_filename;
- list_add_tail(&acl->head.list, &domain->acl_info_list);
+ list_add_tail_rcu(&acl->head.list, &domain->acl_info_list);
error = 0;
goto out;
delete:
error = -ENOENT;
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
if (tomoyo_acl_type2(ptr) != TOMOYO_TYPE_SINGLE_PATH_ACL)
continue;
acl = container_of(ptr, struct tomoyo_single_path_acl_record,
@@ -912,6 +935,8 @@ static int tomoyo_update_single_path_acl
* @is_delete: True if it is a delete request.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_update_double_path_acl(const u8 type, const char *filename1,
const char *filename2,
@@ -937,7 +962,7 @@ static int tomoyo_update_double_path_acl
down_write(&tomoyo_domain_acl_info_list_lock);
if (is_delete)
goto delete;
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
if (tomoyo_acl_type1(ptr) != TOMOYO_TYPE_DOUBLE_PATH_ACL)
continue;
acl = container_of(ptr, struct tomoyo_double_path_acl_record,
@@ -960,12 +985,12 @@ static int tomoyo_update_double_path_acl
acl->perm = perm;
acl->filename1 = saved_filename1;
acl->filename2 = saved_filename2;
- list_add_tail(&acl->head.list, &domain->acl_info_list);
+ list_add_tail_rcu(&acl->head.list, &domain->acl_info_list);
error = 0;
goto out;
delete:
error = -ENOENT;
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
if (tomoyo_acl_type2(ptr) != TOMOYO_TYPE_DOUBLE_PATH_ACL)
continue;
acl = container_of(ptr, struct tomoyo_double_path_acl_record,
@@ -992,6 +1017,8 @@ static int tomoyo_update_double_path_acl
* @filename: Filename to check.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_check_single_path_acl(struct tomoyo_domain_info *domain,
const u8 type,
@@ -1011,6 +1038,8 @@ static int tomoyo_check_single_path_acl(
* @filename2: Second filename to check.
*
* Returns 0 on success, -EPERM otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_check_double_path_acl(const struct tomoyo_domain_info *domain,
const u8 type,
@@ -1025,8 +1054,7 @@ static int tomoyo_check_double_path_acl(

if (!tomoyo_check_flags(domain, TOMOYO_MAC_FOR_FILE))
return 0;
- down_read(&tomoyo_domain_acl_info_list_lock);
- list_for_each_entry(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
struct tomoyo_double_path_acl_record *acl;
if (tomoyo_acl_type2(ptr) != TOMOYO_TYPE_DOUBLE_PATH_ACL)
continue;
@@ -1041,7 +1069,6 @@ static int tomoyo_check_double_path_acl(
error = 0;
break;
}
- up_read(&tomoyo_domain_acl_info_list_lock);
return error;
}

@@ -1054,6 +1081,8 @@ static int tomoyo_check_double_path_acl(
* @mode: Access control mode.
*
* Returns 0 on success, negative value otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
static int tomoyo_check_single_path_permission2(struct tomoyo_domain_info *
const domain, u8 operation,
@@ -1123,6 +1152,8 @@ int tomoyo_check_file_perm(struct tomoyo
* @filename: Check permission for "execute".
*
* Returns 0 on success, negativevalue otherwise.
+ *
+ * Caller holds tomoyo_read_lock().
*/
int tomoyo_check_exec_perm(struct tomoyo_domain_info *domain,
const struct tomoyo_path_info *filename)
@@ -1151,6 +1182,7 @@ int tomoyo_check_open_permission(struct
struct tomoyo_path_info *buf;
const u8 mode = tomoyo_check_flags(domain, TOMOYO_MAC_FOR_FILE);
const bool is_enforce = (mode == 3);
+ int idx;

if (!mode || !path->mnt)
return 0;
@@ -1162,6 +1194,7 @@ int tomoyo_check_open_permission(struct
* don't call me.
*/
return 0;
+ idx = tomoyo_read_lock();
buf = tomoyo_get_path(path);
if (!buf)
goto out;
@@ -1187,6 +1220,7 @@ int tomoyo_check_open_permission(struct
buf, mode);
out:
tomoyo_free(buf);
+ tomoyo_read_unlock(idx);
if (!is_enforce)
error = 0;
return error;
@@ -1208,9 +1242,11 @@ int tomoyo_check_1path_perm(struct tomoy
struct tomoyo_path_info *buf;
const u8 mode = tomoyo_check_flags(domain, TOMOYO_MAC_FOR_FILE);
const bool is_enforce = (mode == 3);
+ int idx;

if (!mode || !path->mnt)
return 0;
+ idx = tomoyo_read_lock();
buf = tomoyo_get_path(path);
if (!buf)
goto out;
@@ -1229,6 +1265,7 @@ int tomoyo_check_1path_perm(struct tomoy
mode);
out:
tomoyo_free(buf);
+ tomoyo_read_unlock(idx);
if (!is_enforce)
error = 0;
return error;
@@ -1249,9 +1286,12 @@ int tomoyo_check_rewrite_permission(stru
const u8 mode = tomoyo_check_flags(domain, TOMOYO_MAC_FOR_FILE);
const bool is_enforce = (mode == 3);
struct tomoyo_path_info *buf;
+ int idx;

if (!mode || !filp->f_path.mnt)
return 0;
+
+ idx = tomoyo_read_lock();
buf = tomoyo_get_path(&filp->f_path);
if (!buf)
goto out;
@@ -1264,6 +1304,7 @@ int tomoyo_check_rewrite_permission(stru
buf, mode);
out:
tomoyo_free(buf);
+ tomoyo_read_unlock(idx);
if (!is_enforce)
error = 0;
return error;
@@ -1288,9 +1329,11 @@ int tomoyo_check_2path_perm(struct tomoy
const u8 mode = tomoyo_check_flags(domain, TOMOYO_MAC_FOR_FILE);
const bool is_enforce = (mode == 3);
const char *msg;
+ int idx;

if (!mode || !path1->mnt || !path2->mnt)
return 0;
+ idx = tomoyo_read_lock();
buf1 = tomoyo_get_path(path1);
buf2 = tomoyo_get_path(path2);
if (!buf1 || !buf2)
@@ -1329,6 +1372,7 @@ int tomoyo_check_2path_perm(struct tomoy
out:
tomoyo_free(buf1);
tomoyo_free(buf2);
+ tomoyo_read_unlock(idx);
if (!is_enforce)
error = 0;
return error;
--- security-testing-2.6.orig/security/tomoyo/realpath.c
+++ security-testing-2.6/security/tomoyo/realpath.c
@@ -392,11 +392,13 @@ void __init tomoyo_realpath_init(void)
INIT_LIST_HEAD(&tomoyo_name_list[i]);
INIT_LIST_HEAD(&tomoyo_kernel_domain.acl_info_list);
tomoyo_kernel_domain.domainname = tomoyo_save_name(TOMOYO_ROOT_NAME);
- list_add_tail(&tomoyo_kernel_domain.list, &tomoyo_domain_list);
- down_read(&tomoyo_domain_list_lock);
+ /*
+ * tomoyo_read_lock() is not needed because this function is
+ * called before the first "delete" request.
+ */
+ list_add_tail_rcu(&tomoyo_kernel_domain.list, &tomoyo_domain_list);
if (tomoyo_find_domain(TOMOYO_ROOT_NAME) != &tomoyo_kernel_domain)
panic("Can't register tomoyo_kernel_domain");
- up_read(&tomoyo_domain_list_lock);
}

/* Memory allocated for temporary purpose. */
--- security-testing-2.6.orig/security/tomoyo/tomoyo.c
+++ security-testing-2.6/security/tomoyo/tomoyo.c
@@ -76,8 +76,18 @@ static int tomoyo_bprm_check_security(st
* Execute permission is checked against pathname passed to do_execve()
* using current domain.
*/
- if (!domain)
- return tomoyo_find_next_domain(bprm);
+ if (!domain) {
+ /*
+ * We will need to protect whole execve() operation when GC
+ * starts kfree()ing "struct tomoyo_domain_info" because
+ * bprm->cred->security points to "struct tomoyo_domain_info"
+ * but "struct tomoyo_domain_info" does not have a refcounter.
+ */
+ const int idx = tomoyo_read_lock();
+ const int err = tomoyo_find_next_domain(bprm);
+ tomoyo_read_unlock(idx);
+ return err;
+ }
/*
* Read permission is checked against interpreters using next domain.
* '1' is the result of open_to_namei_flags(O_RDONLY).
@@ -297,6 +307,9 @@ static struct security_operations tomoyo
.path_rename = tomoyo_path_rename,
};

+/* Lock for GC. */
+struct srcu_struct tomoyo_ss;
+
static int __init tomoyo_init(void)
{
struct cred *cred = (struct cred *) current_cred();
@@ -304,7 +317,8 @@ static int __init tomoyo_init(void)
if (!security_module_enable(&tomoyo_security_ops))
return 0;
/* register ourselves with the security framework */
- if (register_security(&tomoyo_security_ops))
+ if (register_security(&tomoyo_security_ops) ||
+ init_srcu_struct(&tomoyo_ss))
panic("Failure registering TOMOYO Linux");
printk(KERN_INFO "TOMOYO Linux initialized\n");
cred->security = &tomoyo_kernel_domain;