[apparmor] [PATCH 4/5] Add the ability to read proc attr interfaces

John Johansen john.johansen at canonical.com
Fri Jul 15 02:35:15 UTC 2011


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/14/2011 04:29 PM, Steve Beattie wrote:
> On Tue, Jul 12, 2011 at 01:50:22PM -0700, John Johansen wrote:
>> On 07/12/2011 12:01 PM, Seth Arnold wrote:
>>> The ret=1 just before the for loop isn't needed (except to quiet warnings?) 
>>> I think the entire for loop would be easier read as a while loop.
>>>
>> heh, sorry you were right, I missed that I was double assigning ret.
>>
>> revised patch using while and fixing a couple of bugs below
>>
>> ---
>>
>> Add the ability to read proc attr interfaces
>>
>> Signed-off-by: John Johansen <john.johansen at canonical.com>
>>
>> diff --git a/libraries/libapparmor/src/kernel_interface.c b/libraries/libapparmor/src/kernel_interface.c
>> index 0e61065..9f16bd0 100644
>> --- a/libraries/libapparmor/src/kernel_interface.c
>> +++ b/libraries/libapparmor/src/kernel_interface.c
>> @@ -54,6 +54,68 @@ static char *procattr_path(pid_t pid, const char *attr)
>>  	return NULL;
>>  }
>>  
>> +/**
>> + * getprocattr - get the contents of @attr for @tid into @buf
>> + * @tid: tid of task to query
>> + * @attr: which /proc/<tid>/current/attr to query
> 
> Nitpick: "which /proc/<tid>/attr/<attr> to query"
> 
>> + * @buf: buffer to store the result in
>> + * @len: size of the buffer
>> + *
>> + * Returns: size of data read or -1 on error, and sets errno
>> + */
>> +static int getprocattr(pid_t tid, const char *attr, char *buf, int len)
>> +{
>> +	int rc = -1;
>> +	int fd, ret;
>> +	char *ctl = NULL;
>> +	int size = 0;
>> +
>> +	if (!buf || len <= 0) {
>> +		errno = EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	ctl = procattr_path(tid, attr);
>> +	if (!ctl)
>> +		goto out;
>> +
>> +	fd = open(ctl, O_RDONLY);
>> +	if (fd == -1) {
>> +		goto out;
>> +	}
>> +
>> +	do {
>> +		ret = read(fd, buf, len);
>> +		if (ret <= 0)
>> +			break;
>> +		buf += ret;
>> +		size += ret;
>> +		len -= ret;
>> +		if (len < 0) {
>> +			errno = ENAMETOOLONG;
>> +			break;
> 
> Is the intent to require the caller to check both errno and the return
> value rc? Because with the current code, if the buffer is too small, rc
> is still set to the amount read. You can see this with the attached
> patch to the aa_logmisc.c unit tests (I had to make getprocattr not
> static for it to work).
> 
nope, you should only need to check errno if rc == -1

so I have fixed, both of your complaints.  Revised patch below

- ---

Library interface for tasks introspecting confinement.

Signed-off-by: John Johansen <john.johansen at canonical.com>

diff --git a/libraries/libapparmor/src/apparmor.h b/libraries/libapparmor/src/apparmor.h
index 4ae0a03..e8c9975 100644
- --- a/libraries/libapparmor/src/apparmor.h
+++ b/libraries/libapparmor/src/apparmor.h
@@ -33,6 +33,8 @@ extern int aa_change_onexec(const char *profile);
 
 extern int aa_change_hatv(const char *subprofiles[], unsigned long token);
 extern int (aa_change_hat_vargs)(unsigned long token, int count, ...);
+extern int aa_query_confinement(pid_t target, char **confinement);
+extern int aa_introspect_confinement(char **confinement);
 
 #define __macroarg_counter(Y...) __macroarg_count1 ( , ##Y)
 #define __macroarg_count1(Y...) __macroarg_count2 (Y, 16,15,14,13,12,11,10,9,8,7,6,5,4,3,2,1,0)
diff --git a/libraries/libapparmor/src/kernel_interface.c b/libraries/libapparmor/src/kernel_interface.c
index 9f16bd0..63825cb 100644
- --- a/libraries/libapparmor/src/kernel_interface.c
+++ b/libraries/libapparmor/src/kernel_interface.c
@@ -57,7 +57,7 @@ static char *procattr_path(pid_t pid, const char *attr)
 /**
  * getprocattr - get the contents of @attr for @tid into @buf
  * @tid: tid of task to query
- - * @attr: which /proc/<tid>/current/attr to query
+ * @attr: which /proc/<tid>/attr/current to query
  * @buf: buffer to store the result in
  * @len: size of the buffer
  *
@@ -93,6 +93,7 @@ static int getprocattr(pid_t tid, const char *attr, char *buf, int len)
 		len -= ret;
 		if (len < 0) {
 			errno = ENAMETOOLONG;
+			ret = -1;
 			break;
 		}
 	} while (ret > 0);
@@ -333,3 +334,33 @@ int (aa_change_hat_vargs)(unsigned long token, int nhats, ...)
 	va_end(ap);
 	return aa_change_hatv(argv, token);
 }
+
+/**
+ * aa_query_confinement - query what the confinement for task @target is
+ * @target: task to query
+ * @confinement: pointer to the buffer with the profile name if successful
+ *
+ * Returns: length of confinement data or -1 on error and sets errno
+ */
+int aa_query_confinement(pid_t target, char **confinement)
+{
+	int size;
+	char *buffer = malloc(PATH_MAX);
+	if (!buffer)
+		return -1;
+	size = getprocattr(target, "current", buffer, PATH_MAX);
+	if (size != -1)
+		*confinement = buffer;
+	return size;
+}
+
+/**
+ * aa_introspect_confinement - query what the confinement for current task is
+ * @confinement: pointer to the buffer with the profile name if successful
+ *
+ * Returns: length of confinement data or -1 on error and sets errno
+ */
+int aa_introspect_confinement(char **confinement)
+{
+	return aa_query_confinement(aa_gettid(), confinement);
+}
diff --git a/libraries/libapparmor/src/libapparmor.map b/libraries/libapparmor/src/libapparmor.map
index c56cb86..ebd13c9 100644
- --- a/libraries/libapparmor/src/libapparmor.map
+++ b/libraries/libapparmor/src/libapparmor.map
@@ -21,6 +21,8 @@ APPARMOR_1.1 {
         aa_change_hat_vargs;
         aa_change_profile;
         aa_change_onexec;
+        aa_query_confinement;
+        aa_introspect_confinement;
         parse_record;
         free_record;
   local:
diff --git a/libraries/libapparmor/swig/SWIG/libapparmor.i b/libraries/libapparmor/swig/SWIG/libapparmor.i
index 1f2ede3..830f1c0 100644
- --- a/libraries/libapparmor/swig/SWIG/libapparmor.i
+++ b/libraries/libapparmor/swig/SWIG/libapparmor.i
@@ -18,4 +18,5 @@ extern int aa_change_profile(const char *profile);
 extern int aa_change_onexec(const char *profile);
 extern int aa_change_hatv(const char *subprofiles[], unsigned long token);
 extern int aa_change_hat_vargs(unsigned long token, int count, ...);
- -
+extern int aa_query_confinement(pid_t target, char **confinement);
+extern int aa_introspect_confinement(char **confinement);
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk4fp1wACgkQxAVxIsEKI+YgqQCfZsplrn2BxlNL2V+UK06P+zBd
ZYYAn2pRJP5dP4hBjAWNeOliFQkaZFjT
=aw5z
-----END PGP SIGNATURE-----



More information about the AppArmor mailing list