Re: [PATCH] Smack: remove task_wait() hook.

[Casey Schaufler]

On 8/13/2012 4:45 AM, Jarkko Sakkinen wrote:
> On Thu, Aug 09, 2012 at 05:46:38PM -0700, Casey Schaufler wrote:
>> On 12/20/2011 11:20 PM, Jarkko Sakkinen wrote:
>>> Allow SIGCHLD to be passed to child process without
>>> explicit policy. This will help to keep the access
>>> control policy simple and easily maintainable with
>>> complex applications that require use of multiple
>>> security contexts. It will also help to keep them
>>> as isolated as possible.
>>>
>>> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxx>
>> I have a slightly different version that applies to the
>> current smack-next tree.
>>
>> Allow SIGCHLD to be passed to child process without
>> explicit policy. This will help to keep the access
>> control policy simple and easily maintainable with
>> complex applications that require use of multiple
>> security contexts. It will also help to keep them
>> as isolated as possible.
>>
>> Signed-off-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
> Acked-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxx>

Applied to git://git.gitorious.org/smack-next/kernel.git

>>  security/smack/smack_lsm.c |   37 ++++++++-----------------------------
>>  1 files changed, 8 insertions(+), 29 deletions(-)
>>
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index 8221514..ce9273a 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -1691,40 +1691,19 @@ static int smack_task_kill(struct task_struct *p, struct siginfo *info,
>>   * smack_task_wait - Smack access check for waiting
>>   * @p: task to wait for
>>   *
>> - * Returns 0 if current can wait for p, error code otherwise
>> + * Returns 0
>>   */
>>  static int smack_task_wait(struct task_struct *p)
>>  {
>> -	struct smk_audit_info ad;
>> -	char *sp = smk_of_current();
>> -	char *tsp = smk_of_forked(task_security(p));
>> -	int rc;
>> -
>> -	/* we don't log here, we can be overriden */
>> -	rc = smk_access(tsp, sp, MAY_WRITE, NULL);
>> -	if (rc == 0)
>> -		goto out_log;
>> -
>>  	/*
>> -	 * Allow the operation to succeed if either task
>> -	 * has privilege to perform operations that might
>> -	 * account for the smack labels having gotten to
>> -	 * be different in the first place.
>> -	 *
>> -	 * This breaks the strict subject/object access
>> -	 * control ideal, taking the object's privilege
>> -	 * state into account in the decision as well as
>> -	 * the smack value.
>> +	 * Allow the operation to succeed.
>> +	 * Zombies are bad.
>> +	 * In userless environments (e.g. phones) programs
>> +	 * get marked with SMACK64EXEC and even if the parent
>> +	 * and child shouldn't be talking the parent still
>> +	 * may expect to know when the child exits.
>>  	 */
>> -	if (smack_privileged(CAP_MAC_OVERRIDE) ||
>> -	    has_capability(p, CAP_MAC_OVERRIDE))
>> -		rc = 0;
>> -	/* we log only if we didn't get overriden */
>> - out_log:
>> -	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK);
>> -	smk_ad_setfield_u_tsk(&ad, p);
>> -	smack_log(tsp, sp, MAY_WRITE, rc, &ad);
>> -	return rc;
>> +	return 0;
>>  }
>>  
>>  /**
>>
>>> ---
>>>  security/smack/smack_lsm.c |   40 ----------------------------------------
>>>  1 files changed, 0 insertions(+), 40 deletions(-)
>>>
>>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>>> index 7db62b4..cc788f5 100644
>>> --- a/security/smack/smack_lsm.c
>>> +++ b/security/smack/smack_lsm.c
>>> @@ -1685,45 +1685,6 @@ static int smack_task_kill(struct task_struct *p, struct siginfo *info,
>>>  }
>>>  
>>>  /**
>>> - * smack_task_wait - Smack access check for waiting
>>> - * @p: task to wait for
>>> - *
>>> - * Returns 0 if current can wait for p, error code otherwise
>>> - */
>>> -static int smack_task_wait(struct task_struct *p)
>>> -{
>>> -	struct smk_audit_info ad;
>>> -	char *sp = smk_of_current();
>>> -	char *tsp = smk_of_forked(task_security(p));
>>> -	int rc;
>>> -
>>> -	/* we don't log here, we can be overriden */
>>> -	rc = smk_access(tsp, sp, MAY_WRITE, NULL);
>>> -	if (rc == 0)
>>> -		goto out_log;
>>> -
>>> -	/*
>>> -	 * Allow the operation to succeed if either task
>>> -	 * has privilege to perform operations that might
>>> -	 * account for the smack labels having gotten to
>>> -	 * be different in the first place.
>>> -	 *
>>> -	 * This breaks the strict subject/object access
>>> -	 * control ideal, taking the object's privilege
>>> -	 * state into account in the decision as well as
>>> -	 * the smack value.
>>> -	 */
>>> -	if (capable(CAP_MAC_OVERRIDE) || has_capability(p, CAP_MAC_OVERRIDE))
>>> -		rc = 0;
>>> -	/* we log only if we didn't get overriden */
>>> - out_log:
>>> -	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_TASK);
>>> -	smk_ad_setfield_u_tsk(&ad, p);
>>> -	smack_log(tsp, sp, MAY_WRITE, rc, &ad);
>>> -	return rc;
>>> -}
>>> -
>>> -/**
>>>   * smack_task_to_inode - copy task smack into the inode blob
>>>   * @p: task to copy from
>>>   * @inode: inode to copy to
>>> @@ -3549,7 +3510,6 @@ struct security_operations smack_ops = {
>>>  	.task_getscheduler = 		smack_task_getscheduler,
>>>  	.task_movememory = 		smack_task_movememory,
>>>  	.task_kill = 			smack_task_kill,
>>> -	.task_wait = 			smack_task_wait,
>>>  	.task_to_inode = 		smack_task_to_inode,
>>>  
>>>  	.ipc_permission = 		smack_ipc_permission,
> /Jarkko
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/