Commit 87335b61 authored by Marco Elver's avatar Marco Elver Committed by Peter Zijlstra
Browse files

security/tomoyo: Enable context analysis 



Enable context analysis for security/tomoyo.

This demonstrates a larger conversion to use Clang's context
analysis. The benefit is additional static checking of locking rules,
along with better documentation.

Tomoyo makes use of several synchronization primitives, yet its clear
design made it relatively straightforward to enable context analysis.

One notable finding was:

  security/tomoyo/gc.c:664:20: error: reading variable 'write_buf' requires holding mutex '&tomoyo_io_buffer::io_sem'
    664 |                 is_write = head->write_buf != NULL;

For which Tetsuo writes:

  "Good catch. This should be data_race(), for tomoyo_write_control()
   might concurrently update head->write_buf from non-NULL to non-NULL
   with head->io_sem held."

Signed-off-by: default avatarMarco Elver <elver@google.com>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Link: https://patch.msgid.link/20251219154418.3592607-35-elver@google.com
parent 8ec56d9a

security/tomoyo/Makefile

+2 −0
Original line number Diff line number Diff line 
# SPDX-License-Identifier: GPL-2.0

CONTEXT_ANALYSIS := y



obj-y = audit.o common.o condition.o domain.o environ.o file.o gc.o group.o load_policy.o memory.o mount.o network.o realpath.o securityfs_if.o tomoyo.o util.o



targets += builtin-policy.h

security/tomoyo/common.c

