[-next] security: use octal not symbolic permissions
diff mbox

Message ID 1e91f8e10ce76d3208239b6b5899aab76d1543ff.1528743633.git.joe@perches.com
State Changes Requested
Headers show

Commit Message

Joe Perches June 11, 2018, 7:01 p.m. UTC
Currently security files use a mixture of octal and symbolic styles
for permissions.

Using octal and not symbolic permissions is preferred by many as more
readable.

see: https://lkml.org/lkml/2016/8/2/1945

Prefer the direct use of octal for permissions.

Done using:

$ git ls-files security | \
  xargs ./scripts/checkpatch.pl -f --fix-inplace --types=symbolic_perms --strict

and some typing.

Before:	 $ git grep -P -w "0[0-7]{3,3}" security | wc -l
53
After:	 $ git grep -P -w "0[0-7]{3,3}" security | wc -l
136

Miscellanea:

o Whitespace neatening and line wrapping around these conversions.
o Remove now superfluous parentheses around direct use of 0600

Signed-off-by: Joe Perches <joe@perches.com>
---
 security/apparmor/apparmorfs.c  |  5 ++--
 security/apparmor/lsm.c         | 23 ++++++++---------
 security/integrity/ima/ima.h    |  4 +--
 security/integrity/ima/ima_fs.c | 13 +++++-----
 security/selinux/hooks.c        |  4 +--
 security/selinux/selinuxfs.c    | 57 ++++++++++++++++++++---------------------
 security/smack/smack_lsm.c      |  6 ++---
 security/smack/smackfs.c        | 46 ++++++++++++++++-----------------
 security/tomoyo/condition.c     | 18 ++++++-------
 9 files changed, 85 insertions(+), 91 deletions(-)

Comments

Casey Schaufler June 11, 2018, 8:07 p.m. UTC | #1
On 6/11/2018 12:01 PM, Joe Perches wrote:
> Currently security files use a mixture of octal and symbolic styles
> for permissions.
>
> Using octal and not symbolic permissions is preferred by many as more
> readable.
>
> see: https://lkml.org/lkml/2016/8/2/1945
>
> Prefer the direct use of octal for permissions.
>
> Done using:
>
> $ git ls-files security | \
>   xargs ./scripts/checkpatch.pl -f --fix-inplace --types=symbolic_perms --strict
>
> and some typing.
>
> Before:	 $ git grep -P -w "0[0-7]{3,3}" security | wc -l
> 53
> After:	 $ git grep -P -w "0[0-7]{3,3}" security | wc -l
> 136
>
> Miscellanea:
>
> o Whitespace neatening and line wrapping around these conversions.
> o Remove now superfluous parentheses around direct use of 0600
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  security/apparmor/apparmorfs.c  |  5 ++--
>  security/apparmor/lsm.c         | 23 ++++++++---------
>  security/integrity/ima/ima.h    |  4 +--
>  security/integrity/ima/ima_fs.c | 13 +++++-----
>  security/selinux/hooks.c        |  4 +--
>  security/selinux/selinuxfs.c    | 57 ++++++++++++++++++++---------------------
>  security/smack/smack_lsm.c      |  6 ++---
>  security/smack/smackfs.c        | 46 ++++++++++++++++-----------------
>  security/tomoyo/condition.c     | 18 ++++++-------
>  9 files changed, 85 insertions(+), 91 deletions(-)

If you want to break this up by security module I would take
the Smack part as soon as James does the tree update. If James
wants to take the whole thing at once you can add my:

Acked-by: Casey Schaufler <casey@schaufler-ca.com>

for the Smack changes.

