[systemd-devel] [systemd-commits] src/shared

[Lennart Poettering]

On Wed, 15.10.14 01:58, Michal Sekletar (msekleta at kemper.freedesktop.org) wrote:

>  src/shared/label.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> New commits:
> commit 5e78424f4a27c07be50e246308035c877f204038
> Author: Michal Sekletar <msekleta at redhat.com>
> Date:   Mon Oct 13 15:25:09 2014 +0200
> 
>     selinux: fix potential double free crash in child process
>     
>     Before returning from function we should reset ret to NULL, thus cleanup
>     function is nop.
>     
>     Also context_str() returns pointer to a string containing context but not a
>     copy, hence we must make copy it explicitly.
> 
> diff --git a/src/shared/label.c b/src/shared/label.c
> index b6af38d..69d4616 100644
> --- a/src/shared/label.c
> +++ b/src/shared/label.c
> @@ -334,7 +334,7 @@ int label_get_child_mls_label(int socket_fd, const char *exe, char **label) {
>          }
>  
>          freecon(mycon);
> -        mycon = context_str(bcon);
> +        mycon = strdup(context_str(bcon));

This looks wrong!

I meanm what is mycon? a string or a security_context_t? If it's the
latter (which it appears to be according to its type), then it should
not be assigned a string as returned by strdup(). (i figure internally
they might be the same, but if they do the dance with two distinct
types we should follow this non-sense here too...)

Shouldn't this be context_new() rather than strdup()?

libselinux definitely has one of the most horrible APIs around! Yuck,
just yuck!

Lennart

-- 
Lennart Poettering, Red Hat