[apparmor] libapparmor: Clarify that mode strings are not to be freed
Tyler Hicks
tyhicks at canonical.com
Wed Sep 4 22:46:45 UTC 2013
On 2013-09-04 15:11:08, Tyler Hicks wrote:
> 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);>
Oops - I forgot one thing in this patch. This prototype in the man page
should be changed from
B<int aa_getprocattr(pid_t tid, const char *attr, char **buf, char **mode);>
to
B<int aa_getprocattr(pid_t tid, const char *attr, char **con, char **mode);>
Rather than send a v2, I think I can safely assume that this doesn't
void sarnold's ack and I'll make the change locally before committing it
to trunk. :)
Tyler
>
> @@ -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)
> {
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
-------------- 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/56977161/attachment-0001.pgp>
More information about the AppArmor
mailing list