>
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 949dd8a48164..c09dc0f3c3fe 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -2426,10 +2426,9 @@ static int aa_mk_null_file(struct dentry *parent)
>  	}
>  
>  	inode->i_ino = get_next_ino();
> -	inode->i_mode = S_IFCHR | S_IRUGO | S_IWUGO;
> +	inode->i_mode = S_IFCHR | 0666;
>  	inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
> -	init_special_inode(inode, S_IFCHR | S_IRUGO | S_IWUGO,
> -			   MKDEV(MEM_MAJOR, 3));
> +	init_special_inode(inode, S_IFCHR | 0666, MKDEV(MEM_MAJOR, 3));
>  	d_instantiate(dentry, inode);
>  	aa_null.dentry = dget(dentry);
>  	aa_null.mnt = mntget(mount);
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index fbb08bc78bee..6759a70918de 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1255,45 +1255,42 @@ static int param_get_mode(char *buffer, const struct kernel_param *kp);
>  /* AppArmor global enforcement switch - complain, enforce, kill */
>  enum profile_mode aa_g_profile_mode = APPARMOR_ENFORCE;
>  module_param_call(mode, param_set_mode, param_get_mode,
> -		  &aa_g_profile_mode, S_IRUSR | S_IWUSR);
> +		  &aa_g_profile_mode, 0600);
>  
>  /* whether policy verification hashing is enabled */
>  bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
>  #ifdef CONFIG_SECURITY_APPARMOR_HASH
> -module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR | S_IWUSR);
> +module_param_named(hash_policy, aa_g_hash_policy, aabool, 0600);
>  #endif
>  
>  /* Debug mode */
>  bool aa_g_debug = IS_ENABLED(CONFIG_SECURITY_APPARMOR_DEBUG_MESSAGES);
> -module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
> +module_param_named(debug, aa_g_debug, aabool, 0600);
>  
>  /* Audit mode */
>  enum audit_mode aa_g_audit;
> -module_param_call(audit, param_set_audit, param_get_audit,
> -		  &aa_g_audit, S_IRUSR | S_IWUSR);
> +module_param_call(audit, param_set_audit, param_get_audit, &aa_g_audit, 0600);
>  
>  /* Determines if audit header is included in audited messages.  This
>   * provides more context if the audit daemon is not running
>   */
>  bool aa_g_audit_header = true;
> -module_param_named(audit_header, aa_g_audit_header, aabool,
> -		   S_IRUSR | S_IWUSR);
> +module_param_named(audit_header, aa_g_audit_header, aabool, 0600);
>  
>  /* lock out loading/removal of policy
>   * TODO: add in at boot loading of policy, which is the only way to
>   *       load policy, if lock_policy is set
>   */
>  bool aa_g_lock_policy;
> -module_param_named(lock_policy, aa_g_lock_policy, aalockpolicy,
> -		   S_IRUSR | S_IWUSR);
> +module_param_named(lock_policy, aa_g_lock_policy, aalockpolicy, 0600);
>  
>  /* Syscall logging mode */
>  bool aa_g_logsyscall;
> -module_param_named(logsyscall, aa_g_logsyscall, aabool, S_IRUSR | S_IWUSR);
> +module_param_named(logsyscall, aa_g_logsyscall, aabool, 0600);
>  
>  /* Maximum pathname length before accesses will start getting rejected */
>  unsigned int aa_g_path_max = 2 * PATH_MAX;
> -module_param_named(path_max, aa_g_path_max, aauint, S_IRUSR);
> +module_param_named(path_max, aa_g_path_max, aauint, 0400);
>  
>  /* Determines how paranoid loading of policy is and how much verification
>   * on the loaded policy is done.
> @@ -1301,11 +1298,11 @@ module_param_named(path_max, aa_g_path_max, aauint, S_IRUSR);
>   * that none root users (user namespaces) can load policy.
>   */
>  bool aa_g_paranoid_load = true;
> -module_param_named(paranoid_load, aa_g_paranoid_load, aabool, S_IRUGO);
> +module_param_named(paranoid_load, aa_g_paranoid_load, aabool, 0444);
>  
>  /* Boot time disable flag */
>  static bool apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
> -module_param_named(enabled, apparmor_enabled, bool, S_IRUGO);
> +module_param_named(enabled, apparmor_enabled, bool, 0444);
>  
>  static int __init apparmor_enabled_setup(char *str)
>  {
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 354bb5716ce3..3f7707b8aaa7 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -314,9 +314,9 @@ static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
>  #endif /* CONFIG_IMA_LSM_RULES */
>  
>  #ifdef	CONFIG_IMA_READ_POLICY
> -#define	POLICY_FILE_FLAGS	(S_IWUSR | S_IRUSR)
> +#define	POLICY_FILE_FLAGS	0600
>  #else
> -#define	POLICY_FILE_FLAGS	S_IWUSR
> +#define	POLICY_FILE_FLAGS	0200
>  #endif /* CONFIG_IMA_READ_POLICY */
>  
>  #endif /* __LINUX_IMA_H */
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index ae9d5c766a3c..81700df83f51 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -439,7 +439,7 @@ static int ima_release_policy(struct inode *inode, struct file *file)
>  #elif defined(CONFIG_IMA_WRITE_POLICY)
>  	clear_bit(IMA_FS_BUSY, &ima_fs_flags);
>  #elif defined(CONFIG_IMA_READ_POLICY)
> -	inode->i_mode &= ~S_IWUSR;
> +	inode->i_mode &= ~0200;
>  #endif
>  	return 0;
>  }
> @@ -465,28 +465,29 @@ int __init ima_fs_init(void)
>  
>  	binary_runtime_measurements =
>  	    securityfs_create_file("binary_runtime_measurements",
> -				   S_IRUSR | S_IRGRP, ima_dir, NULL,
> +				   0440, ima_dir, NULL,
>  				   &ima_measurements_ops);
>  	if (IS_ERR(binary_runtime_measurements))
>  		goto out;
>  
>  	ascii_runtime_measurements =
>  	    securityfs_create_file("ascii_runtime_measurements",
> -				   S_IRUSR | S_IRGRP, ima_dir, NULL,
> +				   0440, ima_dir, NULL,
>  				   &ima_ascii_measurements_ops);
>  	if (IS_ERR(ascii_runtime_measurements))
>  		goto out;
>  
>  	runtime_measurements_count =
>  	    securityfs_create_file("runtime_measurements_count",
> -				   S_IRUSR | S_IRGRP, ima_dir, NULL,
> +				   0440, ima_dir, NULL,
>  				   &ima_measurements_count_ops);
>  	if (IS_ERR(runtime_measurements_count))
>  		goto out;
>  
>  	violations =
> -	    securityfs_create_file("violations", S_IRUSR | S_IRGRP,
> -				   ima_dir, NULL, &ima_htable_violations_ops);
> +		securityfs_create_file("violations",
> +				       0440, ima_dir, NULL,
> +				       &ima_htable_violations_ops);
>  	if (IS_ERR(violations))
>  		goto out;
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index a85fac3345df..8ae043be8782 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6336,9 +6336,9 @@ static int selinux_ipc_permission(struct kern_ipc_perm *ipcp, short flag)
>  	u32 av = 0;
>  
>  	av = 0;
> -	if (flag & S_IRUGO)
> +	if (flag & 0444)
>  		av |= IPC__UNIX_READ;
> -	if (flag & S_IWUGO)
> +	if (flag & 0222)
>  		av |= IPC__UNIX_WRITE;
>  
>  	if (av == 0)
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index f3d374d2ca04..bfecac19ba92 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -1376,7 +1376,7 @@ static int sel_make_bools(struct selinux_fs_info *fsi)
>  			goto out;
>  
>  		ret = -ENOMEM;
> -		inode = sel_make_inode(dir->d_sb, S_IFREG | S_IRUGO | S_IWUSR);
> +		inode = sel_make_inode(dir->d_sb, S_IFREG | 0644);
>  		if (!inode)
>  			goto out;
>  
> @@ -1582,10 +1582,10 @@ static int sel_make_avc_files(struct dentry *dir)
>  	int i;
>  	static const struct tree_descr files[] = {
>  		{ "cache_threshold",
> -		  &sel_avc_cache_threshold_ops, S_IRUGO|S_IWUSR },
> -		{ "hash_stats", &sel_avc_hash_stats_ops, S_IRUGO },
> +		  &sel_avc_cache_threshold_ops, 0644 },
> +		{ "hash_stats", &sel_avc_hash_stats_ops, 0444 },
>  #ifdef CONFIG_SECURITY_SELINUX_AVC_STATS
> -		{ "cache_stats", &sel_avc_cache_stats_ops, S_IRUGO },
> +		{ "cache_stats", &sel_avc_cache_stats_ops, 0444 },
>  #endif
>  	};
>  
> @@ -1643,7 +1643,7 @@ static int sel_make_initcon_files(struct dentry *dir)
>  		if (!dentry)
>  			return -ENOMEM;
>  
> -		inode = sel_make_inode(dir->d_sb, S_IFREG|S_IRUGO);
> +		inode = sel_make_inode(dir->d_sb, S_IFREG | 0444);
>  		if (!inode)
>  			return -ENOMEM;
>  
> @@ -1744,7 +1744,7 @@ static int sel_make_perm_files(char *objclass, int classvalue,
>  			goto out;
>  
>  		rc = -ENOMEM;
> -		inode = sel_make_inode(dir->d_sb, S_IFREG|S_IRUGO);
> +		inode = sel_make_inode(dir->d_sb, S_IFREG | 0444);
>  		if (!inode)
>  			goto out;
>  
> @@ -1774,7 +1774,7 @@ static int sel_make_class_dir_entries(char *classname, int index,
>  	if (!dentry)
>  		return -ENOMEM;
>  
> -	inode = sel_make_inode(dir->d_sb, S_IFREG|S_IRUGO);
> +	inode = sel_make_inode(dir->d_sb, S_IFREG | 0444);
>  	if (!inode)
>  		return -ENOMEM;
>  
> @@ -1870,7 +1870,7 @@ static struct dentry *sel_make_dir(struct dentry *dir, const char *name,
>  	if (!dentry)
>  		return ERR_PTR(-ENOMEM);
>  
> -	inode = sel_make_inode(dir->d_sb, S_IFDIR | S_IRUGO | S_IXUGO);
> +	inode = sel_make_inode(dir->d_sb, S_IFDIR | 0555);
>  	if (!inode) {
>  		dput(dentry);
>  		return ERR_PTR(-ENOMEM);
> @@ -1899,25 +1899,24 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
>  	struct inode_security_struct *isec;
>  
>  	static const struct tree_descr selinux_files[] = {
> -		[SEL_LOAD] = {"load", &sel_load_ops, S_IRUSR|S_IWUSR},
> -		[SEL_ENFORCE] = {"enforce", &sel_enforce_ops, S_IRUGO|S_IWUSR},
> -		[SEL_CONTEXT] = {"context", &transaction_ops, S_IRUGO|S_IWUGO},
> -		[SEL_ACCESS] = {"access", &transaction_ops, S_IRUGO|S_IWUGO},
> -		[SEL_CREATE] = {"create", &transaction_ops, S_IRUGO|S_IWUGO},
> -		[SEL_RELABEL] = {"relabel", &transaction_ops, S_IRUGO|S_IWUGO},
> -		[SEL_USER] = {"user", &transaction_ops, S_IRUGO|S_IWUGO},
> -		[SEL_POLICYVERS] = {"policyvers", &sel_policyvers_ops, S_IRUGO},
> -		[SEL_COMMIT_BOOLS] = {"commit_pending_bools", &sel_commit_bools_ops, S_IWUSR},
> -		[SEL_MLS] = {"mls", &sel_mls_ops, S_IRUGO},
> -		[SEL_DISABLE] = {"disable", &sel_disable_ops, S_IWUSR},
> -		[SEL_MEMBER] = {"member", &transaction_ops, S_IRUGO|S_IWUGO},
> -		[SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, S_IRUGO|S_IWUSR},
> -		[SEL_REJECT_UNKNOWN] = {"reject_unknown", &sel_handle_unknown_ops, S_IRUGO},
> -		[SEL_DENY_UNKNOWN] = {"deny_unknown", &sel_handle_unknown_ops, S_IRUGO},
> -		[SEL_STATUS] = {"status", &sel_handle_status_ops, S_IRUGO},
> -		[SEL_POLICY] = {"policy", &sel_policy_ops, S_IRUGO},
> -		[SEL_VALIDATE_TRANS] = {"validatetrans", &sel_transition_ops,
> -					S_IWUGO},
> +		[SEL_LOAD] = {"load", &sel_load_ops, 0600},
> +		[SEL_ENFORCE] = {"enforce", &sel_enforce_ops, 0644},
> +		[SEL_CONTEXT] = {"context", &transaction_ops, 0666},
> +		[SEL_ACCESS] = {"access", &transaction_ops, 0666},
> +		[SEL_CREATE] = {"create", &transaction_ops, 0666},
> +		[SEL_RELABEL] = {"relabel", &transaction_ops, 0666},
> +		[SEL_USER] = {"user", &transaction_ops, 0666},
> +		[SEL_POLICYVERS] = {"policyvers", &sel_policyvers_ops, 0444},
> +		[SEL_COMMIT_BOOLS] = {"commit_pending_bools", &sel_commit_bools_ops, 0200},
> +		[SEL_MLS] = {"mls", &sel_mls_ops, 0444},
> +		[SEL_DISABLE] = {"disable", &sel_disable_ops, 0200},
> +		[SEL_MEMBER] = {"member", &transaction_ops, 0666},
> +		[SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, 0644},
> +		[SEL_REJECT_UNKNOWN] = {"reject_unknown", &sel_handle_unknown_ops, 0444},
> +		[SEL_DENY_UNKNOWN] = {"deny_unknown", &sel_handle_unknown_ops, 0444},
> +		[SEL_STATUS] = {"status", &sel_handle_status_ops, 0444},
> +		[SEL_POLICY] = {"policy", &sel_policy_ops, 0444},
> +		[SEL_VALIDATE_TRANS] = {"validatetrans", &sel_transition_ops, 0222},
>  		/* last one */ {""}
>  	};
>  
> @@ -1943,7 +1942,7 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
>  		goto err;
>  
>  	ret = -ENOMEM;
> -	inode = sel_make_inode(sb, S_IFCHR | S_IRUGO | S_IWUGO);
> +	inode = sel_make_inode(sb, S_IFCHR | 0666);
>  	if (!inode)
>  		goto err;
>  
> @@ -1953,7 +1952,7 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
>  	isec->sclass = SECCLASS_CHR_FILE;
>  	isec->initialized = LABEL_INITIALIZED;
>  
> -	init_special_inode(inode, S_IFCHR | S_IRUGO | S_IWUGO, MKDEV(MEM_MAJOR, 3));
> +	init_special_inode(inode, S_IFCHR | 0666, MKDEV(MEM_MAJOR, 3));
>  	d_add(dentry, inode);
>  
>  	dentry = sel_make_dir(sb->s_root, "avc", &fsi->last_ino);
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index dcb976f98df2..8953440c6559 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2945,11 +2945,11 @@ static int smack_flags_to_may(int flags)
>  {
>  	int may = 0;
>  
> -	if (flags & S_IRUGO)
> +	if (flags & 0444)
>  		may |= MAY_READ;
> -	if (flags & S_IWUGO)
> +	if (flags & 0222)
>  		may |= MAY_WRITE;
> -	if (flags & S_IXUGO)
> +	if (flags & 0111)
>  		may |= MAY_EXEC;
>  
>  	return may;
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index f6482e53d55a..270cd3a308f0 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -2857,55 +2857,53 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
>  
>  	static const struct tree_descr smack_files[] = {
>  		[SMK_LOAD] = {
> -			"load", &smk_load_ops, S_IRUGO|S_IWUSR},
> +			"load", &smk_load_ops, 0644},
>  		[SMK_CIPSO] = {
> -			"cipso", &smk_cipso_ops, S_IRUGO|S_IWUSR},
> +			"cipso", &smk_cipso_ops, 0644},
>  		[SMK_DOI] = {
> -			"doi", &smk_doi_ops, S_IRUGO|S_IWUSR},
> +			"doi", &smk_doi_ops, 0644},
>  		[SMK_DIRECT] = {
> -			"direct", &smk_direct_ops, S_IRUGO|S_IWUSR},
> +			"direct", &smk_direct_ops, 0644},
>  		[SMK_AMBIENT] = {
> -			"ambient", &smk_ambient_ops, S_IRUGO|S_IWUSR},
> +			"ambient", &smk_ambient_ops, 0644},
>  		[SMK_NET4ADDR] = {
> -			"netlabel", &smk_net4addr_ops, S_IRUGO|S_IWUSR},
> +			"netlabel", &smk_net4addr_ops, 0644},
>  		[SMK_ONLYCAP] = {
> -			"onlycap", &smk_onlycap_ops, S_IRUGO|S_IWUSR},
> +			"onlycap", &smk_onlycap_ops, 0644},
>  		[SMK_LOGGING] = {
> -			"logging", &smk_logging_ops, S_IRUGO|S_IWUSR},
> +			"logging", &smk_logging_ops, 0644},
>  		[SMK_LOAD_SELF] = {
> -			"load-self", &smk_load_self_ops, S_IRUGO|S_IWUGO},
> +			"load-self", &smk_load_self_ops, 0666},
>  		[SMK_ACCESSES] = {
> -			"access", &smk_access_ops, S_IRUGO|S_IWUGO},
> +			"access", &smk_access_ops, 0666},
>  		[SMK_MAPPED] = {
> -			"mapped", &smk_mapped_ops, S_IRUGO|S_IWUSR},
> +			"mapped", &smk_mapped_ops, 0644},
>  		[SMK_LOAD2] = {
> -			"load2", &smk_load2_ops, S_IRUGO|S_IWUSR},
> +			"load2", &smk_load2_ops, 0644},
>  		[SMK_LOAD_SELF2] = {
> -			"load-self2", &smk_load_self2_ops, S_IRUGO|S_IWUGO},
> +			"load-self2", &smk_load_self2_ops, 0666},
>  		[SMK_ACCESS2] = {
> -			"access2", &smk_access2_ops, S_IRUGO|S_IWUGO},
> +			"access2", &smk_access2_ops, 0666},
>  		[SMK_CIPSO2] = {
> -			"cipso2", &smk_cipso2_ops, S_IRUGO|S_IWUSR},
> +			"cipso2", &smk_cipso2_ops, 0644},
>  		[SMK_REVOKE_SUBJ] = {
> -			"revoke-subject", &smk_revoke_subj_ops,
> -			S_IRUGO|S_IWUSR},
> +			"revoke-subject", &smk_revoke_subj_ops, 0644},
>  		[SMK_CHANGE_RULE] = {
> -			"change-rule", &smk_change_rule_ops, S_IRUGO|S_IWUSR},
> +			"change-rule", &smk_change_rule_ops, 0644},
>  		[SMK_SYSLOG] = {
> -			"syslog", &smk_syslog_ops, S_IRUGO|S_IWUSR},
> +			"syslog", &smk_syslog_ops, 0644},
>  		[SMK_PTRACE] = {
> -			"ptrace", &smk_ptrace_ops, S_IRUGO|S_IWUSR},
> +			"ptrace", &smk_ptrace_ops, 0644},
>  #ifdef CONFIG_SECURITY_SMACK_BRINGUP
>  		[SMK_UNCONFINED] = {
> -			"unconfined", &smk_unconfined_ops, S_IRUGO|S_IWUSR},
> +			"unconfined", &smk_unconfined_ops, 0644},
>  #endif
>  #if IS_ENABLED(CONFIG_IPV6)
>  		[SMK_NET6ADDR] = {
> -			"ipv6host", &smk_net6addr_ops, S_IRUGO|S_IWUSR},
> +			"ipv6host", &smk_net6addr_ops, 0644},
>  #endif /* CONFIG_IPV6 */
>  		[SMK_RELABEL_SELF] = {
> -			"relabel-self", &smk_relabel_self_ops,
> -				S_IRUGO|S_IWUGO},
> +			"relabel-self", &smk_relabel_self_ops, 0666},
>  		/* last one */
>  			{""}
>  	};
> diff --git a/security/tomoyo/condition.c b/security/tomoyo/condition.c
> index 8d0e1b9c9c57..2069f5912469 100644
> --- a/security/tomoyo/condition.c
> +++ b/security/tomoyo/condition.c
> @@ -874,31 +874,31 @@ bool tomoyo_condition(struct tomoyo_request_info *r,
>  				value = S_ISVTX;
>  				break;
>  			case TOMOYO_MODE_OWNER_READ:
> -				value = S_IRUSR;
> +				value = 0400;
>  				break;
>  			case TOMOYO_MODE_OWNER_WRITE:
> -				value = S_IWUSR;
> +				value = 0200;
>  				break;
>  			case TOMOYO_MODE_OWNER_EXECUTE:
> -				value = S_IXUSR;
> +				value = 0100;
>  				break;
>  			case TOMOYO_MODE_GROUP_READ:
> -				value = S_IRGRP;
> +				value = 0040;
>  				break;
>  			case TOMOYO_MODE_GROUP_WRITE:
> -				value = S_IWGRP;
> +				value = 0020;
>  				break;
>  			case TOMOYO_MODE_GROUP_EXECUTE:
> -				value = S_IXGRP;
> +				value = 0010;
>  				break;
>  			case TOMOYO_MODE_OTHERS_READ:
> -				value = S_IROTH;
> +				value = 0004;
>  				break;
>  			case TOMOYO_MODE_OTHERS_WRITE:
> -				value = S_IWOTH;
> +				value = 0002;
>  				break;
>  			case TOMOYO_MODE_OTHERS_EXECUTE:
> -				value = S_IXOTH;
> +				value = 0001;
>  				break;
>  			case TOMOYO_EXEC_ARGC:
>  				if (!bprm)
James Morris June 12, 2018, 8:32 p.m. UTC | #2
On Mon, 11 Jun 2018, Casey Schaufler wrote:

> If you want to break this up by security module I would take
> the Smack part as soon as James does the tree update. If James
> wants to take the whole thing at once you can add my:
> 
> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
> 
> for the Smack changes.

It's probably simplest for me to take them as one patch.
Paul Moore June 12, 2018, 9:12 p.m. UTC | #3
On Tue, Jun 12, 2018 at 4:32 PM, James Morris <jmorris@namei.org> wrote:
> On Mon, 11 Jun 2018, Casey Schaufler wrote:
>
>> If you want to break this up by security module I would take
>> the Smack part as soon as James does the tree update. If James
>> wants to take the whole thing at once you can add my:
>>
>> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
>>
>> for the Smack changes.
>
> It's probably simplest for me to take them as one patch.

I would prefer if the SELinux changes were split into a separate
patch.  I'm guessing John would probably want the same for the
AppArmor patches, but take his work for it, not mine.

Joe, in general I really appreciate the fixes you send, but these
patches that cross a lot of subsystem boundaries (this isn't the first
one that does this) causes unnecessary conflicts in -next and during
the merge window.  Could you split your patches up from now on please?
John Johansen June 12, 2018, 9:29 p.m. UTC | #4
On 06/12/2018 02:12 PM, Paul Moore wrote:
> On Tue, Jun 12, 2018 at 4:32 PM, James Morris <jmorris@namei.org> wrote:
>> On Mon, 11 Jun 2018, Casey Schaufler wrote:
>>
>>> If you want to break this up by security module I would take
>>> the Smack part as soon as James does the tree update. If James
>>> wants to take the whole thing at once you can add my:
>>>
>>> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
>>>
>>> for the Smack changes.
>>
>> It's probably simplest for me to take them as one patch.
> 
> I would prefer if the SELinux changes were split into a separate
> patch.  I'm guessing John would probably want the same for the
> AppArmor patches, but take his work for it, not mine.

yes that would be preferred

> 
> Joe, in general I really appreciate the fixes you send, but these
> patches that cross a lot of subsystem boundaries (this isn't the first
> one that does this) causes unnecessary conflicts in -next and during
> the merge window.  Could you split your patches up from now on please?
> 

yeah splitting patches at subsystem boundaries is highly recommended.
Mimi Zohar June 12, 2018, 9:36 p.m. UTC | #5
On Tue, 2018-06-12 at 14:29 -0700, John Johansen wrote:
> On 06/12/2018 02:12 PM, Paul Moore wrote:
> > On Tue, Jun 12, 2018 at 4:32 PM, James Morris <jmorris@namei.org> wrote:
> >> On Mon, 11 Jun 2018, Casey Schaufler wrote:
> >>
> >>> If you want to break this up by security module I would take
> >>> the Smack part as soon as James does the tree update. If James
> >>> wants to take the whole thing at once you can add my:
> >>>
> >>> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
> >>>
> >>> for the Smack changes.
> >>
> >> It's probably simplest for me to take them as one patch.
> > 
> > I would prefer if the SELinux changes were split into a separate
> > patch.  I'm guessing John would probably want the same for the
> > AppArmor patches, but take his work for it, not mine.
> 
> yes that would be preferred

Agreed
> 
> > 
> > Joe, in general I really appreciate the fixes you send, but these
> > patches that cross a lot of subsystem boundaries (this isn't the first
> > one that does this) causes unnecessary conflicts in -next and during
> > the merge window.  Could you split your patches up from now on please?
> > 
> 
> yeah splitting patches at subsystem boundaries is highly recommended.

Agreed
Joe Perches June 13, 2018, 12:29 a.m. UTC | #6
On Tue, 2018-06-12 at 17:12 -0400, Paul Moore wrote:
> On Tue, Jun 12, 2018 at 4:32 PM, James Morris <jmorris@namei.org> wrote:
> > On Mon, 11 Jun 2018, Casey Schaufler wrote:
> > 
> > > If you want to break this up by security module I would take
> > > the Smack part as soon as James does the tree update. If James
> > > wants to take the whole thing at once you can add my:
> > > 
> > > Acked-by: Casey Schaufler <casey@schaufler-ca.com>
> > > 
> > > for the Smack changes.
> > 
> > It's probably simplest for me to take them as one patch.
> 
> I would prefer if the SELinux changes were split into a separate
> patch.  I'm guessing John would probably want the same for the
> AppArmor patches, but take his work for it, not mine.
> 
> Joe, in general I really appreciate the fixes you send, but these
> patches that cross a lot of subsystem boundaries (this isn't the first
> one that does this) causes unnecessary conflicts in -next and during
> the merge window.  Could you split your patches up from now on please?

Sorry. No.  Merge conflicts are inherent in this system.

There is just no good way to do this as sending a set
of per subsystem patches guarantees partial application
of the entire set.

The nominal best way is for a script to be run and
applied at the top level rather than sending a patch
at all.

If you prefer, each sub-subsystem maintainer, at
whatever granularity desired, could apply the patch
to the appropriate subsystem by using
"git am --include=<subsystem_path>".

cheers, Joe
Serge E. Hallyn June 13, 2018, 3:19 p.m. UTC | #7
Quoting Joe Perches (joe@perches.com):
> Currently security files use a mixture of octal and symbolic styles
> for permissions.
> 
> Using octal and not symbolic permissions is preferred by many as more
> readable.
> 
> see: https://lkml.org/lkml/2016/8/2/1945

Heh, well see also https://lkml.org/lkml/2016/8/2/2062 .  Your patch
definately improves readability, but will doubtless make backpointing
future important patches more painful.

> Prefer the direct use of octal for permissions.
> 
> Done using:
> 
> $ git ls-files security | \
>   xargs ./scripts/checkpatch.pl -f --fix-inplace --types=symbolic_perms --strict
> 
> and some typing.
> 
> Before:	 $ git grep -P -w "0[0-7]{3,3}" security | wc -l
> 53
> After:	 $ git grep -P -w "0[0-7]{3,3}" security | wc -l
> 136
> 
> Miscellanea:
> 
> o Whitespace neatening and line wrapping around these conversions.
> o Remove now superfluous parentheses around direct use of 0600
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  security/apparmor/apparmorfs.c  |  5 ++--
>  security/apparmor/lsm.c         | 23 ++++++++---------
>  security/integrity/ima/ima.h    |  4 +--
>  security/integrity/ima/ima_fs.c | 13 +++++-----
>  security/selinux/hooks.c        |  4 +--
>  security/selinux/selinuxfs.c    | 57 ++++++++++++++++++++---------------------
>  security/smack/smack_lsm.c      |  6 ++---
>  security/smack/smackfs.c        | 46 ++++++++++++++++-----------------
>  security/tomoyo/condition.c     | 18 ++++++-------
>  9 files changed, 85 insertions(+), 91 deletions(-)
> 
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 949dd8a48164..c09dc0f3c3fe 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -2426,10 +2426,9 @@ static int aa_mk_null_file(struct dentry *parent)
>  	}
>  
>  	inode->i_ino = get_next_ino();
> -	inode->i_mode = S_IFCHR | S_IRUGO | S_IWUGO;
> +	inode->i_mode = S_IFCHR | 0666;
>  	inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
> -	init_special_inode(inode, S_IFCHR | S_IRUGO | S_IWUGO,
> -			   MKDEV(MEM_MAJOR, 3));
> +	init_special_inode(inode, S_IFCHR | 0666, MKDEV(MEM_MAJOR, 3));
>  	d_instantiate(dentry, inode);
>  	aa_null.dentry = dget(dentry);
>  	aa_null.mnt = mntget(mount);
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index fbb08bc78bee..6759a70918de 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1255,45 +1255,42 @@ static int param_get_mode(char *buffer, const struct kernel_param *kp);
>  /* AppArmor global enforcement switch - complain, enforce, kill */
>  enum profile_mode aa_g_profile_mode = APPARMOR_ENFORCE;
>  module_param_call(mode, param_set_mode, param_get_mode,
> -		  &aa_g_profile_mode, S_IRUSR | S_IWUSR);
> +		  &aa_g_profile_mode, 0600);
>  
>  /* whether policy verification hashing is enabled */
>  bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
>  #ifdef CONFIG_SECURITY_APPARMOR_HASH
> -module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR | S_IWUSR);
> +module_param_named(hash_policy, aa_g_hash_policy, aabool, 0600);
>  #endif
>  
>  /* Debug mode */
>  bool aa_g_debug = IS_ENABLED(CONFIG_SECURITY_APPARMOR_DEBUG_MESSAGES);
> -module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
> +module_param_named(debug, aa_g_debug, aabool, 0600);
>  
>  /* Audit mode */
>  enum audit_mode aa_g_audit;
> -module_param_call(audit, param_set_audit, param_get_audit,
> -		  &aa_g_audit, S_IRUSR | S_IWUSR);
> +module_param_call(audit, param_set_audit, param_get_audit, &aa_g_audit, 0600);
>  
>  /* Determines if audit header is included in audited messages.  This
>   * provides more context if the audit daemon is not running
>   */
>  bool aa_g_audit_header = true;
> -module_param_named(audit_header, aa_g_audit_header, aabool,
> -		   S_IRUSR | S_IWUSR);
> +module_param_named(audit_header, aa_g_audit_header, aabool, 0600);
>  
>  /* lock out loading/removal of policy
>   * TODO: add in at boot loading of policy, which is the only way to
>   *       load policy, if lock_policy is set
>   */
>  bool aa_g_lock_policy;
> -module_param_named(lock_policy, aa_g_lock_policy, aalockpolicy,
> -		   S_IRUSR | S_IWUSR);
> +module_param_named(lock_policy, aa_g_lock_policy, aalockpolicy, 0600);
>  
>  /* Syscall logging mode */
>  bool aa_g_logsyscall;
> -module_param_named(logsyscall, aa_g_logsyscall, aabool, S_IRUSR | S_IWUSR);
> +module_param_named(logsyscall, aa_g_logsyscall, aabool, 0600);
>  
>  /* Maximum pathname length before accesses will start getting rejected */
>  unsigned int aa_g_path_max = 2 * PATH_MAX;
> -module_param_named(path_max, aa_g_path_max, aauint, S_IRUSR);
> +module_param_named(path_max, aa_g_path_max, aauint, 0400);
>  
>  /* Determines how paranoid loading of policy is and how much verification
>   * on the loaded policy is done.
> @@ -1301,11 +1298,11 @@ module_param_named(path_max, aa_g_path_max, aauint, S_IRUSR);
>   * that none root users (user namespaces) can load policy.
>   */
>  bool aa_g_paranoid_load = true;
> -module_param_named(paranoid_load, aa_g_paranoid_load, aabool, S_IRUGO);
> +module_param_named(paranoid_load, aa_g_paranoid_load, aabool, 0444);
>  
>  /* Boot time disable flag */
>  static bool apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
> -module_param_named(enabled, apparmor_enabled, bool, S_IRUGO);
> +module_param_named(enabled, apparmor_enabled, bool, 0444);
>  
>  static int __init apparmor_enabled_setup(char *str)
>  {
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 354bb5716ce3..3f7707b8aaa7 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -314,9 +314,9 @@ static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
>  #endif /* CONFIG_IMA_LSM_RULES */
>  
>  #ifdef	CONFIG_IMA_READ_POLICY
> -#define	POLICY_FILE_FLAGS	(S_IWUSR | S_IRUSR)
> +#define	POLICY_FILE_FLAGS	0600
>  #else
> -#define	POLICY_FILE_FLAGS	S_IWUSR
> +#define	POLICY_FILE_FLAGS	0200
>  #endif /* CONFIG_IMA_READ_POLICY */
>  
>  #endif /* __LINUX_IMA_H */
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index ae9d5c766a3c..81700df83f51 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -439,7 +439,7 @@ static int ima_release_policy(struct inode *inode, struct file *file)
>  #elif defined(CONFIG_IMA_WRITE_POLICY)
>  	clear_bit(IMA_FS_BUSY, &ima_fs_flags);
>  #elif defined(CONFIG_IMA_READ_POLICY)
> -	inode->i_mode &= ~S_IWUSR;
> +	inode->i_mode &= ~0200;
>  #endif
>  	return 0;
>  }
> @@ -465,28 +465,29 @@ int __init ima_fs_init(void)
>  
>  	binary_runtime_measurements =
>  	    securityfs_create_file("binary_runtime_measurements",
> -				   S_IRUSR | S_IRGRP, ima_dir, NULL,
> +				   0440, ima_dir, NULL,
>  				   &ima_measurements_ops);
>  	if (IS_ERR(binary_runtime_measurements))
>  		goto out;
>  
>  	ascii_runtime_measurements =
>  	    securityfs_create_file("ascii_runtime_measurements",
> -				   S_IRUSR | S_IRGRP, ima_dir, NULL,
> +				   0440, ima_dir, NULL,
>  				   &ima_ascii_measurements_ops);
>  	if (IS_ERR(ascii_runtime_measurements))
>  		goto out;
>  
>  	runtime_measurements_count =
>  	    securityfs_create_file("runtime_measurements_count",
> -				   S_IRUSR | S_IRGRP, ima_dir, NULL,
> +				   0440, ima_dir, NULL,
>  				   &ima_measurements_count_ops);
>  	if (IS_ERR(runtime_measurements_count))
>  		goto out;
>  
>  	violations =
> -	    securityfs_create_file("violations", S_IRUSR | S_IRGRP,
> -				   ima_dir, NULL, &ima_htable_violations_ops);
> +		securityfs_create_file("violations",
> +				       0440, ima_dir, NULL,
> +				       &ima_htable_violations_ops);
>  	if (IS_ERR(violations))
>  		goto out;
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index a85fac3345df..8ae043be8782 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6336,9 +6336,9 @@ static int selinux_ipc_permission(struct kern_ipc_perm *ipcp, short flag)
>  	u32 av = 0;
>  
>  	av = 0;
> -	if (flag & S_IRUGO)
> +	if (flag & 0444)
>  		av |= IPC__UNIX_READ;
> -	if (flag & S_IWUGO)
> +	if (flag & 0222)
>  		av |= IPC__UNIX_WRITE;
>  
>  	if (av == 0)
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index f3d374d2ca04..bfecac19ba92 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -1376,7 +1376,7 @@ static int sel_make_bools(struct selinux_fs_info *fsi)
>  			goto out;
>  
>  		ret = -ENOMEM;
> -		inode = sel_make_inode(dir->d_sb, S_IFREG | S_IRUGO | S_IWUSR);
> +		inode = sel_make_inode(dir->d_sb, S_IFREG | 0644);
>  		if (!inode)
>  			goto out;
>  
> @@ -1582,10 +1582,10 @@ static int sel_make_avc_files(struct dentry *dir)
>  	int i;
>  	static const struct tree_descr files[] = {
>  		{ "cache_threshold",
> -		  &sel_avc_cache_threshold_ops, S_IRUGO|S_IWUSR },
> -		{ "hash_stats", &sel_avc_hash_stats_ops, S_IRUGO },
> +		  &sel_avc_cache_threshold_ops, 0644 },
> +		{ "hash_stats", &sel_avc_hash_stats_ops, 0444 },
>  #ifdef CONFIG_SECURITY_SELINUX_AVC_STATS
> -		{ "cache_stats", &sel_avc_cache_stats_ops, S_IRUGO },
> +		{ "cache_stats", &sel_avc_cache_stats_ops, 0444 },
>  #endif
>  	};
>  
> @@ -1643,7 +1643,7 @@ static int sel_make_initcon_files(struct dentry *dir)
>  		if (!dentry)
>  			return -ENOMEM;
>  
> -		inode = sel_make_inode(dir->d_sb, S_IFREG|S_IRUGO);
> +		inode = sel_make_inode(dir->d_sb, S_IFREG | 0444);
>  		if (!inode)
>  			return -ENOMEM;
>  
> @@ -1744,7 +1744,7 @@ static int sel_make_perm_files(char *objclass, int classvalue,
>  			goto out;
>  
>  		rc = -ENOMEM;
> -		inode = sel_make_inode(dir->d_sb, S_IFREG|S_IRUGO);
> +		inode = sel_make_inode(dir->d_sb, S_IFREG | 0444);
>  		if (!inode)
>  			goto out;
>  
> @@ -1774,7 +1774,7 @@ static int sel_make_class_dir_entries(char *classname, int index,
>  	if (!dentry)
>  		return -ENOMEM;
>  
> -	inode = sel_make_inode(dir->d_sb, S_IFREG|S_IRUGO);
> +	inode = sel_make_inode(dir->d_sb, S_IFREG | 0444);
>  	if (!inode)
>  		return -ENOMEM;
>  
> @@ -1870,7 +1870,7 @@ static struct dentry *sel_make_dir(struct dentry *dir, const char *name,
>  	if (!dentry)
>  		return ERR_PTR(-ENOMEM);
>  
> -	inode = sel_make_inode(dir->d_sb, S_IFDIR | S_IRUGO | S_IXUGO);
> +	inode = sel_make_inode(dir->d_sb, S_IFDIR | 0555);
>  	if (!inode) {
>  		dput(dentry);
>  		return ERR_PTR(-ENOMEM);
> @@ -1899,25 +1899,24 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
>  	struct inode_security_struct *isec;
>  
>  	static const struct tree_descr selinux_files[] = {
> -		[SEL_LOAD] = {"load", &sel_load_ops, S_IRUSR|S_IWUSR},
> -		[SEL_ENFORCE] = {"enforce", &sel_enforce_ops, S_IRUGO|S_IWUSR},
> -		[SEL_CONTEXT] = {"context", &transaction_ops, S_IRUGO|S_IWUGO},
> -		[SEL_ACCESS] = {"access", &transaction_ops, S_IRUGO|S_IWUGO},
> -		[SEL_CREATE] = {"create", &transaction_ops, S_IRUGO|S_IWUGO},
> -		[SEL_RELABEL] = {"relabel", &transaction_ops, S_IRUGO|S_IWUGO},
> -		[SEL_USER] = {"user", &transaction_ops, S_IRUGO|S_IWUGO},
> -		[SEL_POLICYVERS] = {"policyvers", &sel_policyvers_ops, S_IRUGO},
> -		[SEL_COMMIT_BOOLS] = {"commit_pending_bools", &sel_commit_bools_ops, S_IWUSR},
> -		[SEL_MLS] = {"mls", &sel_mls_ops, S_IRUGO},
> -		[SEL_DISABLE] = {"disable", &sel_disable_ops, S_IWUSR},
> -		[SEL_MEMBER] = {"member", &transaction_ops, S_IRUGO|S_IWUGO},
> -		[SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, S_IRUGO|S_IWUSR},
> -		[SEL_REJECT_UNKNOWN] = {"reject_unknown", &sel_handle_unknown_ops, S_IRUGO},
> -		[SEL_DENY_UNKNOWN] = {"deny_unknown", &sel_handle_unknown_ops, S_IRUGO},
> -		[SEL_STATUS] = {"status", &sel_handle_status_ops, S_IRUGO},
> -		[SEL_POLICY] = {"policy", &sel_policy_ops, S_IRUGO},
> -		[SEL_VALIDATE_TRANS] = {"validatetrans", &sel_transition_ops,
> -					S_IWUGO},
> +		[SEL_LOAD] = {"load", &sel_load_ops, 0600},
> +		[SEL_ENFORCE] = {"enforce", &sel_enforce_ops, 0644},
> +		[SEL_CONTEXT] = {"context", &transaction_ops, 0666},
> +		[SEL_ACCESS] = {"access", &transaction_ops, 0666},
> +		[SEL_CREATE] = {"create", &transaction_ops, 0666},
> +		[SEL_RELABEL] = {"relabel", &transaction_ops, 0666},
> +		[SEL_USER] = {"user", &transaction_ops, 0666},
> +		[SEL_POLICYVERS] = {"policyvers", &sel_policyvers_ops, 0444},
> +		[SEL_COMMIT_BOOLS] = {"commit_pending_bools", &sel_commit_bools_ops, 0200},
> +		[SEL_MLS] = {"mls", &sel_mls_ops, 0444},
> +		[SEL_DISABLE] = {"disable", &sel_disable_ops, 0200},
> +		[SEL_MEMBER] = {"member", &transaction_ops, 0666},
> +		[SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, 0644},
> +		[SEL_REJECT_UNKNOWN] = {"reject_unknown", &sel_handle_unknown_ops, 0444},
> +		[SEL_DENY_UNKNOWN] = {"deny_unknown", &sel_handle_unknown_ops, 0444},
> +		[SEL_STATUS] = {"status", &sel_handle_status_ops, 0444},
> +		[SEL_POLICY] = {"policy", &sel_policy_ops, 0444},
> +		[SEL_VALIDATE_TRANS] = {"validatetrans", &sel_transition_ops, 0222},
>  		/* last one */ {""}
>  	};
>  
> @@ -1943,7 +1942,7 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
>  		goto err;
>  
>  	ret = -ENOMEM;
> -	inode = sel_make_inode(sb, S_IFCHR | S_IRUGO | S_IWUGO);
> +	inode = sel_make_inode(sb, S_IFCHR | 0666);
>  	if (!inode)
>  		goto err;
>  
> @@ -1953,7 +1952,7 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
>  	isec->sclass = SECCLASS_CHR_FILE;
>  	isec->initialized = LABEL_INITIALIZED;
>  
> -	init_special_inode(inode, S_IFCHR | S_IRUGO | S_IWUGO, MKDEV(MEM_MAJOR, 3));
> +	init_special_inode(inode, S_IFCHR | 0666, MKDEV(MEM_MAJOR, 3));
>  	d_add(dentry, inode);
>  
>  	dentry = sel_make_dir(sb->s_root, "avc", &fsi->last_ino);
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index dcb976f98df2..8953440c6559 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -2945,11 +2945,11 @@ static int smack_flags_to_may(int flags)
>  {
>  	int may = 0;
>  
> -	if (flags & S_IRUGO)
> +	if (flags & 0444)
>  		may |= MAY_READ;
> -	if (flags & S_IWUGO)
> +	if (flags & 0222)
>  		may |= MAY_WRITE;
> -	if (flags & S_IXUGO)
> +	if (flags & 0111)
>  		may |= MAY_EXEC;
>  
>  	return may;
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index f6482e53d55a..270cd3a308f0 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -2857,55 +2857,53 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
>  
>  	static const struct tree_descr smack_files[] = {
>  		[SMK_LOAD] = {
> -			"load", &smk_load_ops, S_IRUGO|S_IWUSR},
> +			"load", &smk_load_ops, 0644},
>  		[SMK_CIPSO] = {
> -			"cipso", &smk_cipso_ops, S_IRUGO|S_IWUSR},
> +			"cipso", &smk_cipso_ops, 0644},
>  		[SMK_DOI] = {
> -			"doi", &smk_doi_ops, S_IRUGO|S_IWUSR},
> +			"doi", &smk_doi_ops, 0644},
>  		[SMK_DIRECT] = {
> -			"direct", &smk_direct_ops, S_IRUGO|S_IWUSR},
> +			"direct", &smk_direct_ops, 0644},
>  		[SMK_AMBIENT] = {
> -			"ambient", &smk_ambient_ops, S_IRUGO|S_IWUSR},
> +			"ambient", &smk_ambient_ops, 0644},
>  		[SMK_NET4ADDR] = {
> -			"netlabel", &smk_net4addr_ops, S_IRUGO|S_IWUSR},
> +			"netlabel", &smk_net4addr_ops, 0644},
>  		[SMK_ONLYCAP] = {
> -			"onlycap", &smk_onlycap_ops, S_IRUGO|S_IWUSR},
> +			"onlycap", &smk_onlycap_ops, 0644},
>  		[SMK_LOGGING] = {
> -			"logging", &smk_logging_ops, S_IRUGO|S_IWUSR},
> +			"logging", &smk_logging_ops, 0644},
>  		[SMK_LOAD_SELF] = {
> -			"load-self", &smk_load_self_ops, S_IRUGO|S_IWUGO},
> +			"load-self", &smk_load_self_ops, 0666},
>  		[SMK_ACCESSES] = {
> -			"access", &smk_access_ops, S_IRUGO|S_IWUGO},
> +			"access", &smk_access_ops, 0666},
>  		[SMK_MAPPED] = {
> -			"mapped", &smk_mapped_ops, S_IRUGO|S_IWUSR},
> +			"mapped", &smk_mapped_ops, 0644},
>  		[SMK_LOAD2] = {
> -			"load2", &smk_load2_ops, S_IRUGO|S_IWUSR},
> +			"load2", &smk_load2_ops, 0644},
>  		[SMK_LOAD_SELF2] = {
> -			"load-self2", &smk_load_self2_ops, S_IRUGO|S_IWUGO},
> +			"load-self2", &smk_load_self2_ops, 0666},
>  		[SMK_ACCESS2] = {
> -			"access2", &smk_access2_ops, S_IRUGO|S_IWUGO},
> +			"access2", &smk_access2_ops, 0666},
>  		[SMK_CIPSO2] = {
> -			"cipso2", &smk_cipso2_ops, S_IRUGO|S_IWUSR},
> +			"cipso2", &smk_cipso2_ops, 0644},
>  		[SMK_REVOKE_SUBJ] = {
> -			"revoke-subject", &smk_revoke_subj_ops,
> -			S_IRUGO|S_IWUSR},
> +			"revoke-subject", &smk_revoke_subj_ops, 0644},
>  		[SMK_CHANGE_RULE] = {
> -			"change-rule", &smk_change_rule_ops, S_IRUGO|S_IWUSR},
> +			"change-rule", &smk_change_rule_ops, 0644},
>  		[SMK_SYSLOG] = {
> -			"syslog", &smk_syslog_ops, S_IRUGO|S_IWUSR},
> +			"syslog", &smk_syslog_ops, 0644},
>  		[SMK_PTRACE] = {
> -			"ptrace", &smk_ptrace_ops, S_IRUGO|S_IWUSR},
> +			"ptrace", &smk_ptrace_ops, 0644},
>  #ifdef CONFIG_SECURITY_SMACK_BRINGUP
>  		[SMK_UNCONFINED] = {
> -			"unconfined", &smk_unconfined_ops, S_IRUGO|S_IWUSR},
> +			"unconfined", &smk_unconfined_ops, 0644},
>  #endif
>  #if IS_ENABLED(CONFIG_IPV6)
>  		[SMK_NET6ADDR] = {
> -			"ipv6host", &smk_net6addr_ops, S_IRUGO|S_IWUSR},
> +			"ipv6host", &smk_net6addr_ops, 0644},
>  #endif /* CONFIG_IPV6 */
>  		[SMK_RELABEL_SELF] = {
> -			"relabel-self", &smk_relabel_self_ops,
> -				S_IRUGO|S_IWUGO},
> +			"relabel-self", &smk_relabel_self_ops, 0666},
>  		/* last one */
>  			{""}
>  	};
> diff --git a/security/tomoyo/condition.c b/security/tomoyo/condition.c
> index 8d0e1b9c9c57..2069f5912469 100644
> --- a/security/tomoyo/condition.c
> +++ b/security/tomoyo/condition.c
> @@ -874,31 +874,31 @@ bool tomoyo_condition(struct tomoyo_request_info *r,
>  				value = S_ISVTX;
>  				break;
>  			case TOMOYO_MODE_OWNER_READ:
> -				value = S_IRUSR;
> +				value = 0400;
>  				break;
>  			case TOMOYO_MODE_OWNER_WRITE:
> -				value = S_IWUSR;
> +				value = 0200;
>  				break;
>  			case TOMOYO_MODE_OWNER_EXECUTE:
> -				value = S_IXUSR;
> +				value = 0100;
>  				break;
>  			case TOMOYO_MODE_GROUP_READ:
> -				value = S_IRGRP;
> +				value = 0040;
>  				break;
>  			case TOMOYO_MODE_GROUP_WRITE:
> -				value = S_IWGRP;
> +				value = 0020;
>  				break;
>  			case TOMOYO_MODE_GROUP_EXECUTE:
> -				value = S_IXGRP;
> +				value = 0010;
>  				break;
>  			case TOMOYO_MODE_OTHERS_READ:
> -				value = S_IROTH;
> +				value = 0004;
>  				break;
>  			case TOMOYO_MODE_OTHERS_WRITE:
> -				value = S_IWOTH;
> +				value = 0002;
>  				break;
>  			case TOMOYO_MODE_OTHERS_EXECUTE:
> -				value = S_IXOTH;
> +				value = 0001;
>  				break;
>  			case TOMOYO_EXEC_ARGC:
>  				if (!bprm)
> -- 
> 2.15.0
Paul Moore June 13, 2018, 3:49 p.m. UTC | #8
On Tue, Jun 12, 2018 at 8:29 PM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2018-06-12 at 17:12 -0400, Paul Moore wrote:
>> Joe, in general I really appreciate the fixes you send, but these
>> patches that cross a lot of subsystem boundaries (this isn't the first
>> one that does this) causes unnecessary conflicts in -next and during
>> the merge window.  Could you split your patches up from now on please?
>
> Sorry. No.  Merge conflicts are inherent in this system.

Yes, merge conflicts are inherent in this system when one makes a
single change which impacts multiple subsystems, e.g. changing a core
kernel function which is called by multiple subsystems.  However, that
isn't what this patch does, it makes a number of self-contained
changes across multiple subsystems; there are no cross-subsystem
dependencies in this patch.  You are increasing the likelihood of
conflicts for no good reason; that is why I'm asking you to split this
patch and others like it.

> There is just no good way to do this as sending a set
> of per subsystem patches guarantees partial application
> of the entire set.

Please explain why all of these changes need to be made at the same
time?  Looking quickly at the patch it would appear that each chunk of
the patch could be applied independently and the kernel would still
build and operate correctly.  I'm not suggesting that you decompose
this patch with that level of granularity, that would be silly, but
splitting this patch (and many of the others you tend to submit) at
subsystem boundaries should be possible.

Further, as one could see from the responses of the other LSM
maintainers, splitting this patch is not just possible, it is
desirable.

> If you prefer, each sub-subsystem maintainer, at
> whatever granularity desired, could apply the patch
> to the appropriate subsystem by using
> "git am --include=<subsystem_path>".

Or you could split the patch up by subsystem before posting, like so
many other developers do already.
Joe Perches June 13, 2018, 4:04 p.m. UTC | #9
On Wed, 2018-06-13 at 11:49 -0400, Paul Moore wrote:
> On Tue, Jun 12, 2018 at 8:29 PM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2018-06-12 at 17:12 -0400, Paul Moore wrote:
> > > Joe, in general I really appreciate the fixes you send, but these
> > > patches that cross a lot of subsystem boundaries (this isn't the first
> > > one that does this) causes unnecessary conflicts in -next and during
> > > the merge window.  Could you split your patches up from now on please?
> > 
> > Sorry. No.  Merge conflicts are inherent in this system.
> 
> Yes, merge conflicts are inherent in this system when one makes a
> single change which impacts multiple subsystems, e.g. changing a core
> kernel function which is called by multiple subsystems.  However, that
> isn't what this patch does, it makes a number of self-contained
> changes across multiple subsystems; there are no cross-subsystem
> dependencies in this patch.  You are increasing the likelihood of
> conflicts for no good reason; that is why I'm asking you to split this
> patch and others like it.

No.  History shows with high certainty that splitting
patches like this across multiple subsystems of a primary
subsystem means that the entire patchset is not completely
applied.

It's _much_ simpler and provides a generic mechanism to
get the entire patch applied to send a single patch to the
top level subsystem maintainer.
Paul Moore June 13, 2018, 4:19 p.m. UTC | #10
On Wed, Jun 13, 2018 at 12:04 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2018-06-13 at 11:49 -0400, Paul Moore wrote:
>> On Tue, Jun 12, 2018 at 8:29 PM, Joe Perches <joe@perches.com> wrote:
>> > On Tue, 2018-06-12 at 17:12 -0400, Paul Moore wrote:
>> > > Joe, in general I really appreciate the fixes you send, but these
>> > > patches that cross a lot of subsystem boundaries (this isn't the first
>> > > one that does this) causes unnecessary conflicts in -next and during
>> > > the merge window.  Could you split your patches up from now on please?
>> >
>> > Sorry. No.  Merge conflicts are inherent in this system.
>>
>> Yes, merge conflicts are inherent in this system when one makes a
>> single change which impacts multiple subsystems, e.g. changing a core
>> kernel function which is called by multiple subsystems.  However, that
>> isn't what this patch does, it makes a number of self-contained
>> changes across multiple subsystems; there are no cross-subsystem
>> dependencies in this patch.  You are increasing the likelihood of
>> conflicts for no good reason; that is why I'm asking you to split this
>> patch and others like it.
>
> No.  History shows with high certainty that splitting
> patches like this across multiple subsystems of a primary
> subsystem means that the entire patchset is not completely
> applied.

I think that is due more to a lack of effort on the part of the patch
author to keep pushing the individual patches.

> It's _much_ simpler and provides a generic mechanism to
> get the entire patch applied to send a single patch to the
> top level subsystem maintainer.

I understand it is simpler for you, but it is more difficult for everyone else.

Further, where the LSMs are concerned, there is no "top level
subsystem maintainer" anymore.  SELinux and AppArmor send pull
requests directly to Linus.
Joe Perches June 13, 2018, 7:30 p.m. UTC | #11
On Wed, 2018-06-13 at 12:19 -0400, Paul Moore wrote:
> On Wed, Jun 13, 2018 at 12:04 PM, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2018-06-13 at 11:49 -0400, Paul Moore wrote:
> > > On Tue, Jun 12, 2018 at 8:29 PM, Joe Perches <joe@perches.com> wrote:
> > > > On Tue, 2018-06-12 at 17:12 -0400, Paul Moore wrote:
> > > > > Joe, in general I really appreciate the fixes you send, but these
> > > > > patches that cross a lot of subsystem boundaries (this isn't the first
> > > > > one that does this) causes unnecessary conflicts in -next and during
> > > > > the merge window.  Could you split your patches up from now on please?
> > > > 
> > > > Sorry. No.  Merge conflicts are inherent in this system.
> > > 
> > > Yes, merge conflicts are inherent in this system when one makes a
> > > single change which impacts multiple subsystems, e.g. changing a core
> > > kernel function which is called by multiple subsystems.  However, that
> > > isn't what this patch does, it makes a number of self-contained
> > > changes across multiple subsystems; there are no cross-subsystem
> > > dependencies in this patch.  You are increasing the likelihood of
> > > conflicts for no good reason; that is why I'm asking you to split this
> > > patch and others like it.
> > 
> > No.  History shows with high certainty that splitting
> > patches like this across multiple subsystems of a primary
> > subsystem means that the entire patchset is not completely
> > applied.
> 
> I think that is due more to a lack of effort on the part of the patch
> author to keep pushing the individual patches.

Nope.  Try again.

Resistance to change and desire for status quo
occurs in many subsystems.

> > It's _much_ simpler and provides a generic mechanism to
> > get the entire patch applied to send a single patch to the
> > top level subsystem maintainer.
> 
> I understand it is simpler for you, but it is more difficult for everyone else.

Not true.

It's simply a matter of merge resolution being pushed down
where and when necessary.

See changes like the additions of the SPDX license tags.

> Further, where the LSMs are concerned, there is no "top level
> subsystem maintainer" anymore.  SELinux and AppArmor send pull
> requests directly to Linus.

MAINTAINERS-SECURITY SUBSYSTEM
MAINTAINERS-M:  James Morris <jmorris@namei.org>
MAINTAINERS-M:  "Serge E. Hallyn" <serge@hallyn.com>
MAINTAINERS-L:  linux-security-module@vger.kernel.org (suggested Cc:)
MAINTAINERS-T:  git git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git
MAINTAINERS-W:  http://kernsec.org/
MAINTAINERS-S:  Supported
MAINTAINERS:F:  security/
MAINTAINERS-

If James is not approving or merging security/selinux or
security/tomoyo then perhaps the F: entries could be
augmented with appropriate X: entries or made specific
by using specific entries like:

F:	security/*
F:	security/integrity/
F:	security/keys/
Paul Moore June 13, 2018, 7:57 p.m. UTC | #12
On Wed, Jun 13, 2018 at 3:30 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2018-06-13 at 12:19 -0400, Paul Moore wrote:
>> On Wed, Jun 13, 2018 at 12:04 PM, Joe Perches <joe@perches.com> wrote:
>> > On Wed, 2018-06-13 at 11:49 -0400, Paul Moore wrote:
>> > > On Tue, Jun 12, 2018 at 8:29 PM, Joe Perches <joe@perches.com> wrote:
>> > > > On Tue, 2018-06-12 at 17:12 -0400, Paul Moore wrote:
>> > > > > Joe, in general I really appreciate the fixes you send, but these
>> > > > > patches that cross a lot of subsystem boundaries (this isn't the first
>> > > > > one that does this) causes unnecessary conflicts in -next and during
>> > > > > the merge window.  Could you split your patches up from now on please?
>> > > >
>> > > > Sorry. No.  Merge conflicts are inherent in this system.
>> > >
>> > > Yes, merge conflicts are inherent in this system when one makes a
>> > > single change which impacts multiple subsystems, e.g. changing a core
>> > > kernel function which is called by multiple subsystems.  However, that
>> > > isn't what this patch does, it makes a number of self-contained
>> > > changes across multiple subsystems; there are no cross-subsystem
>> > > dependencies in this patch.  You are increasing the likelihood of
>> > > conflicts for no good reason; that is why I'm asking you to split this
>> > > patch and others like it.
>> >
>> > No.  History shows with high certainty that splitting
>> > patches like this across multiple subsystems of a primary
>> > subsystem means that the entire patchset is not completely
>> > applied.
>>
>> I think that is due more to a lack of effort on the part of the patch
>> author to keep pushing the individual patches.
>
> Nope.  Try again.
>
> Resistance to change and desire for status quo
> occurs in many subsystems.

Which gets back to the need for persistence on the part of the patch
author.  If your solution to a stubborn susbsystem is to go around
them by convincing another, potentially unrelated subsystem, to merge
the patch then I firmly believe you are doing it wrong.

>> > It's _much_ simpler and provides a generic mechanism to
>> > get the entire patch applied to send a single patch to the
>> > top level subsystem maintainer.
>>
>> I understand it is simpler for you, but it is more difficult for everyone else.
>
> Not true.

I think we are at the agree to disagree stage.

The way you have structured this patch it makes it easier for you to
submit, but makes it potentially more difficult for me (likely other
LSM maintainers too), the -next folks, and Linus.

> It's simply a matter of merge resolution being pushed down
> where and when necessary.
>
> See changes like the additions of the SPDX license tags.

Please don't even try to suggest that this trivial patch you are
proposing is even remotely as significant as the SPDX change.  There
are always going to be exceptions to every rule, and with each
exception there needs to be a solid reason behind the change.  The
SPDX change had a legitimate reason (legal concern) for doing it the
way it was done; this patch isn't close to that level of concern.

>> Further, where the LSMs are concerned, there is no "top level
>> subsystem maintainer" anymore.  SELinux and AppArmor send pull
>> requests directly to Linus.
>
> MAINTAINERS-SECURITY SUBSYSTEM
> MAINTAINERS-M:  James Morris <jmorris@namei.org>
> MAINTAINERS-M:  "Serge E. Hallyn" <serge@hallyn.com>
> MAINTAINERS-L:  linux-security-module@vger.kernel.org (suggested Cc:)
> MAINTAINERS-T:  git git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git
> MAINTAINERS-W:  http://kernsec.org/
> MAINTAINERS-S:  Supported
> MAINTAINERS:F:  security/
> MAINTAINERS-
>
> If James is not approving or merging security/selinux or
> security/tomoyo then perhaps the F: entries could be
> augmented with appropriate X: entries or made specific
> by using specific entries like:
>
> F:      security/*
> F:      security/integrity/
> F:      security/keys/

That is a good point.  I'll put together a patch for selinux/next as
soon as the merge window closes.  I'll let the other LSMs do as they
see fit.  As I said previously, I believe the only other LSM that
sends directly to Linux is AppArmor.
Casey Schaufler June 13, 2018, 9:14 p.m. UTC | #13
On 6/13/2018 12:57 PM, Paul Moore wrote:
> On Wed, Jun 13, 2018 at 3:30 PM, Joe Perches <joe@perches.com> wrote:
>> On Wed, 2018-06-13 at 12:19 -0400, Paul Moore wrote:
>>> On Wed, Jun 13, 2018 at 12:04 PM, Joe Perches <joe@perches.com> wrote:
>>>> On Wed, 2018-06-13 at 11:49 -0400, Paul Moore wrote:
>>>>> On Tue, Jun 12, 2018 at 8:29 PM, Joe Perches <joe@perches.com> wrote:
>>>>>> On Tue, 2018-06-12 at 17:12 -0400, Paul Moore wrote:
>>>>>>> Joe, in general I really appreciate the fixes you send, but these
>>>>>>> patches that cross a lot of subsystem boundaries (this isn't the first
>>>>>>> one that does this) causes unnecessary conflicts in -next and during
>>>>>>> the merge window.  Could you split your patches up from now on please?
>>>>>> Sorry. No.  Merge conflicts are inherent in this system.
>>>>> Yes, merge conflicts are inherent in this system when one makes a
>>>>> single change which impacts multiple subsystems, e.g. changing a core
>>>>> kernel function which is called by multiple subsystems.  However, that
>>>>> isn't what this patch does, it makes a number of self-contained
>>>>> changes across multiple subsystems; there are no cross-subsystem
>>>>> dependencies in this patch.  You are increasing the likelihood of
>>>>> conflicts for no good reason; that is why I'm asking you to split this
>>>>> patch and others like it.
>>>> No.  History shows with high certainty that splitting
>>>> patches like this across multiple subsystems of a primary
>>>> subsystem means that the entire patchset is not completely
>>>> applied.
>>> I think that is due more to a lack of effort on the part of the patch
>>> author to keep pushing the individual patches.
>> Nope.  Try again.
>>
>> Resistance to change and desire for status quo
>> occurs in many subsystems.
> Which gets back to the need for persistence on the part of the patch
> author.  If your solution to a stubborn susbsystem is to go around
> them by convincing another, potentially unrelated subsystem, to merge
> the patch then I firmly believe you are doing it wrong.
>
>>>> It's _much_ simpler and provides a generic mechanism to
>>>> get the entire patch applied to send a single patch to the
>>>> top level subsystem maintainer.
>>> I understand it is simpler for you, but it is more difficult for everyone else.
>> Not true.
> I think we are at the agree to disagree stage.
>
> The way you have structured this patch it makes it easier for you to
> submit, but makes it potentially more difficult for me (likely other
> LSM maintainers too), the -next folks, and Linus.

I am agreeing with Paul. There is no reason that I/he should
be compelled to accept the Smack/SELinux patches because he/I
accepted the SELinux/Smack bits.

>
>> It's simply a matter of merge resolution being pushed down
>> where and when necessary.
>>
>> See changes like the additions of the SPDX license tags.
> Please don't even try to suggest that this trivial patch you are
> proposing is even remotely as significant as the SPDX change.  There
> are always going to be exceptions to every rule, and with each
> exception there needs to be a solid reason behind the change.  The
> SPDX change had a legitimate reason (legal concern) for doing it the
> way it was done; this patch isn't close to that level of concern.
>
>>> Further, where the LSMs are concerned, there is no "top level
>>> subsystem maintainer" anymore.  SELinux and AppArmor send pull
>>> requests directly to Linus.
>> MAINTAINERS-SECURITY SUBSYSTEM
>> MAINTAINERS-M:  James Morris <jmorris@namei.org>
>> MAINTAINERS-M:  "Serge E. Hallyn" <serge@hallyn.com>
>> MAINTAINERS-L:  linux-security-module@vger.kernel.org (suggested Cc:)
>> MAINTAINERS-T:  git git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git
>> MAINTAINERS-W:  http://kernsec.org/
>> MAINTAINERS-S:  Supported
>> MAINTAINERS:F:  security/
>> MAINTAINERS-
>>
>> If James is not approving or merging security/selinux or
>> security/tomoyo then perhaps the F: entries could be
>> augmented with appropriate X: entries or made specific
>> by using specific entries like:
>>
>> F:      security/*
>> F:      security/integrity/
>> F:      security/keys/

There are already F: entries for security/selinux, security/smack
and security/apparmor so I don't get your point.

> That is a good point.  I'll put together a patch for selinux/next as
> soon as the merge window closes.  I'll let the other LSMs do as they
> see fit.  As I said previously, I believe the only other LSM that
> sends directly to Linux is AppArmor.

Smack will continue using the security subsystem so long as
James offers the service. There's overhead in setting up the
environment for sending directly to Linus that I'm in no rush
to incur.
Joe Perches June 13, 2018, 11:49 p.m. UTC | #14
On Wed, 2018-06-13 at 10:19 -0500, Serge E. Hallyn wrote:
> Using octal and not symbolic permissions is preferred by many as more
> > readable.
> > 
> > see: https://lkml.org/lkml/2016/8/2/1945
> 
> Heh, well see also https://lkml.org/lkml/2016/8/2/2062 .

We all love Al.

I did reply then too.

cheers, Joe

Patch
diff mbox

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 949dd8a48164..c09dc0f3c3fe 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -2426,10 +2426,9 @@  static int aa_mk_null_file(struct dentry *parent)
 	}
 
 	inode->i_ino = get_next_ino();
-	inode->i_mode = S_IFCHR | S_IRUGO | S_IWUGO;
+	inode->i_mode = S_IFCHR | 0666;
 	inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
-	init_special_inode(inode, S_IFCHR | S_IRUGO | S_IWUGO,
-			   MKDEV(MEM_MAJOR, 3));
+	init_special_inode(inode, S_IFCHR | 0666, MKDEV(MEM_MAJOR, 3));
 	d_instantiate(dentry, inode);
 	aa_null.dentry = dget(dentry);
 	aa_null.mnt = mntget(mount);
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index fbb08bc78bee..6759a70918de 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1255,45 +1255,42 @@  static int param_get_mode(char *buffer, const struct kernel_param *kp);
 /* AppArmor global enforcement switch - complain, enforce, kill */
 enum profile_mode aa_g_profile_mode = APPARMOR_ENFORCE;
 module_param_call(mode, param_set_mode, param_get_mode,
-		  &aa_g_profile_mode, S_IRUSR | S_IWUSR);
+		  &aa_g_profile_mode, 0600);
 
 /* whether policy verification hashing is enabled */
 bool aa_g_hash_policy = IS_ENABLED(CONFIG_SECURITY_APPARMOR_HASH_DEFAULT);
 #ifdef CONFIG_SECURITY_APPARMOR_HASH
