Re: [PATCH 1/2] libsepol: calloc all the *_to_val_structs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/19/2016 11:13 AM, Roberts, William C wrote:
> 
> 
>> -----Original Message-----
>> From: Stephen Smalley [mailto:sds@xxxxxxxxxxxxx]
>> Sent: Friday, August 19, 2016 6:06 AM
>> To: Roberts, William C <william.c.roberts@xxxxxxxxx>; selinux@xxxxxxxxxxxxx;
>> jwcart2@xxxxxxxxxxxxx; seandroid-list@xxxxxxxxxxxxx
>> Subject: Re: [PATCH 1/2] libsepol: calloc all the *_to_val_structs
>>
>> On 08/18/2016 04:54 PM, william.c.roberts@xxxxxxxxx wrote:
>>> From: William Roberts <william.c.roberts@xxxxxxxxx>
>>>
>>> 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@xxxxxxxxx>
>>> ---
>>>  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.


_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux