[PATCH] libsepol: create new keys with copy of 'name' variable

[PATCH] libsepol: create new keys with copy of 'name' variable

[Petr Lautrbach]

sepol_*_key_create functions, which create keys based only on name, copy only
a pointer to a new key. This pointer doesn't need to be valid during the new key
lifetime therefore it's necessary to create a copy of it.

E.g. Python 3 SWIG bindings temporarily allocate memory needed for
PyUnicodeObject->char * conversion. This memory is deallocated shortly
after underlying C function returns.

Patch-by: Michal Srb <msrb@redhat.com>
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 libsepol/src/boolean_record.c | 7 +++++--
 libsepol/src/iface_record.c   | 7 +++++--
 libsepol/src/user_record.c    | 7 +++++--
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/libsepol/src/boolean_record.c b/libsepol/src/boolean_record.c
index 8b64413..0d139ac 100644
--- a/libsepol/src/boolean_record.c
+++ b/libsepol/src/boolean_record.c
@@ -25,12 +25,14 @@ int sepol_bool_key_create(sepol_handle_t * handle,
 	sepol_bool_key_t *tmp_key =
 	    (sepol_bool_key_t *) malloc(sizeof(struct sepol_bool_key));
 
-	if (!tmp_key) {
+	char *tmp_name = strdup(name);
+
+	if (!tmp_key || !tmp_name) {
 		ERR(handle, "out of memory, " "could not create boolean key");
 		return STATUS_ERR;
 	}
 
-	tmp_key->name = name;
+	tmp_key->name = tmp_name;
 
 	*key_ptr = tmp_key;
 	return STATUS_SUCCESS;
@@ -62,6 +64,7 @@ int sepol_bool_key_extract(sepol_handle_t * handle,
 
 void sepol_bool_key_free(sepol_bool_key_t * key)
 {
+	free(key->name);
 	free(key);
 }
 
diff --git a/libsepol/src/iface_record.c b/libsepol/src/iface_record.c
index 09adeb7..4315238 100644
--- a/libsepol/src/iface_record.c
+++ b/libsepol/src/iface_record.c
@@ -31,12 +31,14 @@ int sepol_iface_key_create(sepol_handle_t * handle,
 	sepol_iface_key_t *tmp_key =
 	    (sepol_iface_key_t *) malloc(sizeof(sepol_iface_key_t));
 
-	if (!tmp_key) {
+	char *tmp_name = strdup(name);
+
+	if (!tmp_key || !tmp_name) {
 		ERR(handle, "out of memory, could not create interface key");
 		return STATUS_ERR;
 	}
 
-	tmp_key->name = name;
+	tmp_key->name = tmp_name;
 
 	*key_ptr = tmp_key;
 	return STATUS_SUCCESS;
@@ -68,6 +70,7 @@ int sepol_iface_key_extract(sepol_handle_t * handle,
 
 void sepol_iface_key_free(sepol_iface_key_t * key)
 {
+	free(key->name);
 	free(key);
 }
 
diff --git a/libsepol/src/user_record.c b/libsepol/src/user_record.c
index c59c54b..dfc66e5 100644
--- a/libsepol/src/user_record.c
+++ b/libsepol/src/user_record.c
@@ -34,13 +34,15 @@ int sepol_user_key_create(sepol_handle_t * handle,
 	sepol_user_key_t *tmp_key =
 	    (sepol_user_key_t *) malloc(sizeof(sepol_user_key_t));
 
-	if (!tmp_key) {
+	char *tmp_name = strdup(name);
+
+	if (!tmp_key || !tmp_name) {
 		ERR(handle, "out of memory, "
 		    "could not create selinux user key");
 		return STATUS_ERR;
 	}
 
-	tmp_key->name = name;
+	tmp_key->name = tmp_name;
 
 	*key_ptr = tmp_key;
 	return STATUS_SUCCESS;
@@ -71,6 +73,7 @@ int sepol_user_key_extract(sepol_handle_t * handle,
 
 void sepol_user_key_free(sepol_user_key_t * key)
 {
+	free(key->name);
 	free(key);
 }
 
-- 
2.5.0

Re: [PATCH] libsepol: create new keys with copy of 'name' variable

[James Carter]

On 08/25/2015 10:05 AM, Petr Lautrbach wrote:
> sepol_*_key_create functions, which create keys based only on name, copy only
> a pointer to a new key. This pointer doesn't need to be valid during the new key
> lifetime therefore it's necessary to create a copy of it.
>
> E.g. Python 3 SWIG bindings temporarily allocate memory needed for
> PyUnicodeObject->char * conversion. This memory is deallocated shortly
> after underlying C function returns.
>
> Patch-by: Michal Srb <msrb@redhat.com>
> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
> ---
>   libsepol/src/boolean_record.c | 7 +++++--
>   libsepol/src/iface_record.c   | 7 +++++--
>   libsepol/src/user_record.c    | 7 +++++--
>   3 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/libsepol/src/boolean_record.c b/libsepol/src/boolean_record.c
> index 8b64413..0d139ac 100644
> --- a/libsepol/src/boolean_record.c
> +++ b/libsepol/src/boolean_record.c
> @@ -25,12 +25,14 @@ int sepol_bool_key_create(sepol_handle_t * handle,
>   	sepol_bool_key_t *tmp_key =
>   	    (sepol_bool_key_t *) malloc(sizeof(struct sepol_bool_key));
>
> -	if (!tmp_key) {
> +	char *tmp_name = strdup(name);
> +
> +	if (!tmp_key || !tmp_name) {
>   		ERR(handle, "out of memory, " "could not create boolean key");
>   		return STATUS_ERR;
>   	}
>

Need to free tmp_key if it is allocated, but the strdup for tmp_name fails. And 
vice versa.


> -	tmp_key->name = name;
> +	tmp_key->name = tmp_name;
>
>   	*key_ptr = tmp_key;
>   	return STATUS_SUCCESS;
> @@ -62,6 +64,7 @@ int sepol_bool_key_extract(sepol_handle_t * handle,
>
>   void sepol_bool_key_free(sepol_bool_key_t * key)
>   {
> +	free(key->name);
>   	free(key);
>   }
>
> diff --git a/libsepol/src/iface_record.c b/libsepol/src/iface_record.c
> index 09adeb7..4315238 100644
> --- a/libsepol/src/iface_record.c
> +++ b/libsepol/src/iface_record.c
> @@ -31,12 +31,14 @@ int sepol_iface_key_create(sepol_handle_t * handle,
>   	sepol_iface_key_t *tmp_key =
>   	    (sepol_iface_key_t *) malloc(sizeof(sepol_iface_key_t));
>
> -	if (!tmp_key) {
> +	char *tmp_name = strdup(name);
> +
> +	if (!tmp_key || !tmp_name) {
>   		ERR(handle, "out of memory, could not create interface key");
>   		return STATUS_ERR;
>   	}
>

Same here.


> -	tmp_key->name = name;
> +	tmp_key->name = tmp_name;
>
>   	*key_ptr = tmp_key;
>   	return STATUS_SUCCESS;
> @@ -68,6 +70,7 @@ int sepol_iface_key_extract(sepol_handle_t * handle,
>
>   void sepol_iface_key_free(sepol_iface_key_t * key)
>   {
> +	free(key->name);
>   	free(key);
>   }
>
> diff --git a/libsepol/src/user_record.c b/libsepol/src/user_record.c
> index c59c54b..dfc66e5 100644
> --- a/libsepol/src/user_record.c
> +++ b/libsepol/src/user_record.c
> @@ -34,13 +34,15 @@ int sepol_user_key_create(sepol_handle_t * handle,
>   	sepol_user_key_t *tmp_key =
>   	    (sepol_user_key_t *) malloc(sizeof(sepol_user_key_t));
>
> -	if (!tmp_key) {
> +	char *tmp_name = strdup(name);
> +
> +	if (!tmp_key || !tmp_name) {
>   		ERR(handle, "out of memory, "
>   		    "could not create selinux user key");
>   		return STATUS_ERR;
>   	}
>

And here.

> -	tmp_key->name = name;
> +	tmp_key->name = tmp_name;
>
>   	*key_ptr = tmp_key;
>   	return STATUS_SUCCESS;
> @@ -71,6 +73,7 @@ int sepol_user_key_extract(sepol_handle_t * handle,
>
>   void sepol_user_key_free(sepol_user_key_t * key)
>   {
> +	free(key->name);
>   	free(key);
>   }
>
>


-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency

Re: [PATCH] libsepol: create new keys with copy of 'name' variable

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 1415 bytes --]

On 08/25/2015 08:27 PM, James Carter wrote:
> On 08/25/2015 10:05 AM, Petr Lautrbach wrote:
>> --- a/libsepol/src/boolean_record.c
>> +++ b/libsepol/src/boolean_record.c
>> @@ -25,12 +25,14 @@ int sepol_bool_key_create(sepol_handle_t * handle,
>>       sepol_bool_key_t *tmp_key =
>>           (sepol_bool_key_t *) malloc(sizeof(struct sepol_bool_key));
>>
>> -    if (!tmp_key) {
>> +    char *tmp_name = strdup(name);
>> +
>> +    if (!tmp_key || !tmp_name) {
>>           ERR(handle, "out of memory, " "could not create boolean key");
>>           return STATUS_ERR;
>>       }
>>
> 
> Need to free tmp_key if it is allocated, but the strdup for tmp_name
> fails. And vice versa.
> 

The patch is apparently not ready yet. It can't be even compiled, see
bellow. Sorry for the noise.

cc -Werror -Wall -W -Wundef -Wshadow -Wmissing-format-attribute -O2 -I.
-I../include -D_GNU_SOURCE -I../cil/include -fPIC -c -o boolean_record.o
boolean_record.c
boolean_record.c: In function ‘sepol_bool_key_free’:
boolean_record.c:67:7: error: passing argument 1 of ‘free’ discards
‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
  free(key->name);
       ^
In file included from boolean_record.c:2:0:
/usr/include/stdlib.h:483:13: note: expected ‘void *’ but argument is of
type ‘const char *’
 extern void free (void *__ptr) __THROW;



-- 
Petr Lautrbach


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]