-module_param_named(hash_policy, aa_g_hash_policy, aabool, S_IRUSR | S_IWUSR);
+module_param_named(hash_policy, aa_g_hash_policy, aabool, 0600);
 #endif
 
 /* Debug mode */
 bool aa_g_debug = IS_ENABLED(CONFIG_SECURITY_APPARMOR_DEBUG_MESSAGES);
-module_param_named(debug, aa_g_debug, aabool, S_IRUSR | S_IWUSR);
+module_param_named(debug, aa_g_debug, aabool, 0600);
 
 /* Audit mode */
 enum audit_mode aa_g_audit;
-module_param_call(audit, param_set_audit, param_get_audit,
-		  &aa_g_audit, S_IRUSR | S_IWUSR);
+module_param_call(audit, param_set_audit, param_get_audit, &aa_g_audit, 0600);
 
 /* Determines if audit header is included in audited messages.  This
  * provides more context if the audit daemon is not running
  */
 bool aa_g_audit_header = true;
-module_param_named(audit_header, aa_g_audit_header, aabool,
-		   S_IRUSR | S_IWUSR);
+module_param_named(audit_header, aa_g_audit_header, aabool, 0600);
 
 /* lock out loading/removal of policy
  * TODO: add in at boot loading of policy, which is the only way to
  *       load policy, if lock_policy is set
  */
 bool aa_g_lock_policy;
