[apparmor] libapparmor: Clarify that mode strings are not to be freed
Tyler Hicks
tyhicks at canonical.com
Wed Sep 4 22:11:08 UTC 2013
The aa_getcon man page only implies that the *mode strings returned by
aa_getprocattr(), aa_gettaskcon(), aa_getcon(), and aa_getpeercon()
should not be freed. A developer using the man page to build against
libapparmor may miss that subtlety and end up hitting double free issues.
This patch makes the man page more clear, makes the function comments
more clear, and changes the aa_getprocattr() *buf param to *con. The use
of *buf should reserved for the aa_get*_raw() functions that do not
allocate a buffer for the confinement context and all documents now
clearly mention that *con must be freed.
Additionally, this patch removes the line wrapping of the
aa_getprocattr_raw() prototype in the aa_getcon man page source. The
line wrapping caused incorrect formatting of the function prototype when
viewing the man page.
Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
diff:
=== modified file 'libraries/libapparmor/doc/aa_getcon.pod'
--- libraries/libapparmor/doc/aa_getcon.pod 2013-06-25 22:55:52 +0000
+++ libraries/libapparmor/doc/aa_getcon.pod 2013-09-04 22:07:40 +0000
@@ -32,8 +32,7 @@
B<#include E<lt>sys/apparmor.hE<gt>>
-B<int aa_getprocattr_raw(pid_t tid, const char *attr, char *buf, int len,
- char **mode);>
+B<int aa_getprocattr_raw(pid_t tid, const char *attr, char *buf, int len, char **mode);>
B<int aa_getprocattr(pid_t tid, const char *attr, char **buf, char **mode);>
@@ -52,7 +51,10 @@
The aa_getcon function gets the current AppArmor confinement context for the
current task. The confinement context is usually just the name of the AppArmor
profile restricting the task, but it may include the profile namespace or in
-some cases a set of profile names (known as a stack of profiles). The returned string *con should be freed using free().
+some cases a set of profile names (known as a stack of profiles). The returned
+string *con should be freed using free(), but the returned string *mode should
+not be freed. The *con and *mode strings come from a single buffer allocation
+and are separated by a NUL character.
The aa_gettaskcon function is like the aa_getcon function except it will work
for any arbitrary task in the system.
=== modified file 'libraries/libapparmor/src/apparmor.h'
--- libraries/libapparmor/src/apparmor.h 2013-07-31 16:22:40 +0000
+++ libraries/libapparmor/src/apparmor.h 2013-09-04 22:07:40 +0000
@@ -76,7 +76,7 @@
*/
extern int aa_getprocattr_raw(pid_t tid, const char *attr, char *buf, int len,
char **mode);
-extern int aa_getprocattr(pid_t tid, const char *attr, char **buf, char **mode);
+extern int aa_getprocattr(pid_t tid, const char *attr, char **con, char **mode);
extern int aa_gettaskcon(pid_t target, char **con, char **mode);
extern int aa_getcon(char **con, char **mode);
extern int aa_getpeercon_raw(int fd, char *buf, int *len, char **mode);
=== modified file 'libraries/libapparmor/src/kernel_interface.c'
--- libraries/libapparmor/src/kernel_interface.c 2013-09-04 21:50:55 +0000
+++ libraries/libapparmor/src/kernel_interface.c 2013-09-04 22:07:40 +0000
@@ -184,7 +184,7 @@
* @attr: which /proc/<tid>/attr/<attr> to query
* @buf: buffer to store the result in
* @len: size of the buffer
- * @mode: if set will point to mode string within buffer if it is present
+ * @mode: if non-NULL and a mode is present, will point to mode string in @buf
*
* Returns: size of data read or -1 on error, and sets errno
*/
@@ -265,17 +265,24 @@
* aa_getprocattr - get the contents of @attr for @tid into @buf
* @tid: tid of task to query
* @attr: which /proc/<tid>/attr/<attr> to query
- * @buf: allocated buffer the result is stored in
- * @mode: if set will point to mode string within buffer if it is present
+ * @con: allocated buffer the result is stored in
+ * @mode: if non-NULL and a mode is present, will point to mode string in @con
*
* Returns: size of data read or -1 on error, and sets errno
+ *
+ * Guarantees that @con and @mode are null terminated. The length returned
+ * is for all data including both @con and @mode, and maybe > than strlen(@con)
+ * even if @mode is NULL
+ *
+ * Caller is responsible for freeing the buffer returned in @con. @mode is
+ * always contained within @con's buffer and so NEVER do free(@mode)
*/
-int aa_getprocattr(pid_t tid, const char *attr, char **buf, char **mode)
+int aa_getprocattr(pid_t tid, const char *attr, char **con, char **mode)
{
int rc, size = INITIAL_GUESS_SIZE/2;
char *buffer = NULL;
- if (!buf) {
+ if (!con) {
errno = EINVAL;
return -1;
}
@@ -292,11 +299,11 @@
if (rc == -1) {
free(buffer);
- *buf = NULL;
+ *con = NULL;
if (mode)
*mode = NULL;
} else
- *buf = buffer;
+ *con = buffer;
return rc;
}
@@ -523,11 +530,11 @@
* aa_gettaskcon - get the confinement for task @target in an allocated buffer
* @target: task to query
* @con: pointer to returned buffer with the confinement string
- * @mode: if provided will point to the mode string in @con if present
+ * @mode: if non-NULL and a mode is present, will point to mode string in @con
*
* Returns: length of confinement data or -1 on error and sets errno
*
- * Guarentees that @con and @mode are null terminated. The length returned
+ * Guarantees that @con and @mode are null terminated. The length returned
* is for all data including both @con and @mode, and maybe > than strlen(@con)
* even if @mode is NULL
*
@@ -542,11 +549,11 @@
/**
* aa_getcon - get the confinement for current task in an allocated buffer
* @con: pointer to return buffer with the confinement if successful
- * @mode: if provided will point to the mode string in @con if present
+ * @mode: if non-NULL and a mode is present, will point to mode string in @con
*
* Returns: length of confinement data or -1 on error and sets errno
*
- * Guarentees that @con and @mode are null terminated. The length returned
+ * Guarantees that @con and @mode are null terminated. The length returned
* is for all data including both @con and @mode, and may > than strlen(@con)
* even if @mode is NULL
*
@@ -568,7 +575,7 @@
* @fd: socket to get peer confinement for
* @buf: buffer to store the result in
* @len: initially contains size of the buffer, returns size of data read
- * @mode: if set will point to mode string within buffer if it is present
+ * @mode: if non-NULL and a mode is present, will point to mode string in @buf
*
* Returns: length of confinement data including null termination or -1 on error
* if errno == ERANGE then @len will hold the size needed
@@ -616,11 +623,16 @@
* aa_getpeercon - get the confinement of the socket's peer (other end)
* @fd: socket to get peer confinement for
* @con: pointer to allocated buffer with the confinement string
- * @mode: if provided will point to the mode string in @con if present
+ * @mode: if non-NULL and a mode is present, will point to mode string in @con
*
* Returns: length of confinement data including null termination or -1 on error
*
- * Caller is responsible for freeing the buffer returned.
+ * Guarantees that @con and @mode are null terminated. The length returned
+ * is for all data including both @con and @mode, and maybe > than strlen(@con)
+ * even if @mode is NULL
+ *
+ * Caller is responsible for freeing the buffer returned in @con. @mode is
+ * always contained within @con's buffer and so NEVER do free(@mode)
*/
int aa_getpeercon(int fd, char **con, char **mode)
{
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20130904/b9e9640c/attachment.pgp>
More information about the AppArmor
mailing list