[PATCH v2 4/5] selinux: Use pointer to switch policydb and sidtab

[peter enderborg]

On 01/30/2018 03:37 PM, Stephen Smalley wrote:
> On Fri, 2018-01-26 at 15:32 +0100, peter.enderborg at sony.com wrote:
> goto err;
>  
> -	rc = security_preserve_bools(newpolicydb);
> +	rc = security_preserve_bools(&next_rcu->policydb);
>  	if (rc) {
>  		printk(KERN_ERR "SELinux:  unable to preserve
> booleans\n");
>  		goto err;
> Most of this shouldn't need to be under the read lock.
>
>> @@ -2189,7 +2194,7 @@ int security_load_policy(void *data, size_t
>> len)
>>  	 * in the new SID table.
>>  	 */
>>  	args.oldp = &crm->policydb;
>> -	args.newp = newpolicydb;
>> +	args.newp = &next_rcu->policydb;
>>  	rc = sidtab_map(&newsidtab, convert_context, &args);
>>  	if (rc) {
>>  		printk(KERN_ERR "SELinux:  unable to convert the
>> internal"
>> @@ -2204,8 +2209,9 @@ int security_load_policy(void *data, size_t
>> len)
>>  
>>  	/* Install the new policydb and SID table. */
>>  	/* next */
>> +	security_load_policycaps(&next_rcu->policydb);
> This cannot be done outside of the write lock; it has to be atomic with
> the policy switch.
Can you please elaborate, does some else write the policydb without a lock?
Is there any other data that is shared? I see this as a private until we switch the pointer.
>> +	read_unlock(&policy_rwlock);
>>  	write_lock_irq(&policy_rwlock);
>> -	memcpy(&next_rcu->policydb, newpolicydb, sizeof(struct
>> policydb));
>>  	sidtab_set(&next_rcu->sidtab, &newsidtab);
>>  	security_load_policycaps(&next_rcu->policydb);
>>  	oldmap = crm->current_mapping;
>> @@ -2213,8 +2219,9 @@ int security_load_policy(void *data, size_t
>> len)
>>  	next_rcu->current_mapping_size = map_size;
>>  
>>  	seqno = ++latest_granting;
>> -	write_unlock_irq(&policy_rwlock);
>> +	old_rcu = crm;
>>  	crm = next_rcu;
>> +	write_unlock_irq(&policy_rwlock);
>>  
>>  	/* Free the old policydb and SID table. */
>>  	policydb_destroy(oldpolicydb);
>> @@ -2226,17 +2233,16 @@ int security_load_policy(void *data, size_t
>> len)
>>  	selinux_status_update_policyload(seqno);
>>  	selinux_netlbl_cache_invalidate();
>>  	selinux_xfrm_notify_policyload();
>> +	kfree(oldpolicydb);
>> +	kfree(old_rcu);
>>  
>>  	rc = 0;
>>  	goto out;
>> -
>>  err:
>>  	kfree(map);
>>  	sidtab_destroy(&newsidtab);
>> -	policydb_destroy(newpolicydb);
>> -
>> +	

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html