-module_param_named(lock_policy, aa_g_lock_policy, aalockpolicy,
-		   S_IRUSR | S_IWUSR);
+module_param_named(lock_policy, aa_g_lock_policy, aalockpolicy, 0600);
 
 /* Syscall logging mode */
 bool aa_g_logsyscall;
-module_param_named(logsyscall, aa_g_logsyscall, aabool, S_IRUSR | S_IWUSR);
+module_param_named(logsyscall, aa_g_logsyscall, aabool, 0600);
 
 /* Maximum pathname length before accesses will start getting rejected */
 unsigned int aa_g_path_max = 2 * PATH_MAX;
-module_param_named(path_max, aa_g_path_max, aauint, S_IRUSR);
+module_param_named(path_max, aa_g_path_max, aauint, 0400);
 
 /* Determines how paranoid loading of policy is and how much verification
  * on the loaded policy is done.
@@ -1301,11 +1298,11 @@  module_param_named(path_max, aa_g_path_max, aauint, S_IRUSR);
  * that none root users (user namespaces) can load policy.
  */
 bool aa_g_paranoid_load = true;
-module_param_named(paranoid_load, aa_g_paranoid_load, aabool, S_IRUGO);
+module_param_named(paranoid_load, aa_g_paranoid_load, aabool, 0444);
 
 /* Boot time disable flag */
 static bool apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE;
