linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Smack: remove task_wait() hook.
@ 2011-12-21  7:20 Jarkko Sakkinen
  2012-08-10  0:46 ` Casey Schaufler
  0 siblings, 1 reply; 4+ messages in thread
From: Jarkko Sakkinen @ 2011-12-21  7:20 UTC (permalink / raw)
  To: Casey Schaufler; +Cc: linux-kernel, linux-security-module, Jarkko Sakkinen

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@intel.com>
---
 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,
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Smack: remove task_wait() hook.
  2011-12-21  7:20 [PATCH] Smack: remove task_wait() hook Jarkko Sakkinen
@ 2012-08-10  0:46 ` Casey Schaufler
  2012-08-13 11:45   ` Jarkko Sakkinen
  0 siblings, 1 reply; 4+ messages in thread
From: Casey Schaufler @ 2012-08-10  0:46 UTC (permalink / raw)
  To: Jarkko Sakkinen; +Cc: linux-kernel, linux-security-module, Casey Schaufler

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@intel.com>

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@schaufler-ca.com>

 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,


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Smack: remove task_wait() hook.
  2012-08-10  0:46 ` Casey Schaufler
@ 2012-08-13 11:45   ` Jarkko Sakkinen
  2012-08-13 17:46     ` Casey Schaufler
  0 siblings, 1 reply; 4+ messages in thread
From: Jarkko Sakkinen @ 2012-08-13 11:45 UTC (permalink / raw)
  To: Casey Schaufler; +Cc: linux-kernel, linux-security-module

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@intel.com>
> 
> 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@schaufler-ca.com>

Acked-by: Jarkko Sakkinen <jarkko.sakkinen@intel.com>

> 
>  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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Smack: remove task_wait() hook.
  2012-08-13 11:45   ` Jarkko Sakkinen
@ 2012-08-13 17:46     ` Casey Schaufler
  0 siblings, 0 replies; 4+ messages in thread
From: Casey Schaufler @ 2012-08-13 17:46 UTC (permalink / raw)
  To: Jarkko Sakkinen; +Cc: linux-kernel, linux-security-module, 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@intel.com>
>> 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@schaufler-ca.com>
> Acked-by: Jarkko Sakkinen <jarkko.sakkinen@intel.com>

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
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-08-13 17:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-21  7:20 [PATCH] Smack: remove task_wait() hook Jarkko Sakkinen
2012-08-10  0:46 ` Casey Schaufler
2012-08-13 11:45   ` Jarkko Sakkinen
2012-08-13 17:46     ` Casey Schaufler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).