+48 −4
Original line number Diff line number Diff line
@@ -268,6 +268,7 @@ static void tomoyo_io_printf(struct tomoyo_io_buffer *head, const char *fmt, 
 */

static void tomoyo_io_printf(struct tomoyo_io_buffer *head, const char *fmt,

			     ...)

	__must_hold(&head->io_sem)

{

	va_list args;

	size_t len;
@@ -416,8 +417,9 @@ static void tomoyo_print_name_union_quoted(struct tomoyo_io_buffer *head, 
 *

 * Returns nothing.

 */

static void tomoyo_print_number_union_nospace

(struct tomoyo_io_buffer *head, const struct tomoyo_number_union *ptr)

static void

tomoyo_print_number_union_nospace(struct tomoyo_io_buffer *head, const struct tomoyo_number_union *ptr)

	__must_hold(&head->io_sem)

{

	if (ptr->group) {

		tomoyo_set_string(head, "@");
@@ -466,6 +468,7 @@ static void tomoyo_print_number_union_nospace 
 */

static void tomoyo_print_number_union(struct tomoyo_io_buffer *head,

				      const struct tomoyo_number_union *ptr)

	__must_hold(&head->io_sem)

{

	tomoyo_set_space(head);

	tomoyo_print_number_union_nospace(head, ptr);
@@ -664,6 +667,7 @@ static int tomoyo_set_mode(char *name, const char *value, 
 * Returns 0 on success, negative value otherwise.

 */

static int tomoyo_write_profile(struct tomoyo_io_buffer *head)

	__must_hold(&head->io_sem)

{

	char *data = head->write_buf;

	unsigned int i;
@@ -719,6 +723,7 @@ static int tomoyo_write_profile(struct tomoyo_io_buffer *head) 
 * Caller prints functionality's name.

 */

static void tomoyo_print_config(struct tomoyo_io_buffer *head, const u8 config)

	__must_hold(&head->io_sem)

{

	tomoyo_io_printf(head, "={ mode=%s grant_log=%s reject_log=%s }\n",

			 tomoyo_mode[config & 3],
@@ -734,6 +739,7 @@ static void tomoyo_print_config(struct tomoyo_io_buffer *head, const u8 config) 
 * Returns nothing.

 */

static void tomoyo_read_profile(struct tomoyo_io_buffer *head)

	__must_hold(&head->io_sem)

{

	u8 index;

	struct tomoyo_policy_namespace *ns =
@@ -852,6 +858,7 @@ static bool tomoyo_same_manager(const struct tomoyo_acl_head *a, 
 */

static int tomoyo_update_manager_entry(const char *manager,

				       const bool is_delete)

	__must_hold_shared(&tomoyo_ss)

{

	struct tomoyo_manager e = { };

	struct tomoyo_acl_param param = {
@@ -883,6 +890,8 @@ static int tomoyo_update_manager_entry(const char *manager, 
 * Caller holds tomoyo_read_lock().

 */

static int tomoyo_write_manager(struct tomoyo_io_buffer *head)

	__must_hold_shared(&tomoyo_ss)

	__must_hold(&head->io_sem)

{

	char *data = head->write_buf;
@@ -901,6 +910,7 @@ static int tomoyo_write_manager(struct tomoyo_io_buffer *head) 
 * Caller holds tomoyo_read_lock().

 */

static void tomoyo_read_manager(struct tomoyo_io_buffer *head)

	__must_hold_shared(&tomoyo_ss)

{

	if (head->r.eof)

		return;
@@ -927,6 +937,7 @@ static void tomoyo_read_manager(struct tomoyo_io_buffer *head) 
 * Caller holds tomoyo_read_lock().

 */

static bool tomoyo_manager(void)

	__must_hold_shared(&tomoyo_ss)

{

	struct tomoyo_manager *ptr;

	const char *exe;
@@ -981,6 +992,8 @@ static struct tomoyo_domain_info *tomoyo_find_domain_by_qid 
 */

static bool tomoyo_select_domain(struct tomoyo_io_buffer *head,

				 const char *data)

	__must_hold_shared(&tomoyo_ss)

	__must_hold(&head->io_sem)

{

	unsigned int pid;

	struct tomoyo_domain_info *domain = NULL;
@@ -1051,6 +1064,7 @@ static bool tomoyo_same_task_acl(const struct tomoyo_acl_info *a, 
 * Caller holds tomoyo_read_lock().

 */

static int tomoyo_write_task(struct tomoyo_acl_param *param)

	__must_hold_shared(&tomoyo_ss)

{

	int error = -EINVAL;
@@ -1079,6 +1093,7 @@ static int tomoyo_write_task(struct tomoyo_acl_param *param) 
 * Caller holds tomoyo_read_lock().

 */

static int tomoyo_delete_domain(char *domainname)

	__must_hold_shared(&tomoyo_ss)

{

	struct tomoyo_domain_info *domain;

	struct tomoyo_path_info name;
@@ -1118,6 +1133,7 @@ static int tomoyo_delete_domain(char *domainname) 
static int tomoyo_write_domain2(struct tomoyo_policy_namespace *ns,

				struct list_head *list, char *data,

				const bool is_delete)

	__must_hold_shared(&tomoyo_ss)

{

	struct tomoyo_acl_param param = {

		.ns = ns,
@@ -1162,6 +1178,8 @@ const char * const tomoyo_dif[TOMOYO_MAX_DOMAIN_INFO_FLAGS] = { 
 * Caller holds tomoyo_read_lock().

 */

static int tomoyo_write_domain(struct tomoyo_io_buffer *head)

	__must_hold_shared(&tomoyo_ss)

	__must_hold(&head->io_sem)

{

	char *data = head->write_buf;

	struct tomoyo_policy_namespace *ns;
@@ -1223,6 +1241,7 @@ static int tomoyo_write_domain(struct tomoyo_io_buffer *head) 
 */

static bool tomoyo_print_condition(struct tomoyo_io_buffer *head,

				   const struct tomoyo_condition *cond)

	__must_hold(&head->io_sem)

{

	switch (head->r.cond_step) {

	case 0:
@@ -1364,6 +1383,7 @@ static bool tomoyo_print_condition(struct tomoyo_io_buffer *head, 
 */

static void tomoyo_set_group(struct tomoyo_io_buffer *head,

			     const char *category)

	__must_hold(&head->io_sem)

{

	if (head->type == TOMOYO_EXCEPTIONPOLICY) {

		tomoyo_print_namespace(head);
@@ -1383,6 +1403,7 @@ static void tomoyo_set_group(struct tomoyo_io_buffer *head, 
 */

static bool tomoyo_print_entry(struct tomoyo_io_buffer *head,

			       struct tomoyo_acl_info *acl)

	__must_hold(&head->io_sem)

{

	const u8 acl_type = acl->type;

	bool first = true;
@@ -1588,6 +1609,8 @@ static bool tomoyo_print_entry(struct tomoyo_io_buffer *head, 
 */

static bool tomoyo_read_domain2(struct tomoyo_io_buffer *head,

				struct list_head *list)

	__must_hold_shared(&tomoyo_ss)

	__must_hold(&head->io_sem)

{

	list_for_each_cookie(head->r.acl, list) {

		struct tomoyo_acl_info *ptr =
@@ -1608,6 +1631,8 @@ static bool tomoyo_read_domain2(struct tomoyo_io_buffer *head, 
 * Caller holds tomoyo_read_lock().

 */

static void tomoyo_read_domain(struct tomoyo_io_buffer *head)

	__must_hold_shared(&tomoyo_ss)

	__must_hold(&head->io_sem)

{

	if (head->r.eof)

		return;
@@ -1686,6 +1711,7 @@ static int tomoyo_write_pid(struct tomoyo_io_buffer *head) 
 * using read()/write() interface rather than sysctl() interface.

 */

static void tomoyo_read_pid(struct tomoyo_io_buffer *head)

	__must_hold(&head->io_sem)

{

	char *buf = head->write_buf;

	bool global_pid = false;
@@ -1746,6 +1772,8 @@ static const char *tomoyo_group_name[TOMOYO_MAX_GROUP] = { 
 * Caller holds tomoyo_read_lock().

 */

static int tomoyo_write_exception(struct tomoyo_io_buffer *head)

	__must_hold_shared(&tomoyo_ss)

	__must_hold(&head->io_sem)

{

	const bool is_delete = head->w.is_delete;

	struct tomoyo_acl_param param = {
@@ -1787,6 +1815,8 @@ static int tomoyo_write_exception(struct tomoyo_io_buffer *head) 
 * Caller holds tomoyo_read_lock().

 */

static bool tomoyo_read_group(struct tomoyo_io_buffer *head, const int idx)

	__must_hold_shared(&tomoyo_ss)

	__must_hold(&head->io_sem)

{

	struct tomoyo_policy_namespace *ns =

		container_of(head->r.ns, typeof(*ns), namespace_list);
@@ -1846,6 +1876,7 @@ static bool tomoyo_read_group(struct tomoyo_io_buffer *head, const int idx) 
 * Caller holds tomoyo_read_lock().

 */

static bool tomoyo_read_policy(struct tomoyo_io_buffer *head, const int idx)

	__must_hold_shared(&tomoyo_ss)

{

	struct tomoyo_policy_namespace *ns =

		container_of(head->r.ns, typeof(*ns), namespace_list);
@@ -1906,6 +1937,8 @@ static bool tomoyo_read_policy(struct tomoyo_io_buffer *head, const int idx) 
 * Caller holds tomoyo_read_lock().

 */

static void tomoyo_read_exception(struct tomoyo_io_buffer *head)

	__must_hold_shared(&tomoyo_ss)

	__must_hold(&head->io_sem)

{

	struct tomoyo_policy_namespace *ns =

		container_of(head->r.ns, typeof(*ns), namespace_list);
@@ -2097,6 +2130,7 @@ static void tomoyo_patternize_path(char *buffer, const int len, char *entry) 
 * Returns nothing.

 */

static void tomoyo_add_entry(struct tomoyo_domain_info *domain, char *header)

	__must_hold_shared(&tomoyo_ss)

{

	char *buffer;

	char *realpath = NULL;
@@ -2301,6 +2335,7 @@ static __poll_t tomoyo_poll_query(struct file *file, poll_table *wait) 
 * @head: Pointer to "struct tomoyo_io_buffer".

 */

static void tomoyo_read_query(struct tomoyo_io_buffer *head)

	__must_hold(&head->io_sem)

{

	struct list_head *tmp;

	unsigned int pos = 0;
@@ -2362,6 +2397,7 @@ static void tomoyo_read_query(struct tomoyo_io_buffer *head) 
 * Returns 0 on success, -EINVAL otherwise.

 */

static int tomoyo_write_answer(struct tomoyo_io_buffer *head)

	__must_hold(&head->io_sem)

{

	char *data = head->write_buf;

	struct list_head *tmp;
@@ -2401,6 +2437,7 @@ static int tomoyo_write_answer(struct tomoyo_io_buffer *head) 
 * Returns version information.

 */

static void tomoyo_read_version(struct tomoyo_io_buffer *head)

	__must_hold(&head->io_sem)

{

	if (!head->r.eof) {

		tomoyo_io_printf(head, "2.6.0");
@@ -2449,6 +2486,7 @@ void tomoyo_update_stat(const u8 index) 
 * Returns nothing.

 */

static void tomoyo_read_stat(struct tomoyo_io_buffer *head)

	__must_hold(&head->io_sem)

{

	u8 i;

	unsigned int total = 0;
@@ -2493,6 +2531,7 @@ static void tomoyo_read_stat(struct tomoyo_io_buffer *head) 
 * Returns 0.

 */

static int tomoyo_write_stat(struct tomoyo_io_buffer *head)

	__must_hold(&head->io_sem)

{

	char *data = head->write_buf;

	u8 i;
@@ -2717,6 +2756,8 @@ ssize_t tomoyo_read_control(struct tomoyo_io_buffer *head, char __user *buffer, 
 * Caller holds tomoyo_read_lock().

 */

static int tomoyo_parse_policy(struct tomoyo_io_buffer *head, char *line)

	__must_hold_shared(&tomoyo_ss)

	__must_hold(&head->io_sem)

{

	/* Delete request? */

	head->w.is_delete = !strncmp(line, "delete ", 7);
@@ -2969,8 +3010,11 @@ void __init tomoyo_load_builtin_policy(void) 
				break;

			*end = '\0';

			tomoyo_normalize_line(start);

			/* head is stack-local and not shared. */

			context_unsafe(

				head.write_buf = start;

				tomoyo_parse_policy(&head, start);

			);

			start = end + 1;

		}

	}

security/tomoyo/common.h

+40 −37
Original line number Diff line number Diff line
@@ -827,13 +827,13 @@ struct tomoyo_io_buffer { 
		bool is_delete;

	} w;

	/* Buffer for reading.                  */

	char *read_buf;

	char *read_buf		__guarded_by(&io_sem);

	/* Size of read buffer.                 */

	size_t readbuf_size;

	size_t readbuf_size	__guarded_by(&io_sem);

	/* Buffer for writing.                  */

	char *write_buf;

	char *write_buf		__guarded_by(&io_sem);

	/* Size of write buffer.                */

	size_t writebuf_size;

	size_t writebuf_size	__guarded_by(&io_sem);

	/* Type of this interface.              */

	enum tomoyo_securityfs_interface_index type;

	/* Users counter protected by tomoyo_io_buffer_list_lock. */
@@ -922,6 +922,35 @@ struct tomoyo_task { 
	struct tomoyo_domain_info *old_domain_info;

};



/********** External variable definitions. **********/



extern bool tomoyo_policy_loaded;

extern int tomoyo_enabled;

extern const char * const tomoyo_condition_keyword

[TOMOYO_MAX_CONDITION_KEYWORD];

extern const char * const tomoyo_dif[TOMOYO_MAX_DOMAIN_INFO_FLAGS];

extern const char * const tomoyo_mac_keywords[TOMOYO_MAX_MAC_INDEX

					      + TOMOYO_MAX_MAC_CATEGORY_INDEX];

extern const char * const tomoyo_mode[TOMOYO_CONFIG_MAX_MODE];

extern const char * const tomoyo_path_keyword[TOMOYO_MAX_PATH_OPERATION];

extern const char * const tomoyo_proto_keyword[TOMOYO_SOCK_MAX];

extern const char * const tomoyo_socket_keyword[TOMOYO_MAX_NETWORK_OPERATION];

extern const u8 tomoyo_index2category[TOMOYO_MAX_MAC_INDEX];

extern const u8 tomoyo_pn2mac[TOMOYO_MAX_PATH_NUMBER_OPERATION];

extern const u8 tomoyo_pnnn2mac[TOMOYO_MAX_MKDEV_OPERATION];

extern const u8 tomoyo_pp2mac[TOMOYO_MAX_PATH2_OPERATION];

extern struct list_head tomoyo_condition_list;

extern struct list_head tomoyo_domain_list;

extern struct list_head tomoyo_name_list[TOMOYO_MAX_HASH];

extern struct list_head tomoyo_namespace_list;

extern struct mutex tomoyo_policy_lock;

extern struct srcu_struct tomoyo_ss;

extern struct tomoyo_domain_info tomoyo_kernel_domain;

extern struct tomoyo_policy_namespace tomoyo_kernel_namespace;

extern unsigned int tomoyo_memory_quota[TOMOYO_MAX_MEMORY_STAT];

extern unsigned int tomoyo_memory_used[TOMOYO_MAX_MEMORY_STAT];

extern struct lsm_blob_sizes tomoyo_blob_sizes;



/********** Function prototypes. **********/



int tomoyo_interface_init(void);
@@ -971,10 +1000,10 @@ const struct tomoyo_path_info *tomoyo_path_matches_group 
int tomoyo_check_open_permission(struct tomoyo_domain_info *domain,

				 const struct path *path, const int flag);

void tomoyo_close_control(struct tomoyo_io_buffer *head);

int tomoyo_env_perm(struct tomoyo_request_info *r, const char *env);

int tomoyo_env_perm(struct tomoyo_request_info *r, const char *env) __must_hold_shared(&tomoyo_ss);

int tomoyo_execute_permission(struct tomoyo_request_info *r,

			      const struct tomoyo_path_info *filename);

int tomoyo_find_next_domain(struct linux_binprm *bprm);

			      const struct tomoyo_path_info *filename) __must_hold_shared(&tomoyo_ss);

int tomoyo_find_next_domain(struct linux_binprm *bprm) __must_hold_shared(&tomoyo_ss);

int tomoyo_get_mode(const struct tomoyo_policy_namespace *ns, const u8 profile,

		    const u8 index);

int tomoyo_init_request_info(struct tomoyo_request_info *r,
@@ -1002,6 +1031,7 @@ int tomoyo_socket_listen_permission(struct socket *sock); 
int tomoyo_socket_sendmsg_permission(struct socket *sock, struct msghdr *msg,

				     int size);

int tomoyo_supervisor(struct tomoyo_request_info *r, const char *fmt, ...)

	__must_hold_shared(&tomoyo_ss)

	__printf(2, 3);

int tomoyo_update_domain(struct tomoyo_acl_info *new_entry, const int size,

			 struct tomoyo_acl_param *param,
@@ -1061,7 +1091,7 @@ void tomoyo_print_ulong(char *buffer, const int buffer_len, 
			const unsigned long value, const u8 type);

void tomoyo_put_name_union(struct tomoyo_name_union *ptr);

void tomoyo_put_number_union(struct tomoyo_number_union *ptr);

void tomoyo_read_log(struct tomoyo_io_buffer *head);

void tomoyo_read_log(struct tomoyo_io_buffer *head) __must_hold(&head->io_sem);

void tomoyo_update_stat(const u8 index);

void tomoyo_warn_oom(const char *function);

void tomoyo_write_log(struct tomoyo_request_info *r, const char *fmt, ...)
@@ -1069,35 +1099,6 @@ void tomoyo_write_log(struct tomoyo_request_info *r, const char *fmt, ...) 
void tomoyo_write_log2(struct tomoyo_request_info *r, int len, const char *fmt,

		       va_list args) __printf(3, 0);



/********** External variable definitions. **********/



extern bool tomoyo_policy_loaded;

extern int tomoyo_enabled;

extern const char * const tomoyo_condition_keyword

[TOMOYO_MAX_CONDITION_KEYWORD];

extern const char * const tomoyo_dif[TOMOYO_MAX_DOMAIN_INFO_FLAGS];

extern const char * const tomoyo_mac_keywords[TOMOYO_MAX_MAC_INDEX

					      + TOMOYO_MAX_MAC_CATEGORY_INDEX];

extern const char * const tomoyo_mode[TOMOYO_CONFIG_MAX_MODE];

extern const char * const tomoyo_path_keyword[TOMOYO_MAX_PATH_OPERATION];

extern const char * const tomoyo_proto_keyword[TOMOYO_SOCK_MAX];

extern const char * const tomoyo_socket_keyword[TOMOYO_MAX_NETWORK_OPERATION];

extern const u8 tomoyo_index2category[TOMOYO_MAX_MAC_INDEX];

extern const u8 tomoyo_pn2mac[TOMOYO_MAX_PATH_NUMBER_OPERATION];

extern const u8 tomoyo_pnnn2mac[TOMOYO_MAX_MKDEV_OPERATION];

extern const u8 tomoyo_pp2mac[TOMOYO_MAX_PATH2_OPERATION];

extern struct list_head tomoyo_condition_list;

extern struct list_head tomoyo_domain_list;

extern struct list_head tomoyo_name_list[TOMOYO_MAX_HASH];

extern struct list_head tomoyo_namespace_list;

extern struct mutex tomoyo_policy_lock;

extern struct srcu_struct tomoyo_ss;

extern struct tomoyo_domain_info tomoyo_kernel_domain;

extern struct tomoyo_policy_namespace tomoyo_kernel_namespace;

extern unsigned int tomoyo_memory_quota[TOMOYO_MAX_MEMORY_STAT];

extern unsigned int tomoyo_memory_used[TOMOYO_MAX_MEMORY_STAT];

extern struct lsm_blob_sizes tomoyo_blob_sizes;



/********** Inlined functions. **********/



/**
@@ -1106,6 +1107,7 @@ extern struct lsm_blob_sizes tomoyo_blob_sizes; 
 * Returns index number for tomoyo_read_unlock().

 */

static inline int tomoyo_read_lock(void)

	__acquires_shared(&tomoyo_ss)

{

	return srcu_read_lock(&tomoyo_ss);

}
@@ -1118,6 +1120,7 @@ static inline int tomoyo_read_lock(void) 
 * Returns nothing.

 */

static inline void tomoyo_read_unlock(int idx)

	__releases_shared(&tomoyo_ss)

{

	srcu_read_unlock(&tomoyo_ss, idx);

}

security/tomoyo/domain.c

+1 −0
Original line number Diff line number Diff line
@@ -611,6 +611,7 @@ struct tomoyo_domain_info *tomoyo_assign_domain(const char *domainname, 
 * Returns 0 on success, negative value otherwise.

 */

static int tomoyo_environ(struct tomoyo_execve *ee)

	__must_hold_shared(&tomoyo_ss)

{

	struct tomoyo_request_info *r = &ee->r;

	struct linux_binprm *bprm = ee->bprm;

security/tomoyo/environ.c

+1 −0
Original line number Diff line number Diff line
@@ -32,6 +32,7 @@ static bool tomoyo_check_env_acl(struct tomoyo_request_info *r, 
 * Returns 0 on success, negative value otherwise.

 */

static int tomoyo_audit_env_log(struct tomoyo_request_info *r)

	__must_hold_shared(&tomoyo_ss)

{

	return tomoyo_supervisor(r, "misc env %s\n",

				 r->param.environ.name->name);