-module_param_named(enabled, apparmor_enabled, bool, S_IRUGO);
+module_param_named(enabled, apparmor_enabled, bool, 0444);
 
 static int __init apparmor_enabled_setup(char *str)
 {
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 354bb5716ce3..3f7707b8aaa7 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -314,9 +314,9 @@  static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
 #endif /* CONFIG_IMA_LSM_RULES */
 
 #ifdef	CONFIG_IMA_READ_POLICY
-#define	POLICY_FILE_FLAGS	(S_IWUSR | S_IRUSR)
+#define	POLICY_FILE_FLAGS	0600
 #else
-#define	POLICY_FILE_FLAGS	S_IWUSR
+#define	POLICY_FILE_FLAGS	0200
 #endif /* CONFIG_IMA_READ_POLICY */
 
 #endif /* __LINUX_IMA_H */
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index ae9d5c766a3c..81700df83f51 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -439,7 +439,7 @@  static int ima_release_policy(struct inode *inode, struct file *file)
 #elif defined(CONFIG_IMA_WRITE_POLICY)
 	clear_bit(IMA_FS_BUSY, &ima_fs_flags);
 #elif defined(CONFIG_IMA_READ_POLICY)
-	inode->i_mode &= ~S_IWUSR;
+	inode->i_mode &= ~0200;
 #endif
 	return 0;
 }
