[v9] LSM: Three basic syscalls

[PATCH v9 09/11] AppArmor: Add selfattr hooks

Posted by Casey Schaufler 2 years, 7 months ago

Add hooks for setselfattr and getselfattr. These hooks are not very
different from their setprocattr and getprocattr equivalents, and
much of the code is shared.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/include/procattr.h |  2 +-
 security/apparmor/lsm.c              | 96 ++++++++++++++++++++++++++--
 security/apparmor/procattr.c         | 11 +++-
 3 files changed, 99 insertions(+), 10 deletions(-)

diff --git a/security/apparmor/include/procattr.h b/security/apparmor/include/procattr.h
index 31689437e0e1..03dbfdb2f2c0 100644
--- a/security/apparmor/include/procattr.h
+++ b/security/apparmor/include/procattr.h
@@ -11,7 +11,7 @@
 #ifndef __AA_PROCATTR_H
 #define __AA_PROCATTR_H
 
-int aa_getprocattr(struct aa_label *label, char **string);
+int aa_getprocattr(struct aa_label *label, char **string, bool newline);
 int aa_setprocattr_changehat(char *args, size_t size, int flags);
 
 #endif /* __AA_PROCATTR_H */
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index ce6ccb7e06ec..bdaa8bac0404 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -630,6 +630,45 @@ static int apparmor_sb_pivotroot(const struct path *old_path,
 	return error;
 }
 
