* [PATCH 1/2] libsepol: calloc all the *_to_val_structs
@ 2016-08-18 20:54 william.c.roberts
2016-08-18 20:54 ` [PATCH 2/2] libsepol: port str_read from kernel william.c.roberts
2016-08-19 13:06 ` [PATCH 1/2] libsepol: calloc all the *_to_val_structs Stephen Smalley
0 siblings, 2 replies; 8+ messages in thread
From: william.c.roberts @ 2016-08-18 20:54 UTC (permalink / raw)
To: selinux, jwcart2, seandroid-list, sds
From: William Roberts <william.c.roberts@intel.com>
The usage patterns between these structures seem similair
to role_val_to_struct usages. Calloc these up to prevent
any unitialized usages.
Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
libsepol/src/mls.c | 2 +-
libsepol/src/policydb.c | 6 +++---
libsepol/src/users.c | 9 ++++++++-
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/libsepol/src/mls.c b/libsepol/src/mls.c
index 2dc5f2b..8047d91 100644
--- a/libsepol/src/mls.c
+++ b/libsepol/src/mls.c
@@ -312,7 +312,7 @@ int mls_context_isvalid(const policydb_t * p, const context_struct_t * c)
if (!c->user || c->user > p->p_users.nprim)
return 0;
usrdatum = p->user_val_to_struct[c->user - 1];
- if (!mls_range_contains(usrdatum->exp_range, c->range))
+ if (!usrdatum || !mls_range_contains(usrdatum->exp_range, c->range))
return 0; /* user may not be associated with range */
return 1;
diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index c225ac6..5f888d3 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -1074,7 +1074,7 @@ int policydb_index_others(sepol_handle_t * handle,
free(p->user_val_to_struct);
p->user_val_to_struct = (user_datum_t **)
- malloc(p->p_users.nprim * sizeof(user_datum_t *));
+ calloc(p->p_users.nprim, sizeof(user_datum_t *));
if (!p->user_val_to_struct)
return -1;
@@ -4006,12 +4006,12 @@ int policydb_reindex_users(policydb_t * p)
free(p->sym_val_to_name[i]);
p->user_val_to_struct = (user_datum_t **)
- malloc(p->p_users.nprim * sizeof(user_datum_t *));
+ calloc(p->p_users.nprim, sizeof(user_datum_t *));
if (!p->user_val_to_struct)
return -1;
p->sym_val_to_name[i] = (char **)
- malloc(p->symtab[i].nprim * sizeof(char *));
+ calloc(p->symtab[i].nprim, sizeof(char *));
if (!p->sym_val_to_name[i])
return -1;
diff --git a/libsepol/src/users.c b/libsepol/src/users.c
index ce54c2b..3ffb166 100644
--- a/libsepol/src/users.c
+++ b/libsepol/src/users.c
@@ -19,12 +19,17 @@ static int user_to_record(sepol_handle_t * handle,
const char *name = policydb->p_user_val_to_name[user_idx];
user_datum_t *usrdatum = policydb->user_val_to_struct[user_idx];
- ebitmap_t *roles = &(usrdatum->roles.roles);
+ ebitmap_t *roles;
ebitmap_node_t *rnode;
unsigned bit;
sepol_user_t *tmp_record = NULL;
+ if (!usrdatum)
+ goto err;
+
+ roles = &(usrdatum->roles.roles);
+
if (sepol_user_create(handle, &tmp_record) < 0)
goto err;
@@ -234,6 +239,7 @@ int sepol_user_modify(sepol_handle_t * handle,
if (!tmp_ptr)
goto omem;
policydb->user_val_to_struct = tmp_ptr;
+ policydb->user_val_to_struct[policydb->p_users.nprim] = NULL;
tmp_ptr = realloc(policydb->sym_val_to_name[SYM_USERS],
(policydb->p_users.nprim +
@@ -241,6 +247,7 @@ int sepol_user_modify(sepol_handle_t * handle,
if (!tmp_ptr)
goto omem;
policydb->sym_val_to_name[SYM_USERS] = tmp_ptr;
+ policydb->p_user_val_to_name[policydb->p_users.nprim] = NULL;
/* Need to copy the user name */
name = strdup(cname);
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] libsepol: port str_read from kernel
2016-08-18 20:54 [PATCH 1/2] libsepol: calloc all the *_to_val_structs william.c.roberts
@ 2016-08-18 20:54 ` william.c.roberts
2016-08-19 13:13 ` Stephen Smalley
2016-08-19 13:06 ` [PATCH 1/2] libsepol: calloc all the *_to_val_structs Stephen Smalley
1 sibling, 1 reply; 8+ messages in thread
From: william.c.roberts @ 2016-08-18 20:54 UTC (permalink / raw)
To: selinux, jwcart2, seandroid-list, sds
From: William Roberts <william.c.roberts@intel.com>
Rather than duplicating the following sequence:
1. Read len from file
2. alloc up space based on 1
3. read the contents into the buffer from 2
4. null terminate the buffer from 2
Use the str_read() function that is in the kernel, which
collapses steps 2 and 4. This not only reduces redundant
code, but also has the side-affect of providing a central
check on zero_or_saturated lengths from step 1 when
generating string values.
Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
libsepol/src/conditional.c | 9 +------
libsepol/src/module.c | 66 ++++++++++++++++++++++------------------------
libsepol/src/policydb.c | 10 +------
libsepol/src/private.h | 1 +
libsepol/src/services.c | 33 +++++++++++++++++++++++
5 files changed, 68 insertions(+), 51 deletions(-)
diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c
index 8680eb2..e1bc961 100644
--- a/libsepol/src/conditional.c
+++ b/libsepol/src/conditional.c
@@ -589,15 +589,8 @@ int cond_read_bool(policydb_t * p,
goto err;
len = le32_to_cpu(buf[2]);
- if (zero_or_saturated(len))
+ if (str_read(&key, fp, len))
goto err;
- key = malloc(len + 1);
- if (!key)
- goto err;
- rc = next_entry(key, fp, len);
- if (rc < 0)
- goto err;
- key[len] = 0;
if (p->policy_type != POLICY_KERN &&
p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
diff --git a/libsepol/src/module.c b/libsepol/src/module.c
index f25df95..a9d7c54 100644
--- a/libsepol/src/module.c
+++ b/libsepol/src/module.c
@@ -793,26 +793,26 @@ int sepol_module_package_info(struct sepol_policy_file *spf, int *type,
i);
goto cleanup;
}
+
len = le32_to_cpu(buf[0]);
- if (zero_or_saturated(len)) {
- ERR(file->handle,
- "invalid module name length: 0x%"PRIx32,
- len);
- goto cleanup;
- }
- *name = malloc(len + 1);
- if (!*name) {
- ERR(file->handle, "out of memory");
- goto cleanup;
- }
- rc = next_entry(*name, file, len);
- if (rc < 0) {
- ERR(file->handle,
- "cannot get module name string (at section %u)",
- i);
+ if (str_read(name, file, len)) {
+ switch(rc) {
+ case EINVAL:
+ ERR(file->handle,
+ "invalid module name length: 0x%"PRIx32,
+ len);
+ break;
+ case ENOMEM:
+ ERR(file->handle, "out of memory");
+ break;
+ default:
+ ERR(file->handle,
+ "cannot get module name string (at section %u)",
+ i);
+ }
goto cleanup;
}
- (*name)[len] = '\0';
+
rc = next_entry(buf, file, sizeof(uint32_t));
if (rc < 0) {
ERR(file->handle,
@@ -821,25 +821,23 @@ int sepol_module_package_info(struct sepol_policy_file *spf, int *type,
goto cleanup;
}
len = le32_to_cpu(buf[0]);
- if (zero_or_saturated(len)) {
- ERR(file->handle,
- "invalid module version length: 0x%"PRIx32,
- len);
- goto cleanup;
- }
- *version = malloc(len + 1);
- if (!*version) {
- ERR(file->handle, "out of memory");
- goto cleanup;
- }
- rc = next_entry(*version, file, len);
- if (rc < 0) {
- ERR(file->handle,
- "cannot get module version string (at section %u)",
- i);
+ if (str_read(version, file, len)) {
+ switch(rc) {
+ case EINVAL:
+ ERR(file->handle,
+ "invalid module name length: 0x%"PRIx32,
+ len);
+ break;
+ case ENOMEM:
+ ERR(file->handle, "out of memory");
+ break;
+ default:
+ ERR(file->handle,
+ "cannot get module version string (at section %u)",
+ i);
+ }
goto cleanup;
}
- (*version)[len] = '\0';
seen |= SEEN_MOD;
break;
default:
diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index 5f888d3..cdb3cde 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -1911,19 +1911,11 @@ static int perm_read(policydb_t * p
goto bad;
len = le32_to_cpu(buf[0]);
- if (zero_or_saturated(len))
+ if(str_read(&key, fp, len))
goto bad;
perdatum->s.value = le32_to_cpu(buf[1]);
- key = malloc(len + 1);
- if (!key)
- goto bad;
- rc = next_entry(key, fp, len);
- if (rc < 0)
- goto bad;
- key[len] = 0;
-
if (hashtab_insert(h, key, perdatum))
goto bad;
diff --git a/libsepol/src/private.h b/libsepol/src/private.h
index 0beb4d4..b884c23 100644
--- a/libsepol/src/private.h
+++ b/libsepol/src/private.h
@@ -65,3 +65,4 @@ extern struct policydb_compat_info *policydb_lookup_compat(unsigned int version,
extern int next_entry(void *buf, struct policy_file *fp, size_t bytes) hidden;
extern size_t put_entry(const void *ptr, size_t size, size_t n,
struct policy_file *fp) hidden;
+extern int str_read(char **strp, struct policy_file *fp, size_t len) hidden;
diff --git a/libsepol/src/services.c b/libsepol/src/services.c
index d2b80b4..f61f692 100644
--- a/libsepol/src/services.c
+++ b/libsepol/src/services.c
@@ -1679,6 +1679,39 @@ size_t hidden put_entry(const void *ptr, size_t size, size_t n,
}
/*
+ * Reads a string and null terminates it from the policy file.
+ * This is a port of str_read from the SE Linux kernel code.
+ *
+ * It returns:
+ * 0 - Success
+ * EINVAL - len is no good
+ * ENOMEM - allocation failed
+ * or any error possible from next_entry().
+ */
+int hidden str_read(char **strp, struct policy_file *fp, size_t len)
+{
+ int rc;
+ char *str;
+
+ if (zero_or_saturated(len))
+ return EINVAL;
+
+ str = malloc(len + 1);
+ if (!str)
+ return ENOMEM;
+
+ /* it's expected the caller should free the str */
+ *strp = str;
+
+ rc = next_entry(str, fp, len);
+ if (rc)
+ return rc;
+
+ str[len] = '\0';
+ return 0;
+}
+
+/*
* Read a new set of configuration data from
* a policy database binary representation file.
*
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] libsepol: calloc all the *_to_val_structs
2016-08-18 20:54 [PATCH 1/2] libsepol: calloc all the *_to_val_structs william.c.roberts
2016-08-18 20:54 ` [PATCH 2/2] libsepol: port str_read from kernel william.c.roberts
@ 2016-08-19 13:06 ` Stephen Smalley
2016-08-19 15:13 ` Roberts, William C
1 sibling, 1 reply; 8+ messages in thread
From: Stephen Smalley @ 2016-08-19 13:06 UTC (permalink / raw)
To: william.c.roberts, selinux, jwcart2, seandroid-list
On 08/18/2016 04:54 PM, william.c.roberts@intel.com wrote:
> From: William Roberts <william.c.roberts@intel.com>
>
> The usage patterns between these structures seem similair
> to role_val_to_struct usages. Calloc these up to prevent
> any unitialized usages.
>
> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> ---
> libsepol/src/mls.c | 2 +-
> libsepol/src/policydb.c | 6 +++---
> libsepol/src/users.c | 9 ++++++++-
> 3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/libsepol/src/mls.c b/libsepol/src/mls.c
> index 2dc5f2b..8047d91 100644
> --- a/libsepol/src/mls.c
> +++ b/libsepol/src/mls.c
> @@ -312,7 +312,7 @@ int mls_context_isvalid(const policydb_t * p, const context_struct_t * c)
> if (!c->user || c->user > p->p_users.nprim)
> return 0;
> usrdatum = p->user_val_to_struct[c->user - 1];
> - if (!mls_range_contains(usrdatum->exp_range, c->range))
> + if (!usrdatum || !mls_range_contains(usrdatum->exp_range, c->range))
> return 0; /* user may not be associated with range */
>
> return 1;
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index c225ac6..5f888d3 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -1074,7 +1074,7 @@ int policydb_index_others(sepol_handle_t * handle,
>
> free(p->user_val_to_struct);
> p->user_val_to_struct = (user_datum_t **)
> - malloc(p->p_users.nprim * sizeof(user_datum_t *));
> + calloc(p->p_users.nprim, sizeof(user_datum_t *));
> if (!p->user_val_to_struct)
> return -1;
>
> @@ -4006,12 +4006,12 @@ int policydb_reindex_users(policydb_t * p)
> free(p->sym_val_to_name[i]);
>
> p->user_val_to_struct = (user_datum_t **)
> - malloc(p->p_users.nprim * sizeof(user_datum_t *));
> + calloc(p->p_users.nprim, sizeof(user_datum_t *));
> if (!p->user_val_to_struct)
> return -1;
>
> p->sym_val_to_name[i] = (char **)
> - malloc(p->symtab[i].nprim * sizeof(char *));
> + calloc(p->symtab[i].nprim, sizeof(char *));
> if (!p->sym_val_to_name[i])
> return -1;
>
> diff --git a/libsepol/src/users.c b/libsepol/src/users.c
> index ce54c2b..3ffb166 100644
> --- a/libsepol/src/users.c
> +++ b/libsepol/src/users.c
> @@ -19,12 +19,17 @@ static int user_to_record(sepol_handle_t * handle,
>
> const char *name = policydb->p_user_val_to_name[user_idx];
> user_datum_t *usrdatum = policydb->user_val_to_struct[user_idx];
> - ebitmap_t *roles = &(usrdatum->roles.roles);
> + ebitmap_t *roles;
> ebitmap_node_t *rnode;
> unsigned bit;
>
> sepol_user_t *tmp_record = NULL;
>
> + if (!usrdatum)
> + goto err;
> +
> + roles = &(usrdatum->roles.roles);
> +
> if (sepol_user_create(handle, &tmp_record) < 0)
> goto err;
>
> @@ -234,6 +239,7 @@ int sepol_user_modify(sepol_handle_t * handle,
> if (!tmp_ptr)
> goto omem;
> policydb->user_val_to_struct = tmp_ptr;
> + policydb->user_val_to_struct[policydb->p_users.nprim] = NULL;
>
> tmp_ptr = realloc(policydb->sym_val_to_name[SYM_USERS],
> (policydb->p_users.nprim +
> @@ -241,6 +247,7 @@ int sepol_user_modify(sepol_handle_t * handle,
> if (!tmp_ptr)
> goto omem;
> policydb->sym_val_to_name[SYM_USERS] = tmp_ptr;
> + policydb->p_user_val_to_name[policydb->p_users.nprim] = NULL;
This one is wrong.
>
> /* Need to copy the user name */
> name = strdup(cname);
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] libsepol: port str_read from kernel
2016-08-18 20:54 ` [PATCH 2/2] libsepol: port str_read from kernel william.c.roberts
@ 2016-08-19 13:13 ` Stephen Smalley
2016-08-19 14:54 ` William Roberts
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Smalley @ 2016-08-19 13:13 UTC (permalink / raw)
To: william.c.roberts, selinux, jwcart2, seandroid-list
On 08/18/2016 04:54 PM, william.c.roberts@intel.com wrote:
> From: William Roberts <william.c.roberts@intel.com>
>
> Rather than duplicating the following sequence:
> 1. Read len from file
> 2. alloc up space based on 1
> 3. read the contents into the buffer from 2
> 4. null terminate the buffer from 2
>
> Use the str_read() function that is in the kernel, which
> collapses steps 2 and 4. This not only reduces redundant
> code, but also has the side-affect of providing a central
> check on zero_or_saturated lengths from step 1 when
> generating string values.
>
> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> ---
> libsepol/src/conditional.c | 9 +------
> libsepol/src/module.c | 66 ++++++++++++++++++++++------------------------
> libsepol/src/policydb.c | 10 +------
> libsepol/src/private.h | 1 +
> libsepol/src/services.c | 33 +++++++++++++++++++++++
> 5 files changed, 68 insertions(+), 51 deletions(-)
>
> diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c
> index 8680eb2..e1bc961 100644
> --- a/libsepol/src/conditional.c
> +++ b/libsepol/src/conditional.c
> @@ -589,15 +589,8 @@ int cond_read_bool(policydb_t * p,
> goto err;
>
> len = le32_to_cpu(buf[2]);
> - if (zero_or_saturated(len))
> + if (str_read(&key, fp, len))
> goto err;
> - key = malloc(len + 1);
> - if (!key)
> - goto err;
> - rc = next_entry(key, fp, len);
> - if (rc < 0)
> - goto err;
> - key[len] = 0;
>
> if (p->policy_type != POLICY_KERN &&
> p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
> diff --git a/libsepol/src/module.c b/libsepol/src/module.c
> index f25df95..a9d7c54 100644
> --- a/libsepol/src/module.c
> +++ b/libsepol/src/module.c
> @@ -793,26 +793,26 @@ int sepol_module_package_info(struct sepol_policy_file *spf, int *type,
> i);
> goto cleanup;
> }
> +
> len = le32_to_cpu(buf[0]);
> - if (zero_or_saturated(len)) {
> - ERR(file->handle,
> - "invalid module name length: 0x%"PRIx32,
> - len);
> - goto cleanup;
> - }
> - *name = malloc(len + 1);
> - if (!*name) {
> - ERR(file->handle, "out of memory");
> - goto cleanup;
> - }
> - rc = next_entry(*name, file, len);
> - if (rc < 0) {
> - ERR(file->handle,
> - "cannot get module name string (at section %u)",
> - i);
> + if (str_read(name, file, len)) {
> + switch(rc) {
> + case EINVAL:
> + ERR(file->handle,
> + "invalid module name length: 0x%"PRIx32,
> + len);
> + break;
> + case ENOMEM:
> + ERR(file->handle, "out of memory");
> + break;
> + default:
> + ERR(file->handle,
> + "cannot get module name string (at section %u)",
> + i);
> + }
1) You didn't set rc = str_read(), so you can't switch on it above.
2) Using positive values for errors is likely to confuse matters when
interacting with the existing code, which always uses negative values
(either -errno as a legacy of common code with the kernel or -1).
3) I think this overcomplicates the error handling / reporting. If
str_read() were to set errno and return -1 instead, then you could just
include strerror(errno) in a single error message. Or you can just
always report the most general error message. But it isn't worth a
switch statement.
Same applies throughout.
> goto cleanup;
> }
> - (*name)[len] = '\0';
> +
> rc = next_entry(buf, file, sizeof(uint32_t));
> if (rc < 0) {
> ERR(file->handle,
> @@ -821,25 +821,23 @@ int sepol_module_package_info(struct sepol_policy_file *spf, int *type,
> goto cleanup;
> }
> len = le32_to_cpu(buf[0]);
> - if (zero_or_saturated(len)) {
> - ERR(file->handle,
> - "invalid module version length: 0x%"PRIx32,
> - len);
> - goto cleanup;
> - }
> - *version = malloc(len + 1);
> - if (!*version) {
> - ERR(file->handle, "out of memory");
> - goto cleanup;
> - }
> - rc = next_entry(*version, file, len);
> - if (rc < 0) {
> - ERR(file->handle,
> - "cannot get module version string (at section %u)",
> - i);
> + if (str_read(version, file, len)) {
> + switch(rc) {
> + case EINVAL:
> + ERR(file->handle,
> + "invalid module name length: 0x%"PRIx32,
> + len);
> + break;
> + case ENOMEM:
> + ERR(file->handle, "out of memory");
> + break;
> + default:
> + ERR(file->handle,
> + "cannot get module version string (at section %u)",
> + i);
> + }
> goto cleanup;
> }
> - (*version)[len] = '\0';
> seen |= SEEN_MOD;
> break;
> default:
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index 5f888d3..cdb3cde 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -1911,19 +1911,11 @@ static int perm_read(policydb_t * p
> goto bad;
>
> len = le32_to_cpu(buf[0]);
> - if (zero_or_saturated(len))
> + if(str_read(&key, fp, len))
> goto bad;
>
> perdatum->s.value = le32_to_cpu(buf[1]);
>
> - key = malloc(len + 1);
> - if (!key)
> - goto bad;
> - rc = next_entry(key, fp, len);
> - if (rc < 0)
> - goto bad;
> - key[len] = 0;
> -
> if (hashtab_insert(h, key, perdatum))
> goto bad;
>
> diff --git a/libsepol/src/private.h b/libsepol/src/private.h
> index 0beb4d4..b884c23 100644
> --- a/libsepol/src/private.h
> +++ b/libsepol/src/private.h
> @@ -65,3 +65,4 @@ extern struct policydb_compat_info *policydb_lookup_compat(unsigned int version,
> extern int next_entry(void *buf, struct policy_file *fp, size_t bytes) hidden;
> extern size_t put_entry(const void *ptr, size_t size, size_t n,
> struct policy_file *fp) hidden;
> +extern int str_read(char **strp, struct policy_file *fp, size_t len) hidden;
> diff --git a/libsepol/src/services.c b/libsepol/src/services.c
> index d2b80b4..f61f692 100644
> --- a/libsepol/src/services.c
> +++ b/libsepol/src/services.c
> @@ -1679,6 +1679,39 @@ size_t hidden put_entry(const void *ptr, size_t size, size_t n,
> }
>
> /*
> + * Reads a string and null terminates it from the policy file.
> + * This is a port of str_read from the SE Linux kernel code.
> + *
> + * It returns:
> + * 0 - Success
> + * EINVAL - len is no good
> + * ENOMEM - allocation failed
> + * or any error possible from next_entry().
> + */
> +int hidden str_read(char **strp, struct policy_file *fp, size_t len)
> +{
> + int rc;
> + char *str;
> +
> + if (zero_or_saturated(len))
> + return EINVAL;
> +
> + str = malloc(len + 1);
> + if (!str)
> + return ENOMEM;
> +
> + /* it's expected the caller should free the str */
> + *strp = str;
> +
> + rc = next_entry(str, fp, len);
> + if (rc)
> + return rc;
> +
> + str[len] = '\0';
> + return 0;
> +}
> +
> +/*
> * Read a new set of configuration data from
> * a policy database binary representation file.
> *
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] libsepol: port str_read from kernel
2016-08-19 13:13 ` Stephen Smalley
@ 2016-08-19 14:54 ` William Roberts
2016-08-19 15:26 ` Stephen Smalley
0 siblings, 1 reply; 8+ messages in thread
From: William Roberts @ 2016-08-19 14:54 UTC (permalink / raw)
To: Stephen Smalley; +Cc: William Roberts, James Carter, selinux, seandroid-list
[-- Attachment #1: Type: text/plain, Size: 10046 bytes --]
On Aug 19, 2016 06:12, "Stephen Smalley" <sds@tycho.nsa.gov> wrote:
>
> On 08/18/2016 04:54 PM, william.c.roberts@intel.com wrote:
> > From: William Roberts <william.c.roberts@intel.com>
> >
> > Rather than duplicating the following sequence:
> > 1. Read len from file
> > 2. alloc up space based on 1
> > 3. read the contents into the buffer from 2
> > 4. null terminate the buffer from 2
> >
> > Use the str_read() function that is in the kernel, which
> > collapses steps 2 and 4. This not only reduces redundant
> > code, but also has the side-affect of providing a central
> > check on zero_or_saturated lengths from step 1 when
> > generating string values.
> >
> > Signed-off-by: William Roberts <william.c.roberts@intel.com>
> > ---
> > libsepol/src/conditional.c | 9 +------
> > libsepol/src/module.c | 66
++++++++++++++++++++++------------------------
> > libsepol/src/policydb.c | 10 +------
> > libsepol/src/private.h | 1 +
> > libsepol/src/services.c | 33 +++++++++++++++++++++++
> > 5 files changed, 68 insertions(+), 51 deletions(-)
> >
> > diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c
> > index 8680eb2..e1bc961 100644
> > --- a/libsepol/src/conditional.c
> > +++ b/libsepol/src/conditional.c
> > @@ -589,15 +589,8 @@ int cond_read_bool(policydb_t * p,
> > goto err;
> >
> > len = le32_to_cpu(buf[2]);
> > - if (zero_or_saturated(len))
> > + if (str_read(&key, fp, len))
> > goto err;
> > - key = malloc(len + 1);
> > - if (!key)
> > - goto err;
> > - rc = next_entry(key, fp, len);
> > - if (rc < 0)
> > - goto err;
> > - key[len] = 0;
> >
> > if (p->policy_type != POLICY_KERN &&
> > p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
> > diff --git a/libsepol/src/module.c b/libsepol/src/module.c
> > index f25df95..a9d7c54 100644
> > --- a/libsepol/src/module.c
> > +++ b/libsepol/src/module.c
> > @@ -793,26 +793,26 @@ int sepol_module_package_info(struct
sepol_policy_file *spf, int *type,
> > i);
> > goto cleanup;
> > }
> > +
> > len = le32_to_cpu(buf[0]);
> > - if (zero_or_saturated(len)) {
> > - ERR(file->handle,
> > - "invalid module name length:
0x%"PRIx32,
> > - len);
> > - goto cleanup;
> > - }
> > - *name = malloc(len + 1);
> > - if (!*name) {
> > - ERR(file->handle, "out of memory");
> > - goto cleanup;
> > - }
> > - rc = next_entry(*name, file, len);
> > - if (rc < 0) {
> > - ERR(file->handle,
> > - "cannot get module name string (at
section %u)",
> > - i);
> > + if (str_read(name, file, len)) {
> > + switch(rc) {
> > + case EINVAL:
> > + ERR(file->handle,
> > + "invalid module name
length: 0x%"PRIx32,
> > + len);
> > + break;
> > + case ENOMEM:
> > + ERR(file->handle, "out of
memory");
> > + break;
> > + default:
> > + ERR(file->handle,
> > + "cannot get module name
string (at section %u)",
> > + i);
> > + }
>
> 1) You didn't set rc = str_read(), so you can't switch on it above.
It's a new gnuc extension ;-p
> 2) Using positive values for errors is likely to confuse matters when
> interacting with the existing code, which always uses negative values
> (either -errno as a legacy of common code with the kernel or -1).
> 3) I think this overcomplicates the error handling / reporting. If
> str_read() were to set errno and return -1 instead, then you could just
> include strerror(errno) in a single error message. Or you can just
> always report the most general error message. But it isn't worth a
> switch statement.
Yeah I had all those thoughts, wasn't sure the best way considering the
conflict on -1 vs -errno. If you're OK with setting errno let's use that.
>
> Same applies throughout.
>
> > goto cleanup;
> > }
> > - (*name)[len] = '\0';
> > +
> > rc = next_entry(buf, file, sizeof(uint32_t));
> > if (rc < 0) {
> > ERR(file->handle,
> > @@ -821,25 +821,23 @@ int sepol_module_package_info(struct
sepol_policy_file *spf, int *type,
> > goto cleanup;
> > }
> > len = le32_to_cpu(buf[0]);
> > - if (zero_or_saturated(len)) {
> > - ERR(file->handle,
> > - "invalid module version length:
0x%"PRIx32,
> > - len);
> > - goto cleanup;
> > - }
> > - *version = malloc(len + 1);
> > - if (!*version) {
> > - ERR(file->handle, "out of memory");
> > - goto cleanup;
> > - }
> > - rc = next_entry(*version, file, len);
> > - if (rc < 0) {
> > - ERR(file->handle,
> > - "cannot get module version string (at
section %u)",
> > - i);
> > + if (str_read(version, file, len)) {
> > + switch(rc) {
> > + case EINVAL:
> > + ERR(file->handle,
> > + "invalid module name
length: 0x%"PRIx32,
> > + len);
> > + break;
> > + case ENOMEM:
> > + ERR(file->handle, "out of
memory");
> > + break;
> > + default:
> > + ERR(file->handle,
> > + "cannot get module
version string (at section %u)",
> > + i);
> > + }
> > goto cleanup;
> > }
> > - (*version)[len] = '\0';
> > seen |= SEEN_MOD;
> > break;
> > default:
> > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> > index 5f888d3..cdb3cde 100644
> > --- a/libsepol/src/policydb.c
> > +++ b/libsepol/src/policydb.c
> > @@ -1911,19 +1911,11 @@ static int perm_read(policydb_t * p
> > goto bad;
> >
> > len = le32_to_cpu(buf[0]);
> > - if (zero_or_saturated(len))
> > + if(str_read(&key, fp, len))
> > goto bad;
> >
> > perdatum->s.value = le32_to_cpu(buf[1]);
> >
> > - key = malloc(len + 1);
> > - if (!key)
> > - goto bad;
> > - rc = next_entry(key, fp, len);
> > - if (rc < 0)
> > - goto bad;
> > - key[len] = 0;
> > -
> > if (hashtab_insert(h, key, perdatum))
> > goto bad;
> >
> > diff --git a/libsepol/src/private.h b/libsepol/src/private.h
> > index 0beb4d4..b884c23 100644
> > --- a/libsepol/src/private.h
> > +++ b/libsepol/src/private.h
> > @@ -65,3 +65,4 @@ extern struct policydb_compat_info
*policydb_lookup_compat(unsigned int version,
> > extern int next_entry(void *buf, struct policy_file *fp, size_t bytes)
hidden;
> > extern size_t put_entry(const void *ptr, size_t size, size_t n,
> > struct policy_file *fp) hidden;
> > +extern int str_read(char **strp, struct policy_file *fp, size_t len)
hidden;
> > diff --git a/libsepol/src/services.c b/libsepol/src/services.c
> > index d2b80b4..f61f692 100644
> > --- a/libsepol/src/services.c
> > +++ b/libsepol/src/services.c
> > @@ -1679,6 +1679,39 @@ size_t hidden put_entry(const void *ptr, size_t
size, size_t n,
> > }
> >
> > /*
> > + * Reads a string and null terminates it from the policy file.
> > + * This is a port of str_read from the SE Linux kernel code.
> > + *
> > + * It returns:
> > + * 0 - Success
> > + * EINVAL - len is no good
> > + * ENOMEM - allocation failed
> > + * or any error possible from next_entry().
> > + */
> > +int hidden str_read(char **strp, struct policy_file *fp, size_t len)
> > +{
> > + int rc;
> > + char *str;
> > +
> > + if (zero_or_saturated(len))
> > + return EINVAL;
> > +
> > + str = malloc(len + 1);
> > + if (!str)
> > + return ENOMEM;
> > +
> > + /* it's expected the caller should free the str */
> > + *strp = str;
> > +
> > + rc = next_entry(str, fp, len);
> > + if (rc)
> > + return rc;
> > +
> > + str[len] = '\0';
> > + return 0;
> > +}
> > +
> > +/*
> > * Read a new set of configuration data from
> > * a policy database binary representation file.
> > *
> >
>
> _______________________________________________
> Seandroid-list mailing list
> Seandroid-list@tycho.nsa.gov
> To unsubscribe, send email to Seandroid-list-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to
Seandroid-list-request@tycho.nsa.gov.
[-- Attachment #2: Type: text/html, Size: 14766 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 1/2] libsepol: calloc all the *_to_val_structs
2016-08-19 13:06 ` [PATCH 1/2] libsepol: calloc all the *_to_val_structs Stephen Smalley
@ 2016-08-19 15:13 ` Roberts, William C
2016-08-19 15:38 ` Stephen Smalley
0 siblings, 1 reply; 8+ messages in thread
From: Roberts, William C @ 2016-08-19 15:13 UTC (permalink / raw)
To: Stephen Smalley, selinux, jwcart2, seandroid-list
> -----Original Message-----
> From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> Sent: Friday, August 19, 2016 6:06 AM
> To: Roberts, William C <william.c.roberts@intel.com>; selinux@tycho.nsa.gov;
> jwcart2@tycho.nsa.gov; seandroid-list@tycho.nsa.gov
> Subject: Re: [PATCH 1/2] libsepol: calloc all the *_to_val_structs
>
> On 08/18/2016 04:54 PM, william.c.roberts@intel.com wrote:
> > From: William Roberts <william.c.roberts@intel.com>
> >
> > The usage patterns between these structures seem similair to
> > role_val_to_struct usages. Calloc these up to prevent any unitialized
> > usages.
> >
> > Signed-off-by: William Roberts <william.c.roberts@intel.com>
> > ---
> > libsepol/src/mls.c | 2 +-
> > libsepol/src/policydb.c | 6 +++---
> > libsepol/src/users.c | 9 ++++++++-
> > 3 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/libsepol/src/mls.c b/libsepol/src/mls.c index
> > 2dc5f2b..8047d91 100644
> > --- a/libsepol/src/mls.c
> > +++ b/libsepol/src/mls.c
> > @@ -312,7 +312,7 @@ int mls_context_isvalid(const policydb_t * p, const
> context_struct_t * c)
> > if (!c->user || c->user > p->p_users.nprim)
> > return 0;
> > usrdatum = p->user_val_to_struct[c->user - 1];
> > - if (!mls_range_contains(usrdatum->exp_range, c->range))
> > + if (!usrdatum || !mls_range_contains(usrdatum->exp_range, c->range))
> > return 0; /* user may not be associated with range */
> >
> > return 1;
> > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c index
> > c225ac6..5f888d3 100644
> > --- a/libsepol/src/policydb.c
> > +++ b/libsepol/src/policydb.c
> > @@ -1074,7 +1074,7 @@ int policydb_index_others(sepol_handle_t *
> > handle,
> >
> > free(p->user_val_to_struct);
> > p->user_val_to_struct = (user_datum_t **)
> > - malloc(p->p_users.nprim * sizeof(user_datum_t *));
> > + calloc(p->p_users.nprim, sizeof(user_datum_t *));
> > if (!p->user_val_to_struct)
> > return -1;
> >
> > @@ -4006,12 +4006,12 @@ int policydb_reindex_users(policydb_t * p)
> > free(p->sym_val_to_name[i]);
> >
> > p->user_val_to_struct = (user_datum_t **)
> > - malloc(p->p_users.nprim * sizeof(user_datum_t *));
> > + calloc(p->p_users.nprim, sizeof(user_datum_t *));
> > if (!p->user_val_to_struct)
> > return -1;
> >
> > p->sym_val_to_name[i] = (char **)
> > - malloc(p->symtab[i].nprim * sizeof(char *));
> > + calloc(p->symtab[i].nprim, sizeof(char *));
> > if (!p->sym_val_to_name[i])
> > return -1;
> >
> > diff --git a/libsepol/src/users.c b/libsepol/src/users.c index
> > ce54c2b..3ffb166 100644
> > --- a/libsepol/src/users.c
> > +++ b/libsepol/src/users.c
> > @@ -19,12 +19,17 @@ static int user_to_record(sepol_handle_t * handle,
> >
> > const char *name = policydb->p_user_val_to_name[user_idx];
> > user_datum_t *usrdatum = policydb->user_val_to_struct[user_idx];
> > - ebitmap_t *roles = &(usrdatum->roles.roles);
> > + ebitmap_t *roles;
> > ebitmap_node_t *rnode;
> > unsigned bit;
> >
> > sepol_user_t *tmp_record = NULL;
> >
> > + if (!usrdatum)
> > + goto err;
> > +
> > + roles = &(usrdatum->roles.roles);
> > +
> > if (sepol_user_create(handle, &tmp_record) < 0)
> > goto err;
> >
> > @@ -234,6 +239,7 @@ int sepol_user_modify(sepol_handle_t * handle,
> > if (!tmp_ptr)
> > goto omem;
> > policydb->user_val_to_struct = tmp_ptr;
> > + policydb->user_val_to_struct[policydb->p_users.nprim] = NULL;
> >
> > tmp_ptr = realloc(policydb->sym_val_to_name[SYM_USERS],
> > (policydb->p_users.nprim +
> > @@ -241,6 +247,7 @@ int sepol_user_modify(sepol_handle_t * handle,
> > if (!tmp_ptr)
> > goto omem;
> > policydb->sym_val_to_name[SYM_USERS] = tmp_ptr;
> > + policydb->p_user_val_to_name[policydb->p_users.nprim] =
> NULL;
>
> This one is wrong.
>
> >
> > /* Need to copy the user name */
> > name = strdup(cname);
> >
After looking at the email and the patch again, that’s just a context hunk, I see no + or - next it,
And I verified it still exists in my source tree. I never touched that hunk or am I missing
Some subtle interaction?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] libsepol: port str_read from kernel
2016-08-19 14:54 ` William Roberts
@ 2016-08-19 15:26 ` Stephen Smalley
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Smalley @ 2016-08-19 15:26 UTC (permalink / raw)
To: William Roberts; +Cc: William Roberts, James Carter, selinux, seandroid-list
On 08/19/2016 10:54 AM, William Roberts wrote:
> On Aug 19, 2016 06:12, "Stephen Smalley" <sds@tycho.nsa.gov
> <mailto:sds@tycho.nsa.gov>> wrote:
>>
>> On 08/18/2016 04:54 PM, william.c.roberts@intel.com
> <mailto:william.c.roberts@intel.com> wrote:
>> > From: William Roberts <william.c.roberts@intel.com
> <mailto:william.c.roberts@intel.com>>
>> >
>> > Rather than duplicating the following sequence:
>> > 1. Read len from file
>> > 2. alloc up space based on 1
>> > 3. read the contents into the buffer from 2
>> > 4. null terminate the buffer from 2
>> >
>> > Use the str_read() function that is in the kernel, which
>> > collapses steps 2 and 4. This not only reduces redundant
>> > code, but also has the side-affect of providing a central
>> > check on zero_or_saturated lengths from step 1 when
>> > generating string values.
>> >
>> > Signed-off-by: William Roberts <william.c.roberts@intel.com
> <mailto:william.c.roberts@intel.com>>
>> > ---
>> > libsepol/src/conditional.c | 9 +------
>> > libsepol/src/module.c | 66
> ++++++++++++++++++++++------------------------
>> > libsepol/src/policydb.c | 10 +------
>> > libsepol/src/private.h | 1 +
>> > libsepol/src/services.c | 33 +++++++++++++++++++++++
>> > 5 files changed, 68 insertions(+), 51 deletions(-)
>> >
>> > diff --git a/libsepol/src/conditional.c b/libsepol/src/conditional.c
>> > index 8680eb2..e1bc961 100644
>> > --- a/libsepol/src/conditional.c
>> > +++ b/libsepol/src/conditional.c
>> > @@ -589,15 +589,8 @@ int cond_read_bool(policydb_t * p,
>> > goto err;
>> >
>> > len = le32_to_cpu(buf[2]);
>> > - if (zero_or_saturated(len))
>> > + if (str_read(&key, fp, len))
>> > goto err;
>> > - key = malloc(len + 1);
>> > - if (!key)
>> > - goto err;
>> > - rc = next_entry(key, fp, len);
>> > - if (rc < 0)
>> > - goto err;
>> > - key[len] = 0;
>> >
>> > if (p->policy_type != POLICY_KERN &&
>> > p->policyvers >= MOD_POLICYDB_VERSION_TUNABLE_SEP) {
>> > diff --git a/libsepol/src/module.c b/libsepol/src/module.c
>> > index f25df95..a9d7c54 100644
>> > --- a/libsepol/src/module.c
>> > +++ b/libsepol/src/module.c
>> > @@ -793,26 +793,26 @@ int sepol_module_package_info(struct
> sepol_policy_file *spf, int *type,
>> > i);
>> > goto cleanup;
>> > }
>> > +
>> > len = le32_to_cpu(buf[0]);
>> > - if (zero_or_saturated(len)) {
>> > - ERR(file->handle,
>> > - "invalid module name length:
> 0x%"PRIx32,
>> > - len);
>> > - goto cleanup;
>> > - }
>> > - *name = malloc(len + 1);
>> > - if (!*name) {
>> > - ERR(file->handle, "out of memory");
>> > - goto cleanup;
>> > - }
>> > - rc = next_entry(*name, file, len);
>> > - if (rc < 0) {
>> > - ERR(file->handle,
>> > - "cannot get module name string (at
> section %u)",
>> > - i);
>> > + if (str_read(name, file, len)) {
>> > + switch(rc) {
>> > + case EINVAL:
>> > + ERR(file->handle,
>> > + "invalid module name
> length: 0x%"PRIx32,
>> > + len);
>> > + break;
>> > + case ENOMEM:
>> > + ERR(file->handle, "out of
> memory");
>> > + break;
>> > + default:
>> > + ERR(file->handle,
>> > + "cannot get module
> name string (at section %u)",
>> > + i);
>> > + }
>>
>> 1) You didn't set rc = str_read(), so you can't switch on it above.
>
> It's a new gnuc extension ;-p
>
>> 2) Using positive values for errors is likely to confuse matters when
>> interacting with the existing code, which always uses negative values
>> (either -errno as a legacy of common code with the kernel or -1).
>> 3) I think this overcomplicates the error handling / reporting. If
>> str_read() were to set errno and return -1 instead, then you could just
>> include strerror(errno) in a single error message. Or you can just
>> always report the most general error message. But it isn't worth a
>> switch statement.
>
> Yeah I had all those thoughts, wasn't sure the best way considering the
> conflict on -1 vs -errno. If you're OK with setting errno let's use that.
Yes, setting errno is fine with me. Not even necessary if the malloc fails.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] libsepol: calloc all the *_to_val_structs
2016-08-19 15:13 ` Roberts, William C
@ 2016-08-19 15:38 ` Stephen Smalley
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Smalley @ 2016-08-19 15:38 UTC (permalink / raw)
To: Roberts, William C, selinux, jwcart2, seandroid-list
On 08/19/2016 11:13 AM, Roberts, William C wrote:
>
>
>> -----Original Message-----
>> From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
>> Sent: Friday, August 19, 2016 6:06 AM
>> To: Roberts, William C <william.c.roberts@intel.com>; selinux@tycho.nsa.gov;
>> jwcart2@tycho.nsa.gov; seandroid-list@tycho.nsa.gov
>> Subject: Re: [PATCH 1/2] libsepol: calloc all the *_to_val_structs
>>
>> On 08/18/2016 04:54 PM, william.c.roberts@intel.com wrote:
>>> From: William Roberts <william.c.roberts@intel.com>
>>>
>>> The usage patterns between these structures seem similair to
>>> role_val_to_struct usages. Calloc these up to prevent any unitialized
>>> usages.
>>>
>>> Signed-off-by: William Roberts <william.c.roberts@intel.com>
>>> ---
>>> libsepol/src/mls.c | 2 +-
>>> libsepol/src/policydb.c | 6 +++---
>>> libsepol/src/users.c | 9 ++++++++-
>>> 3 files changed, 12 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libsepol/src/mls.c b/libsepol/src/mls.c index
>>> 2dc5f2b..8047d91 100644
>>> --- a/libsepol/src/mls.c
>>> +++ b/libsepol/src/mls.c
>>> @@ -312,7 +312,7 @@ int mls_context_isvalid(const policydb_t * p, const
>> context_struct_t * c)
>>> if (!c->user || c->user > p->p_users.nprim)
>>> return 0;
>>> usrdatum = p->user_val_to_struct[c->user - 1];
>>> - if (!mls_range_contains(usrdatum->exp_range, c->range))
>>> + if (!usrdatum || !mls_range_contains(usrdatum->exp_range, c->range))
>>> return 0; /* user may not be associated with range */
>>>
>>> return 1;
>>> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c index
>>> c225ac6..5f888d3 100644
>>> --- a/libsepol/src/policydb.c
>>> +++ b/libsepol/src/policydb.c
>>> @@ -1074,7 +1074,7 @@ int policydb_index_others(sepol_handle_t *
>>> handle,
>>>
>>> free(p->user_val_to_struct);
>>> p->user_val_to_struct = (user_datum_t **)
>>> - malloc(p->p_users.nprim * sizeof(user_datum_t *));
>>> + calloc(p->p_users.nprim, sizeof(user_datum_t *));
>>> if (!p->user_val_to_struct)
>>> return -1;
>>>
>>> @@ -4006,12 +4006,12 @@ int policydb_reindex_users(policydb_t * p)
>>> free(p->sym_val_to_name[i]);
>>>
>>> p->user_val_to_struct = (user_datum_t **)
>>> - malloc(p->p_users.nprim * sizeof(user_datum_t *));
>>> + calloc(p->p_users.nprim, sizeof(user_datum_t *));
>>> if (!p->user_val_to_struct)
>>> return -1;
>>>
>>> p->sym_val_to_name[i] = (char **)
>>> - malloc(p->symtab[i].nprim * sizeof(char *));
>>> + calloc(p->symtab[i].nprim, sizeof(char *));
>>> if (!p->sym_val_to_name[i])
>>> return -1;
>>>
>>> diff --git a/libsepol/src/users.c b/libsepol/src/users.c index
>>> ce54c2b..3ffb166 100644
>>> --- a/libsepol/src/users.c
>>> +++ b/libsepol/src/users.c
>>> @@ -19,12 +19,17 @@ static int user_to_record(sepol_handle_t * handle,
>>>
>>> const char *name = policydb->p_user_val_to_name[user_idx];
>>> user_datum_t *usrdatum = policydb->user_val_to_struct[user_idx];
>>> - ebitmap_t *roles = &(usrdatum->roles.roles);
>>> + ebitmap_t *roles;
>>> ebitmap_node_t *rnode;
>>> unsigned bit;
>>>
>>> sepol_user_t *tmp_record = NULL;
>>>
>>> + if (!usrdatum)
>>> + goto err;
>>> +
>>> + roles = &(usrdatum->roles.roles);
>>> +
>>> if (sepol_user_create(handle, &tmp_record) < 0)
>>> goto err;
>>>
>>> @@ -234,6 +239,7 @@ int sepol_user_modify(sepol_handle_t * handle,
>>> if (!tmp_ptr)
>>> goto omem;
>>> policydb->user_val_to_struct = tmp_ptr;
>>> + policydb->user_val_to_struct[policydb->p_users.nprim] = NULL;
>>>
>>> tmp_ptr = realloc(policydb->sym_val_to_name[SYM_USERS],
>>> (policydb->p_users.nprim +
>>> @@ -241,6 +247,7 @@ int sepol_user_modify(sepol_handle_t * handle,
>>> if (!tmp_ptr)
>>> goto omem;
>>> policydb->sym_val_to_name[SYM_USERS] = tmp_ptr;
>>> + policydb->p_user_val_to_name[policydb->p_users.nprim] =
>> NULL;
>>
>> This one is wrong.
>>
>>>
>>> /* Need to copy the user name */
>>> name = strdup(cname);
>>>
>
> After looking at the email and the patch again, that’s just a context hunk, I see no + or - next it,
> And I verified it still exists in my source tree. I never touched that hunk or am I missing
> Some subtle interaction?
I was referring to the lines above that you added.
But on second thought, never mind - I think this one is ok.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-08-19 15:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-18 20:54 [PATCH 1/2] libsepol: calloc all the *_to_val_structs william.c.roberts
2016-08-18 20:54 ` [PATCH 2/2] libsepol: port str_read from kernel william.c.roberts
2016-08-19 13:13 ` Stephen Smalley
2016-08-19 14:54 ` William Roberts
2016-08-19 15:26 ` Stephen Smalley
2016-08-19 13:06 ` [PATCH 1/2] libsepol: calloc all the *_to_val_structs Stephen Smalley
2016-08-19 15:13 ` Roberts, William C
2016-08-19 15:38 ` Stephen Smalley
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.