@@ -465,28 +465,29 @@  int __init ima_fs_init(void)
 
 	binary_runtime_measurements =
 	    securityfs_create_file("binary_runtime_measurements",
-				   S_IRUSR | S_IRGRP, ima_dir, NULL,
+				   0440, ima_dir, NULL,
 				   &ima_measurements_ops);
 	if (IS_ERR(binary_runtime_measurements))
 		goto out;
 
 	ascii_runtime_measurements =
 	    securityfs_create_file("ascii_runtime_measurements",
-				   S_IRUSR | S_IRGRP, ima_dir, NULL,
+				   0440, ima_dir, NULL,
 				   &ima_ascii_measurements_ops);
 	if (IS_ERR(ascii_runtime_measurements))
 		goto out;
 
 	runtime_measurements_count =
 	    securityfs_create_file("runtime_measurements_count",
-				   S_IRUSR | S_IRGRP, ima_dir, NULL,
+				   0440, ima_dir, NULL,
 				   &ima_measurements_count_ops);
 	if (IS_ERR(runtime_measurements_count))
 		goto out;
 
 	violations =
-	    securityfs_create_file("violations", S_IRUSR | S_IRGRP,
-				   ima_dir, NULL, &ima_htable_violations_ops);
+		securityfs_create_file("violations",
+				       0440, ima_dir, NULL,
+				       &ima_htable_violations_ops);
 	if (IS_ERR(violations))
 		goto out;
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a85fac3345df..8ae043be8782 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6336,9 +6336,9 @@  static int selinux_ipc_permission(struct kern_ipc_perm *ipcp, short flag)
 	u32 av = 0;
 
 	av = 0;