+static int apparmor_getselfattr(unsigned int __user attr,
+				struct lsm_ctx __user *lx, size_t *size,
+				u32 __user flags)
+{
+	int error = -ENOENT;
+	struct aa_task_ctx *ctx = task_ctx(current);
+	struct aa_label *label = NULL;
+	size_t total_len;
+	char *value;
+
+	if (attr == LSM_ATTR_CURRENT)
+		label = aa_get_newest_label(cred_label(current_cred()));
+	else if (attr == LSM_ATTR_PREV && ctx->previous)
+		label = aa_get_newest_label(ctx->previous);
+	else if (attr == LSM_ATTR_EXEC && ctx->onexec)
+		label = aa_get_newest_label(ctx->onexec);
+	else
+		error = -EOPNOTSUPP;
+
+	if (label) {
+		error = aa_getprocattr(label, &value, false);
+		if (error > 0) {
+			total_len = ALIGN(error + sizeof(*ctx), 8);
+			if (total_len > *size)
+				error = -E2BIG;
+			else
+				lsm_fill_user_ctx(lx, value, error,
+						  LSM_ID_APPARMOR, 0);
+		}
+	}
+
+	aa_put_label(label);
+
+	*size = total_len;
+	if (error > 0)
+		return 1;
+	return error;
+}
+
 static int apparmor_getprocattr(struct task_struct *task, const char *name,
 				char **value)
 {
@@ -649,7 +688,7 @@ static int apparmor_getprocattr(struct task_struct *task, const char *name,
 		error = -EINVAL;
 
 	if (label)
-		error = aa_getprocattr(label, value);
+		error = aa_getprocattr(label, value, true);
 
 	aa_put_label(label);
 	put_cred(cred);
@@ -657,8 +696,7 @@ static int apparmor_getprocattr(struct task_struct *task, const char *name,
 	return error;
 }
 
-static int apparmor_setprocattr(const char *name, void *value,
-				size_t size)
+static int do_setattr(u64 attr, void *value, size_t size)
 {
 	char *command, *largs = NULL, *args = value;
 	size_t arg_size;
@@ -689,7 +727,7 @@ static int apparmor_setprocattr(const char *name, void *value,
 		goto out;
 
 	arg_size = size - (args - (largs ? largs : (char *) value));
-	if (strcmp(name, "current") == 0) {
+	if (attr == LSM_ATTR_CURRENT) {
 		if (strcmp(command, "changehat") == 0) {
 			error = aa_setprocattr_changehat(args, arg_size,
 							 AA_CHANGE_NOFLAGS);
@@ -704,7 +742,7 @@ static int apparmor_setprocattr(const char *name, void *value,
 			error = aa_change_profile(args, AA_CHANGE_STACK);
 		} else
 			goto fail;
-	} else if (strcmp(name, "exec") == 0) {
+	} else if (attr == LSM_ATTR_EXEC) {
 		if (strcmp(command, "exec") == 0)
 			error = aa_change_profile(args, AA_CHANGE_ONEXEC);
 		else if (strcmp(command, "stack") == 0)
@@ -724,13 +762,57 @@ static int apparmor_setprocattr(const char *name, void *value,
 
 fail:
 	aad(&sa)->label = begin_current_label_crit_section();
-	aad(&sa)->info = name;
+	if (attr == LSM_ATTR_CURRENT)
+		aad(&sa)->info = "current";
+	else if (attr == LSM_ATTR_EXEC)
+		aad(&sa)->info = "exec";
+	else
+		aad(&sa)->info = "invalid";
 	aad(&sa)->error = error = -EINVAL;
 	aa_audit_msg(AUDIT_APPARMOR_DENIED, &sa, NULL);
 	end_current_label_crit_section(aad(&sa)->label);
 	goto out;
 }
 
+static int apparmor_setselfattr(unsigned int __user attr,
+				struct lsm_ctx __user *ctx, size_t __user size,
+				u32 __user flags)
+{
+	struct lsm_ctx *lctx;
+	void *context;
+	int rc;
+
+	if (attr != LSM_ATTR_CURRENT && attr != LSM_ATTR_EXEC)
+		return -EOPNOTSUPP;
+
+	context = kmalloc(size, GFP_KERNEL);
+	if (context == NULL)
+		return -ENOMEM;
+
+	lctx = (struct lsm_ctx *)context;
+	if (copy_from_user(context, ctx, size))
+		rc = -EFAULT;
+	else if (lctx->ctx_len > size)
+		rc = -EINVAL;
+	else
+		rc = do_setattr(attr, lctx + 1, lctx->ctx_len);
+
+	kfree(context);
+	if (rc > 0)
+		return 0;
+	return rc;
+}
+
+static int apparmor_setprocattr(const char *name, void *value,
+				size_t size)
+{
+	int attr = lsm_name_to_attr(name);
+
+	if (attr)
+		return do_setattr(attr, value, size);
+	return -EINVAL;
+}
+
 /**
  * apparmor_bprm_committing_creds - do task cleanup on committing new creds
  * @bprm: binprm for the exec  (NOT NULL)
@@ -1253,6 +1335,8 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(file_lock, apparmor_file_lock),
 	LSM_HOOK_INIT(file_truncate, apparmor_file_truncate),
 
+	LSM_HOOK_INIT(getselfattr, apparmor_getselfattr),
+	LSM_HOOK_INIT(setselfattr, apparmor_setselfattr),
 	LSM_HOOK_INIT(getprocattr, apparmor_getprocattr),
 	LSM_HOOK_INIT(setprocattr, apparmor_setprocattr),
 
diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c
index 197d41f9c32b..196f319aa3b2 100644
--- a/security/apparmor/procattr.c
+++ b/security/apparmor/procattr.c
@@ -20,6 +20,7 @@
  * aa_getprocattr - Return the label information for @label
  * @label: the label to print label info about  (NOT NULL)
  * @string: Returns - string containing the label info (NOT NULL)
+ * @newline: indicates that a newline should be added
  *
  * Requires: label != NULL && string != NULL
  *
@@ -27,7 +28,7 @@
  *
  * Returns: size of string placed in @string else error code on failure
  */
-int aa_getprocattr(struct aa_label *label, char **string)
+int aa_getprocattr(struct aa_label *label, char **string, bool newline)
 {
 	struct aa_ns *ns = labels_ns(label);
 	struct aa_ns *current_ns = aa_get_current_ns();
@@ -57,10 +58,14 @@ int aa_getprocattr(struct aa_label *label, char **string)
 		return len;
 	}
 
-	(*string)[len] = '\n';
-	(*string)[len + 1] = 0;
+	if (newline)
+		(*string)[len++] = '\n';
+	(*string)[len] = 0;
 
 	aa_put_ns(current_ns);
+
+	if (newline)
+		return len;
 	return len + 1;
 }
 
-- 
2.39.2

Re: [PATCH v9 09/11] AppArmor: Add selfattr hooks

Posted by kernel test robot 2 years, 7 months ago

Hi Casey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/perf/core]
[also build test WARNING on acme/perf/core shuah-kselftest/next shuah-kselftest/fixes linus/master v6.3-rc7]
[cannot apply to next-20230421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230422-024331
base:   tip/perf/core
patch link:    https://lore.kernel.org/r/20230421174259.2458-10-casey%40schaufler-ca.com
patch subject: [PATCH v9 09/11] AppArmor: Add selfattr hooks
config: mips-randconfig-s051-20230421 (https://download.01.org/0day-ci/archive/20230422/202304222257.AqAHzkgi-lkp@intel.com/config)
compiler: mips64el-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/2628bfcd3ff1b12fbae522a5449a7344ffe6ecbd
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230422-024331
        git checkout 2628bfcd3ff1b12fbae522a5449a7344ffe6ecbd
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=mips olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=mips SHELL=/bin/bash security/apparmor/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304222257.AqAHzkgi-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> security/apparmor/lsm.c:1339:9: sparse: sparse: incorrect type in initializer (incompatible argument 3 (different address spaces)) @@     expected int ( *setselfattr )( ... ) @@     got int ( * )( ... ) @@
   security/apparmor/lsm.c:1339:9: sparse:     expected int ( *setselfattr )( ... )
   security/apparmor/lsm.c:1339:9: sparse:     got int ( * )( ... )
>> security/apparmor/lsm.c:643:13: sparse: sparse: dereference of noderef expression
   security/apparmor/lsm.c:645:18: sparse: sparse: dereference of noderef expression
   security/apparmor/lsm.c:647:18: sparse: sparse: dereference of noderef expression
   security/apparmor/lsm.c:785:13: sparse: sparse: dereference of noderef expression
   security/apparmor/lsm.c:785:41: sparse: sparse: dereference of noderef expression
   security/apparmor/lsm.c:788:27: sparse: sparse: dereference of noderef expression
   security/apparmor/lsm.c:788:27: sparse: sparse: dereference of noderef expression
   security/apparmor/lsm.c:793:42: sparse: sparse: dereference of noderef expression
   security/apparmor/lsm.c:793:42: sparse: sparse: dereference of noderef expression
   security/apparmor/lsm.c:795:34: sparse: sparse: dereference of noderef expression
   security/apparmor/lsm.c:798:33: sparse: sparse: dereference of noderef expression

vim +1339 security/apparmor/lsm.c

  1305	
  1306	static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
  1307		LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check),
  1308		LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme),
  1309		LSM_HOOK_INIT(capget, apparmor_capget),
  1310		LSM_HOOK_INIT(capable, apparmor_capable),
  1311	
  1312		LSM_HOOK_INIT(sb_mount, apparmor_sb_mount),
  1313		LSM_HOOK_INIT(sb_umount, apparmor_sb_umount),
  1314		LSM_HOOK_INIT(sb_pivotroot, apparmor_sb_pivotroot),
  1315	
  1316		LSM_HOOK_INIT(path_link, apparmor_path_link),
  1317		LSM_HOOK_INIT(path_unlink, apparmor_path_unlink),
  1318		LSM_HOOK_INIT(path_symlink, apparmor_path_symlink),
  1319		LSM_HOOK_INIT(path_mkdir, apparmor_path_mkdir),
  1320		LSM_HOOK_INIT(path_rmdir, apparmor_path_rmdir),
  1321		LSM_HOOK_INIT(path_mknod, apparmor_path_mknod),
  1322		LSM_HOOK_INIT(path_rename, apparmor_path_rename),
  1323		LSM_HOOK_INIT(path_chmod, apparmor_path_chmod),
  1324		LSM_HOOK_INIT(path_chown, apparmor_path_chown),
  1325		LSM_HOOK_INIT(path_truncate, apparmor_path_truncate),
  1326		LSM_HOOK_INIT(inode_getattr, apparmor_inode_getattr),
  1327	
  1328		LSM_HOOK_INIT(file_open, apparmor_file_open),
  1329		LSM_HOOK_INIT(file_receive, apparmor_file_receive),
  1330		LSM_HOOK_INIT(file_permission, apparmor_file_permission),
  1331		LSM_HOOK_INIT(file_alloc_security, apparmor_file_alloc_security),
  1332		LSM_HOOK_INIT(file_free_security, apparmor_file_free_security),
  1333		LSM_HOOK_INIT(mmap_file, apparmor_mmap_file),
  1334		LSM_HOOK_INIT(file_mprotect, apparmor_file_mprotect),
  1335		LSM_HOOK_INIT(file_lock, apparmor_file_lock),
  1336		LSM_HOOK_INIT(file_truncate, apparmor_file_truncate),
  1337	
  1338		LSM_HOOK_INIT(getselfattr, apparmor_getselfattr),
> 1339		LSM_HOOK_INIT(setselfattr, apparmor_setselfattr),
  1340		LSM_HOOK_INIT(getprocattr, apparmor_getprocattr),
  1341		LSM_HOOK_INIT(setprocattr, apparmor_setprocattr),
  1342	
  1343		LSM_HOOK_INIT(sk_alloc_security, apparmor_sk_alloc_security),
  1344		LSM_HOOK_INIT(sk_free_security, apparmor_sk_free_security),
  1345		LSM_HOOK_INIT(sk_clone_security, apparmor_sk_clone_security),
  1346	
  1347		LSM_HOOK_INIT(socket_create, apparmor_socket_create),
  1348		LSM_HOOK_INIT(socket_post_create, apparmor_socket_post_create),
  1349		LSM_HOOK_INIT(socket_bind, apparmor_socket_bind),
  1350		LSM_HOOK_INIT(socket_connect, apparmor_socket_connect),
  1351		LSM_HOOK_INIT(socket_listen, apparmor_socket_listen),
  1352		LSM_HOOK_INIT(socket_accept, apparmor_socket_accept),
  1353		LSM_HOOK_INIT(socket_sendmsg, apparmor_socket_sendmsg),
  1354		LSM_HOOK_INIT(socket_recvmsg, apparmor_socket_recvmsg),
  1355		LSM_HOOK_INIT(socket_getsockname, apparmor_socket_getsockname),
  1356		LSM_HOOK_INIT(socket_getpeername, apparmor_socket_getpeername),
  1357		LSM_HOOK_INIT(socket_getsockopt, apparmor_socket_getsockopt),
  1358		LSM_HOOK_INIT(socket_setsockopt, apparmor_socket_setsockopt),
  1359		LSM_HOOK_INIT(socket_shutdown, apparmor_socket_shutdown),
  1360	#ifdef CONFIG_NETWORK_SECMARK
  1361		LSM_HOOK_INIT(socket_sock_rcv_skb, apparmor_socket_sock_rcv_skb),
  1362	#endif
  1363		LSM_HOOK_INIT(socket_getpeersec_stream,
  1364			      apparmor_socket_getpeersec_stream),
  1365		LSM_HOOK_INIT(socket_getpeersec_dgram,
  1366			      apparmor_socket_getpeersec_dgram),
  1367		LSM_HOOK_INIT(sock_graft, apparmor_sock_graft),
  1368	#ifdef CONFIG_NETWORK_SECMARK
  1369		LSM_HOOK_INIT(inet_conn_request, apparmor_inet_conn_request),
  1370	#endif
  1371	
  1372		LSM_HOOK_INIT(cred_alloc_blank, apparmor_cred_alloc_blank),
  1373		LSM_HOOK_INIT(cred_free, apparmor_cred_free),
  1374		LSM_HOOK_INIT(cred_prepare, apparmor_cred_prepare),
  1375		LSM_HOOK_INIT(cred_transfer, apparmor_cred_transfer),
  1376	
  1377		LSM_HOOK_INIT(bprm_creds_for_exec, apparmor_bprm_creds_for_exec),
  1378		LSM_HOOK_INIT(bprm_committing_creds, apparmor_bprm_committing_creds),
  1379		LSM_HOOK_INIT(bprm_committed_creds, apparmor_bprm_committed_creds),
  1380	
  1381		LSM_HOOK_INIT(task_free, apparmor_task_free),
  1382		LSM_HOOK_INIT(task_alloc, apparmor_task_alloc),
  1383		LSM_HOOK_INIT(current_getsecid_subj, apparmor_current_getsecid_subj),
  1384		LSM_HOOK_INIT(task_getsecid_obj, apparmor_task_getsecid_obj),
  1385		LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit),
  1386		LSM_HOOK_INIT(task_kill, apparmor_task_kill),
  1387	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v9 09/11] AppArmor: Add selfattr hooks

Posted by kernel test robot 2 years, 7 months ago

Hi Casey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/perf/core]
[also build test WARNING on acme/perf/core shuah-kselftest/next shuah-kselftest/fixes linus/master v6.3-rc7]
[cannot apply to next-20230421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230422-024331
base:   tip/perf/core
patch link:    https://lore.kernel.org/r/20230421174259.2458-10-casey%40schaufler-ca.com
patch subject: [PATCH v9 09/11] AppArmor: Add selfattr hooks
config: x86_64-randconfig-a005 (https://download.01.org/0day-ci/archive/20230422/202304220930.OFrY92as-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/2628bfcd3ff1b12fbae522a5449a7344ffe6ecbd
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Casey-Schaufler/LSM-Maintain-a-table-of-LSM-attribute-data/20230422-024331
        git checkout 2628bfcd3ff1b12fbae522a5449a7344ffe6ecbd
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash security/apparmor/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304220930.OFrY92as-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> security/apparmor/lsm.c:654:7: warning: variable 'total_len' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
                   if (error > 0) {
                       ^~~~~~~~~
   security/apparmor/lsm.c:666:10: note: uninitialized use occurs here
           *size = total_len;
                   ^~~~~~~~~
   security/apparmor/lsm.c:654:3: note: remove the 'if' if its condition is always true
                   if (error > 0) {
                   ^~~~~~~~~~~~~~~
   security/apparmor/lsm.c:652:6: warning: variable 'total_len' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
           if (label) {
               ^~~~~
   security/apparmor/lsm.c:666:10: note: uninitialized use occurs here
           *size = total_len;
                   ^~~~~~~~~
   security/apparmor/lsm.c:652:2: note: remove the 'if' if its condition is always true
           if (label) {
           ^~~~~~~~~~~
   security/apparmor/lsm.c:640:18: note: initialize the variable 'total_len' to silence this warning
           size_t total_len;
                           ^
                            = 0
   2 warnings generated.


vim +654 security/apparmor/lsm.c

   632	
   633	static int apparmor_getselfattr(unsigned int __user attr,
   634					struct lsm_ctx __user *lx, size_t *size,
   635					u32 __user flags)
   636	{
   637		int error = -ENOENT;
   638		struct aa_task_ctx *ctx = task_ctx(current);
   639		struct aa_label *label = NULL;
   640		size_t total_len;
   641		char *value;
   642	
   643		if (attr == LSM_ATTR_CURRENT)
   644			label = aa_get_newest_label(cred_label(current_cred()));
   645		else if (attr == LSM_ATTR_PREV && ctx->previous)
   646			label = aa_get_newest_label(ctx->previous);
   647		else if (attr == LSM_ATTR_EXEC && ctx->onexec)
   648			label = aa_get_newest_label(ctx->onexec);
   649		else
   650			error = -EOPNOTSUPP;
   651	
   652		if (label) {
   653			error = aa_getprocattr(label, &value, false);
 > 654			if (error > 0) {
   655				total_len = ALIGN(error + sizeof(*ctx), 8);
   656				if (total_len > *size)
   657					error = -E2BIG;
   658				else
   659					lsm_fill_user_ctx(lx, value, error,
   660							  LSM_ID_APPARMOR, 0);
   661			}
   662		}
   663	
   664		aa_put_label(label);
   665	
   666		*size = total_len;
   667		if (error > 0)
   668			return 1;
   669		return error;
   670	}
   671	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v9 09/11] AppArmor: Add selfattr hooks

Posted by Kees Cook 2 years, 7 months ago

On Fri, Apr 21, 2023 at 10:42:57AM -0700, Casey Schaufler wrote:
> Add hooks for setselfattr and getselfattr. These hooks are not very
> different from their setprocattr and getprocattr equivalents, and
> much of the code is shared.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> Cc: John Johansen <john.johansen@canonical.com>
> ---
>  security/apparmor/include/procattr.h |  2 +-
>  security/apparmor/lsm.c              | 96 ++++++++++++++++++++++++++--
>  security/apparmor/procattr.c         | 11 +++-
>  3 files changed, 99 insertions(+), 10 deletions(-)
> 
> diff --git a/security/apparmor/include/procattr.h b/security/apparmor/include/procattr.h
> index 31689437e0e1..03dbfdb2f2c0 100644
> --- a/security/apparmor/include/procattr.h
> +++ b/security/apparmor/include/procattr.h
> @@ -11,7 +11,7 @@
>  #ifndef __AA_PROCATTR_H
>  #define __AA_PROCATTR_H
>  
> -int aa_getprocattr(struct aa_label *label, char **string);
> +int aa_getprocattr(struct aa_label *label, char **string, bool newline);
>  int aa_setprocattr_changehat(char *args, size_t size, int flags);
>  
>  #endif /* __AA_PROCATTR_H */
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index ce6ccb7e06ec..bdaa8bac0404 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -630,6 +630,45 @@ static int apparmor_sb_pivotroot(const struct path *old_path,
>  	return error;
>  }
>  
> +static int apparmor_getselfattr(unsigned int __user attr,
> +				struct lsm_ctx __user *lx, size_t *size,
> +				u32 __user flags)
> +{
> +	int error = -ENOENT;
> +	struct aa_task_ctx *ctx = task_ctx(current);
> +	struct aa_label *label = NULL;
> +	size_t total_len;
> +	char *value;
> +
> +	if (attr == LSM_ATTR_CURRENT)
> +		label = aa_get_newest_label(cred_label(current_cred()));
> +	else if (attr == LSM_ATTR_PREV && ctx->previous)
> +		label = aa_get_newest_label(ctx->previous);
> +	else if (attr == LSM_ATTR_EXEC && ctx->onexec)
> +		label = aa_get_newest_label(ctx->onexec);
> +	else
> +		error = -EOPNOTSUPP;

Is "-EOPNOTSUPP" correct for LSM_ATTR_PREV when !ctx->previous? That
seems like it should be -EINVAL? I think this would be better served as
a switch statement.

> +
> +	if (label) {
> +		error = aa_getprocattr(label, &value, false);
> +		if (error > 0) {
> +			total_len = ALIGN(error + sizeof(*ctx), 8);
> +			if (total_len > *size)
> +				error = -E2BIG;
> +			else
> +				lsm_fill_user_ctx(lx, value, error,
> +						  LSM_ID_APPARMOR, 0);
> +		}
> +	}
> +
> +	aa_put_label(label);
> +
> +	*size = total_len;
> +	if (error > 0)
> +		return 1;
> +	return error;
> +}
> +
>  static int apparmor_getprocattr(struct task_struct *task, const char *name,
>  				char **value)
>  {
> @@ -649,7 +688,7 @@ static int apparmor_getprocattr(struct task_struct *task, const char *name,
>  		error = -EINVAL;
>  
>  	if (label)
> -		error = aa_getprocattr(label, value);
> +		error = aa_getprocattr(label, value, true);
>  
>  	aa_put_label(label);
>  	put_cred(cred);
> @@ -657,8 +696,7 @@ static int apparmor_getprocattr(struct task_struct *task, const char *name,
>  	return error;
>  }
>  
> -static int apparmor_setprocattr(const char *name, void *value,
> -				size_t size)
> +static int do_setattr(u64 attr, void *value, size_t size)
>  {
>  	char *command, *largs = NULL, *args = value;
>  	size_t arg_size;
> @@ -689,7 +727,7 @@ static int apparmor_setprocattr(const char *name, void *value,
>  		goto out;
>  
>  	arg_size = size - (args - (largs ? largs : (char *) value));
> -	if (strcmp(name, "current") == 0) {
> +	if (attr == LSM_ATTR_CURRENT) {
>  		if (strcmp(command, "changehat") == 0) {
>  			error = aa_setprocattr_changehat(args, arg_size,
>  							 AA_CHANGE_NOFLAGS);
> @@ -704,7 +742,7 @@ static int apparmor_setprocattr(const char *name, void *value,
>  			error = aa_change_profile(args, AA_CHANGE_STACK);
>  		} else
>  			goto fail;
> -	} else if (strcmp(name, "exec") == 0) {
> +	} else if (attr == LSM_ATTR_EXEC) {
>  		if (strcmp(command, "exec") == 0)
>  			error = aa_change_profile(args, AA_CHANGE_ONEXEC);
>  		else if (strcmp(command, "stack") == 0)
> @@ -724,13 +762,57 @@ static int apparmor_setprocattr(const char *name, void *value,
>  
>  fail:
>  	aad(&sa)->label = begin_current_label_crit_section();
> -	aad(&sa)->info = name;
> +	if (attr == LSM_ATTR_CURRENT)
> +		aad(&sa)->info = "current";
> +	else if (attr == LSM_ATTR_EXEC)
> +		aad(&sa)->info = "exec";
> +	else
> +		aad(&sa)->info = "invalid";
>  	aad(&sa)->error = error = -EINVAL;
>  	aa_audit_msg(AUDIT_APPARMOR_DENIED, &sa, NULL);
>  	end_current_label_crit_section(aad(&sa)->label);
>  	goto out;
>  }
>  
> +static int apparmor_setselfattr(unsigned int __user attr,
> +				struct lsm_ctx __user *ctx, size_t __user size,
> +				u32 __user flags)
> +{
> +	struct lsm_ctx *lctx;
> +	void *context;
> +	int rc;
> +
> +	if (attr != LSM_ATTR_CURRENT && attr != LSM_ATTR_EXEC)
> +		return -EOPNOTSUPP;
> +
> +	context = kmalloc(size, GFP_KERNEL);
> +	if (context == NULL)
> +		return -ENOMEM;
> +
> +	lctx = (struct lsm_ctx *)context;
> +	if (copy_from_user(context, ctx, size))
> +		rc = -EFAULT;
> +	else if (lctx->ctx_len > size)
> +		rc = -EINVAL;
> +	else
> +		rc = do_setattr(attr, lctx + 1, lctx->ctx_len);
> +
> +	kfree(context);
> +	if (rc > 0)
> +		return 0;
> +	return rc;
> +}
> +
> +static int apparmor_setprocattr(const char *name, void *value,
> +				size_t size)
> +{
> +	int attr = lsm_name_to_attr(name);
> +
> +	if (attr)
> +		return do_setattr(attr, value, size);
> +	return -EINVAL;
> +}
> +
>  /**
>   * apparmor_bprm_committing_creds - do task cleanup on committing new creds
>   * @bprm: binprm for the exec  (NOT NULL)
> @@ -1253,6 +1335,8 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(file_lock, apparmor_file_lock),
>  	LSM_HOOK_INIT(file_truncate, apparmor_file_truncate),
>  
> +	LSM_HOOK_INIT(getselfattr, apparmor_getselfattr),
> +	LSM_HOOK_INIT(setselfattr, apparmor_setselfattr),
>  	LSM_HOOK_INIT(getprocattr, apparmor_getprocattr),
>  	LSM_HOOK_INIT(setprocattr, apparmor_setprocattr),
>  
> diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c
> index 197d41f9c32b..196f319aa3b2 100644
> --- a/security/apparmor/procattr.c
> +++ b/security/apparmor/procattr.c
> @@ -20,6 +20,7 @@
>   * aa_getprocattr - Return the label information for @label
>   * @label: the label to print label info about  (NOT NULL)
>   * @string: Returns - string containing the label info (NOT NULL)
> + * @newline: indicates that a newline should be added
>   *
>   * Requires: label != NULL && string != NULL
>   *
> @@ -27,7 +28,7 @@
>   *
>   * Returns: size of string placed in @string else error code on failure
>   */
> -int aa_getprocattr(struct aa_label *label, char **string)
> +int aa_getprocattr(struct aa_label *label, char **string, bool newline)
>  {
>  	struct aa_ns *ns = labels_ns(label);
>  	struct aa_ns *current_ns = aa_get_current_ns();
> @@ -57,10 +58,14 @@ int aa_getprocattr(struct aa_label *label, char **string)
>  		return len;
>  	}
>  
> -	(*string)[len] = '\n';
> -	(*string)[len + 1] = 0;
> +	if (newline)
> +		(*string)[len++] = '\n';
> +	(*string)[len] = 0;
>  
>  	aa_put_ns(current_ns);
> +
> +	if (newline)
> +		return len;
>  	return len + 1;
>  }

This is returning the count including trailing %NUL, yes? Why is this
not always just "return len"? i.e.:

	if (newline)
		(*string)[len++] = '\n';
	(*string)[len++] = 0;

	aa_put_ns(current_ns);
	return len;


>  
> -- 
> 2.39.2
> 

-- 
Kees Cook