-	if (flag & S_IRUGO)
+	if (flag & 0444)
 		av |= IPC__UNIX_READ;
-	if (flag & S_IWUGO)
+	if (flag & 0222)
 		av |= IPC__UNIX_WRITE;
 
 	if (av == 0)
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index f3d374d2ca04..bfecac19ba92 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -1376,7 +1376,7 @@  static int sel_make_bools(struct selinux_fs_info *fsi)
 			goto out;
 
 		ret = -ENOMEM;
-		inode = sel_make_inode(dir->d_sb, S_IFREG | S_IRUGO | S_IWUSR);
+		inode = sel_make_inode(dir->d_sb, S_IFREG | 0644);
 		if (!inode)
 			goto out;
 
@@ -1582,10 +1582,10 @@  static int sel_make_avc_files(struct dentry *dir)
 	int i;
 	static const struct tree_descr files[] = {
 		{ "cache_threshold",
-		  &sel_avc_cache_threshold_ops, S_IRUGO|S_IWUSR },
-		{ "hash_stats", &sel_avc_hash_stats_ops, S_IRUGO },
+		  &sel_avc_cache_threshold_ops, 0644 },
+		{ "hash_stats", &sel_avc_hash_stats_ops, 0444 },
 #ifdef CONFIG_SECURITY_SELINUX_AVC_STATS
-		{ "cache_stats", &sel_avc_cache_stats_ops, S_IRUGO },
+		{ "cache_stats", &sel_avc_cache_stats_ops, 0444 },
 #endif
 	};
 
@@ -1643,7 +1643,7 @@  static int sel_make_initcon_files(struct dentry *dir)
 		if (!dentry)
 			return -ENOMEM;
 
-		inode = sel_make_inode(dir->d_sb, S_IFREG|S_IRUGO);
+		inode = sel_make_inode(dir->d_sb, S_IFREG | 0444);
 		if (!inode)
 			return -ENOMEM;
 
@@ -1744,7 +1744,7 @@  static int sel_make_perm_files(char *objclass, int classvalue,
 			goto out;
 
 		rc = -ENOMEM;
-		inode = sel_make_inode(dir->d_sb, S_IFREG|S_IRUGO);
+		inode = sel_make_inode(dir->d_sb, S_IFREG | 0444);
 		if (!inode)
 			goto out;
 
@@ -1774,7 +1774,7 @@  static int sel_make_class_dir_entries(char *classname, int index,
 	if (!dentry)
 		return -ENOMEM;
 
-	inode = sel_make_inode(dir->d_sb, S_IFREG|S_IRUGO);
+	inode = sel_make_inode(dir->d_sb, S_IFREG | 0444);
 	if (!inode)
 		return -ENOMEM;
 
@@ -1870,7 +1870,7 @@  static struct dentry *sel_make_dir(struct dentry *dir, const char *name,
 	if (!dentry)
 		return ERR_PTR(-ENOMEM);
 
-	inode = sel_make_inode(dir->d_sb, S_IFDIR | S_IRUGO | S_IXUGO);
+	inode = sel_make_inode(dir->d_sb, S_IFDIR | 0555);
 	if (!inode) {
 		dput(dentry);
 		return ERR_PTR(-ENOMEM);
@@ -1899,25 +1899,24 @@  static int sel_fill_super(struct super_block *sb, void *data, int silent)
 	struct inode_security_struct *isec;
 
 	static const struct tree_descr selinux_files[] = {
-		[SEL_LOAD] = {"load", &sel_load_ops, S_IRUSR|S_IWUSR},
-		[SEL_ENFORCE] = {"enforce", &sel_enforce_ops, S_IRUGO|S_IWUSR},
-		[SEL_CONTEXT] = {"context", &transaction_ops, S_IRUGO|S_IWUGO},
-		[SEL_ACCESS] = {"access", &transaction_ops, S_IRUGO|S_IWUGO},
-		[SEL_CREATE] = {"create", &transaction_ops, S_IRUGO|S_IWUGO},
-		[SEL_RELABEL] = {"relabel", &transaction_ops, S_IRUGO|S_IWUGO},
-		[SEL_USER] = {"user", &transaction_ops, S_IRUGO|S_IWUGO},
-		[SEL_POLICYVERS] = {"policyvers", &sel_policyvers_ops, S_IRUGO},
-		[SEL_COMMIT_BOOLS] = {"commit_pending_bools", &sel_commit_bools_ops, S_IWUSR},
-		[SEL_MLS] = {"mls", &sel_mls_ops, S_IRUGO},
-		[SEL_DISABLE] = {"disable", &sel_disable_ops, S_IWUSR},
-		[SEL_MEMBER] = {"member", &transaction_ops, S_IRUGO|S_IWUGO},
-		[SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, S_IRUGO|S_IWUSR},
-		[SEL_REJECT_UNKNOWN] = {"reject_unknown", &sel_handle_unknown_ops, S_IRUGO},
-		[SEL_DENY_UNKNOWN] = {"deny_unknown", &sel_handle_unknown_ops, S_IRUGO},
-		[SEL_STATUS] = {"status", &sel_handle_status_ops, S_IRUGO},
-		[SEL_POLICY] = {"policy", &sel_policy_ops, S_IRUGO},
-		[SEL_VALIDATE_TRANS] = {"validatetrans", &sel_transition_ops,
-					S_IWUGO},
+		[SEL_LOAD] = {"load", &sel_load_ops, 0600},
+		[SEL_ENFORCE] = {"enforce", &sel_enforce_ops, 0644},
+		[SEL_CONTEXT] = {"context", &transaction_ops, 0666},
+		[SEL_ACCESS] = {"access", &transaction_ops, 0666},
+		[SEL_CREATE] = {"create", &transaction_ops, 0666},
+		[SEL_RELABEL] = {"relabel", &transaction_ops, 0666},
+		[SEL_USER] = {"user", &transaction_ops, 0666},
+		[SEL_POLICYVERS] = {"policyvers", &sel_policyvers_ops, 0444},
+		[SEL_COMMIT_BOOLS] = {"commit_pending_bools", &sel_commit_bools_ops, 0200},
+		[SEL_MLS] = {"mls", &sel_mls_ops, 0444},
+		[SEL_DISABLE] = {"disable", &sel_disable_ops, 0200},
+		[SEL_MEMBER] = {"member", &transaction_ops, 0666},
+		[SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, 0644},
+		[SEL_REJECT_UNKNOWN] = {"reject_unknown", &sel_handle_unknown_ops, 0444},
+		[SEL_DENY_UNKNOWN] = {"deny_unknown", &sel_handle_unknown_ops, 0444},
+		[SEL_STATUS] = {"status", &sel_handle_status_ops, 0444},
+		[SEL_POLICY] = {"policy", &sel_policy_ops, 0444},
+		[SEL_VALIDATE_TRANS] = {"validatetrans", &sel_transition_ops, 0222},
 		/* last one */ {""}
 	};
 
@@ -1943,7 +1942,7 @@  static int sel_fill_super(struct super_block *sb, void *data, int silent)
 		goto err;
 
 	ret = -ENOMEM;
-	inode = sel_make_inode(sb, S_IFCHR | S_IRUGO | S_IWUGO);
+	inode = sel_make_inode(sb, S_IFCHR | 0666);
 	if (!inode)
 		goto err;
 
@@ -1953,7 +1952,7 @@  static int sel_fill_super(struct super_block *sb, void *data, int silent)
 	isec->sclass = SECCLASS_CHR_FILE;
 	isec->initialized = LABEL_INITIALIZED;
 
-	init_special_inode(inode, S_IFCHR | S_IRUGO | S_IWUGO, MKDEV(MEM_MAJOR, 3));
+	init_special_inode(inode, S_IFCHR | 0666, MKDEV(MEM_MAJOR, 3));
 	d_add(dentry, inode);
 
 	dentry = sel_make_dir(sb->s_root, "avc", &fsi->last_ino);
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index dcb976f98df2..8953440c6559 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2945,11 +2945,11 @@  static int smack_flags_to_may(int flags)
 {
 	int may = 0;
 
-	if (flags & S_IRUGO)
+	if (flags & 0444)
 		may |= MAY_READ;
-	if (flags & S_IWUGO)
+	if (flags & 0222)
 		may |= MAY_WRITE;
-	if (flags & S_IXUGO)
+	if (flags & 0111)
 		may |= MAY_EXEC;
 
 	return may;
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index f6482e53d55a..270cd3a308f0 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -2857,55 +2857,53 @@  static int smk_fill_super(struct super_block *sb, void *data, int silent)
 
 	static const struct tree_descr smack_files[] = {
 		[SMK_LOAD] = {
-			"load", &smk_load_ops, S_IRUGO|S_IWUSR},
+			"load", &smk_load_ops, 0644},
 		[SMK_CIPSO] = {
-			"cipso", &smk_cipso_ops, S_IRUGO|S_IWUSR},
+			"cipso", &smk_cipso_ops, 0644},
 		[SMK_DOI] = {
-			"doi", &smk_doi_ops, S_IRUGO|S_IWUSR},
+			"doi", &smk_doi_ops, 0644},
 		[SMK_DIRECT] = {
-			"direct", &smk_direct_ops, S_IRUGO|S_IWUSR},
+			"direct", &smk_direct_ops, 0644},
 		[SMK_AMBIENT] = {
-			"ambient", &smk_ambient_ops, S_IRUGO|S_IWUSR},
+			"ambient", &smk_ambient_ops, 0644},
 		[SMK_NET4ADDR] = {
-			"netlabel", &smk_net4addr_ops, S_IRUGO|S_IWUSR},
+			"netlabel", &smk_net4addr_ops, 0644},
 		[SMK_ONLYCAP] = {
-			"onlycap", &smk_onlycap_ops, S_IRUGO|S_IWUSR},
+			"onlycap", &smk_onlycap_ops, 0644},
 		[SMK_LOGGING] = {
-			"logging", &smk_logging_ops, S_IRUGO|S_IWUSR},
+			"logging", &smk_logging_ops, 0644},
 		[SMK_LOAD_SELF] = {
-			"load-self", &smk_load_self_ops, S_IRUGO|S_IWUGO},
+			"load-self", &smk_load_self_ops, 0666},
 		[SMK_ACCESSES] = {
-			"access", &smk_access_ops, S_IRUGO|S_IWUGO},
+			"access", &smk_access_ops, 0666},
 		[SMK_MAPPED] = {
-			"mapped", &smk_mapped_ops, S_IRUGO|S_IWUSR},
+			"mapped", &smk_mapped_ops, 0644},
 		[SMK_LOAD2] = {
-			"load2", &smk_load2_ops, S_IRUGO|S_IWUSR},
+			"load2", &smk_load2_ops, 0644},
 		[SMK_LOAD_SELF2] = {
-			"load-self2", &smk_load_self2_ops, S_IRUGO|S_IWUGO},
+			"load-self2", &smk_load_self2_ops, 0666},
 		[SMK_ACCESS2] = {
-			"access2", &smk_access2_ops, S_IRUGO|S_IWUGO},
+			"access2", &smk_access2_ops, 0666},
 		[SMK_CIPSO2] = {
-			"cipso2", &smk_cipso2_ops, S_IRUGO|S_IWUSR},
+			"cipso2", &smk_cipso2_ops, 0644},
 		[SMK_REVOKE_SUBJ] = {
-			"revoke-subject", &smk_revoke_subj_ops,
-			S_IRUGO|S_IWUSR},
+			"revoke-subject", &smk_revoke_subj_ops, 0644},
 		[SMK_CHANGE_RULE] = {
-			"change-rule", &smk_change_rule_ops, S_IRUGO|S_IWUSR},
+			"change-rule", &smk_change_rule_ops, 0644},
 		[SMK_SYSLOG] = {
-			"syslog", &smk_syslog_ops, S_IRUGO|S_IWUSR},
+			"syslog", &smk_syslog_ops, 0644},
 		[SMK_PTRACE] = {
-			"ptrace", &smk_ptrace_ops, S_IRUGO|S_IWUSR},
+			"ptrace", &smk_ptrace_ops, 0644},
 #ifdef CONFIG_SECURITY_SMACK_BRINGUP
 		[SMK_UNCONFINED] = {
-			"unconfined", &smk_unconfined_ops, S_IRUGO|S_IWUSR},
+			"unconfined", &smk_unconfined_ops, 0644},
 #endif
 #if IS_ENABLED(CONFIG_IPV6)
 		[SMK_NET6ADDR] = {
-			"ipv6host", &smk_net6addr_ops, S_IRUGO|S_IWUSR},
+			"ipv6host", &smk_net6addr_ops, 0644},
 #endif /* CONFIG_IPV6 */
 		[SMK_RELABEL_SELF] = {
-			"relabel-self", &smk_relabel_self_ops,
-				S_IRUGO|S_IWUGO},
+			"relabel-self", &smk_relabel_self_ops, 0666},
 		/* last one */
 			{""}
 	};
diff --git a/security/tomoyo/condition.c b/security/tomoyo/condition.c
index 8d0e1b9c9c57..2069f5912469 100644
--- a/security/tomoyo/condition.c
+++ b/security/tomoyo/condition.c
@@ -874,31 +874,31 @@  bool tomoyo_condition(struct tomoyo_request_info *r,
 				value = S_ISVTX;
 				break;
 			case TOMOYO_MODE_OWNER_READ:
-				value = S_IRUSR;
+				value = 0400;
 				break;
 			case TOMOYO_MODE_OWNER_WRITE:
-				value = S_IWUSR;
+				value = 0200;
 				break;
 			case TOMOYO_MODE_OWNER_EXECUTE:
-				value = S_IXUSR;
+				value = 0100;
 				break;
 			case TOMOYO_MODE_GROUP_READ:
-				value = S_IRGRP;
+				value = 0040;
 				break;
 			case TOMOYO_MODE_GROUP_WRITE:
-				value = S_IWGRP;
+				value = 0020;
 				break;
 			case TOMOYO_MODE_GROUP_EXECUTE:
-				value = S_IXGRP;
+				value = 0010;
 				break;
 			case TOMOYO_MODE_OTHERS_READ:
-				value = S_IROTH;
+				value = 0004;
 				break;
 			case TOMOYO_MODE_OTHERS_WRITE:
-				value = S_IWOTH;
+				value = 0002;
 				break;
 			case TOMOYO_MODE_OTHERS_EXECUTE:
-				value = S_IXOTH;
+				value = 0001;
 				break;
 			case TOMOYO_EXEC_ARGC:
 				if (!bprm)