linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] apparmor: make aa_set_current_onexec return void
@ 2023-01-14 16:49 Quanfa Fu
  2023-01-18  4:15 ` [apparmor] " Tyler Hicks
  0 siblings, 1 reply; 3+ messages in thread
From: Quanfa Fu @ 2023-01-14 16:49 UTC (permalink / raw)
  To: john.johansen, paul, jmorris, serge
  Cc: apparmor, linux-security-module, linux-kernel, Quanfa Fu

Change the return type to void since it always return 0, and no need
to do the checking in aa_set_current_onexec.

Signed-off-by: Quanfa Fu <quanfafu@gmail.com>
---
 security/apparmor/domain.c       | 2 +-
 security/apparmor/include/task.h | 2 +-
 security/apparmor/task.c         | 5 +----
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 6dd3cc5309bf..bbc9c8a87b8e 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -1446,7 +1446,7 @@ int aa_change_profile(const char *fqname, int flags)
 		}
 
 		/* full transition will be built in exec path */
-		error = aa_set_current_onexec(target, stack);
+		aa_set_current_onexec(target, stack);
 	}
 
 audit:
diff --git a/security/apparmor/include/task.h b/security/apparmor/include/task.h
index 13437d62c70f..01717fe432c3 100644
--- a/security/apparmor/include/task.h
+++ b/security/apparmor/include/task.h
@@ -30,7 +30,7 @@ struct aa_task_ctx {
 };
 
 int aa_replace_current_label(struct aa_label *label);
-int aa_set_current_onexec(struct aa_label *label, bool stack);
+void aa_set_current_onexec(struct aa_label *label, bool stack);
 int aa_set_current_hat(struct aa_label *label, u64 token);
 int aa_restore_previous_label(u64 cookie);
 struct aa_label *aa_get_task_label(struct task_struct *task);
diff --git a/security/apparmor/task.c b/security/apparmor/task.c
index 84d16a29bfcb..5671a716fcd2 100644
--- a/security/apparmor/task.c
+++ b/security/apparmor/task.c
@@ -93,9 +93,8 @@ int aa_replace_current_label(struct aa_label *label)
  * aa_set_current_onexec - set the tasks change_profile to happen onexec
  * @label: system label to set at exec  (MAYBE NULL to clear value)
  * @stack: whether stacking should be done
- * Returns: 0 or error on failure
  */
-int aa_set_current_onexec(struct aa_label *label, bool stack)
+void aa_set_current_onexec(struct aa_label *label, bool stack)
 {
 	struct aa_task_ctx *ctx = task_ctx(current);
 
@@ -103,8 +102,6 @@ int aa_set_current_onexec(struct aa_label *label, bool stack)
 	aa_put_label(ctx->onexec);
 	ctx->onexec = label;
 	ctx->token = stack;
-
-	return 0;
 }
 
 /**
-- 
2.31.1


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

* Re: [apparmor] [PATCH] apparmor: make aa_set_current_onexec return void
  2023-01-14 16:49 [PATCH] apparmor: make aa_set_current_onexec return void Quanfa Fu
@ 2023-01-18  4:15 ` Tyler Hicks
  2023-01-18  6:45   ` John Johansen
  0 siblings, 1 reply; 3+ messages in thread
From: Tyler Hicks @ 2023-01-18  4:15 UTC (permalink / raw)
  To: Quanfa Fu
  Cc: john.johansen, paul, jmorris, serge, linux-security-module,
	apparmor, linux-kernel

On 2023-01-15 00:49:52, Quanfa Fu wrote:
> Change the return type to void since it always return 0, and no need
> to do the checking in aa_set_current_onexec.
> 
> Signed-off-by: Quanfa Fu <quanfafu@gmail.com>

This looks like a safe change to me. There's nothing to error check
within aa_set_current_onexec() so returning void is fine.

  Reviewed-by: "Tyler Hicks (Microsoft)" <code@tyhicks.com>

Tyler

> ---
>  security/apparmor/domain.c       | 2 +-
>  security/apparmor/include/task.h | 2 +-
>  security/apparmor/task.c         | 5 +----
>  3 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> index 6dd3cc5309bf..bbc9c8a87b8e 100644
> --- a/security/apparmor/domain.c
> +++ b/security/apparmor/domain.c
> @@ -1446,7 +1446,7 @@ int aa_change_profile(const char *fqname, int flags)
>  		}
>  
>  		/* full transition will be built in exec path */
> -		error = aa_set_current_onexec(target, stack);
> +		aa_set_current_onexec(target, stack);
>  	}
>  
>  audit:
> diff --git a/security/apparmor/include/task.h b/security/apparmor/include/task.h
> index 13437d62c70f..01717fe432c3 100644
> --- a/security/apparmor/include/task.h
> +++ b/security/apparmor/include/task.h
> @@ -30,7 +30,7 @@ struct aa_task_ctx {
>  };
>  
>  int aa_replace_current_label(struct aa_label *label);
> -int aa_set_current_onexec(struct aa_label *label, bool stack);
> +void aa_set_current_onexec(struct aa_label *label, bool stack);
>  int aa_set_current_hat(struct aa_label *label, u64 token);
>  int aa_restore_previous_label(u64 cookie);
>  struct aa_label *aa_get_task_label(struct task_struct *task);
> diff --git a/security/apparmor/task.c b/security/apparmor/task.c
> index 84d16a29bfcb..5671a716fcd2 100644
> --- a/security/apparmor/task.c
> +++ b/security/apparmor/task.c
> @@ -93,9 +93,8 @@ int aa_replace_current_label(struct aa_label *label)
>   * aa_set_current_onexec - set the tasks change_profile to happen onexec
>   * @label: system label to set at exec  (MAYBE NULL to clear value)
>   * @stack: whether stacking should be done
> - * Returns: 0 or error on failure
>   */
> -int aa_set_current_onexec(struct aa_label *label, bool stack)
> +void aa_set_current_onexec(struct aa_label *label, bool stack)
>  {
>  	struct aa_task_ctx *ctx = task_ctx(current);
>  
> @@ -103,8 +102,6 @@ int aa_set_current_onexec(struct aa_label *label, bool stack)
>  	aa_put_label(ctx->onexec);
>  	ctx->onexec = label;
>  	ctx->token = stack;
> -
> -	return 0;
>  }
>  
>  /**
> -- 
> 2.31.1
> 
> 

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

* Re: [apparmor] [PATCH] apparmor: make aa_set_current_onexec return void
  2023-01-18  4:15 ` [apparmor] " Tyler Hicks
@ 2023-01-18  6:45   ` John Johansen
  0 siblings, 0 replies; 3+ messages in thread
From: John Johansen @ 2023-01-18  6:45 UTC (permalink / raw)
  To: Tyler Hicks, Quanfa Fu
  Cc: paul, jmorris, serge, linux-security-module, apparmor,
	linux-kernel

On 1/17/23 20:15, Tyler Hicks wrote:
> On 2023-01-15 00:49:52, Quanfa Fu wrote:
>> Change the return type to void since it always return 0, and no need
>> to do the checking in aa_set_current_onexec.
>>
>> Signed-off-by: Quanfa Fu <quanfafu@gmail.com>
> 
> This looks like a safe change to me. There's nothing to error check
> within aa_set_current_onexec() so returning void is fine.
> 
>    Reviewed-by: "Tyler Hicks (Microsoft)" <code@tyhicks.com>
> 

Looks good, I have pulled this into my tree

Acked-by: John Johansen <john.johansen@canonical.com>

> Tyler
> 
>> ---
>>   security/apparmor/domain.c       | 2 +-
>>   security/apparmor/include/task.h | 2 +-
>>   security/apparmor/task.c         | 5 +----
>>   3 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
>> index 6dd3cc5309bf..bbc9c8a87b8e 100644
>> --- a/security/apparmor/domain.c
>> +++ b/security/apparmor/domain.c
>> @@ -1446,7 +1446,7 @@ int aa_change_profile(const char *fqname, int flags)
>>   		}
>>   
>>   		/* full transition will be built in exec path */
>> -		error = aa_set_current_onexec(target, stack);
>> +		aa_set_current_onexec(target, stack);
>>   	}
>>   
>>   audit:
>> diff --git a/security/apparmor/include/task.h b/security/apparmor/include/task.h
>> index 13437d62c70f..01717fe432c3 100644
>> --- a/security/apparmor/include/task.h
>> +++ b/security/apparmor/include/task.h
>> @@ -30,7 +30,7 @@ struct aa_task_ctx {
>>   };
>>   
>>   int aa_replace_current_label(struct aa_label *label);
>> -int aa_set_current_onexec(struct aa_label *label, bool stack);
>> +void aa_set_current_onexec(struct aa_label *label, bool stack);
>>   int aa_set_current_hat(struct aa_label *label, u64 token);
>>   int aa_restore_previous_label(u64 cookie);
>>   struct aa_label *aa_get_task_label(struct task_struct *task);
>> diff --git a/security/apparmor/task.c b/security/apparmor/task.c
>> index 84d16a29bfcb..5671a716fcd2 100644
>> --- a/security/apparmor/task.c
>> +++ b/security/apparmor/task.c
>> @@ -93,9 +93,8 @@ int aa_replace_current_label(struct aa_label *label)
>>    * aa_set_current_onexec - set the tasks change_profile to happen onexec
>>    * @label: system label to set at exec  (MAYBE NULL to clear value)
>>    * @stack: whether stacking should be done
>> - * Returns: 0 or error on failure
>>    */
>> -int aa_set_current_onexec(struct aa_label *label, bool stack)
>> +void aa_set_current_onexec(struct aa_label *label, bool stack)
>>   {
>>   	struct aa_task_ctx *ctx = task_ctx(current);
>>   
>> @@ -103,8 +102,6 @@ int aa_set_current_onexec(struct aa_label *label, bool stack)
>>   	aa_put_label(ctx->onexec);
>>   	ctx->onexec = label;
>>   	ctx->token = stack;
>> -
>> -	return 0;
>>   }
>>   
>>   /**
>> -- 
>> 2.31.1
>>
>>


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

end of thread, other threads:[~2023-01-18  7:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-14 16:49 [PATCH] apparmor: make aa_set_current_onexec return void Quanfa Fu
2023-01-18  4:15 ` [apparmor] " Tyler Hicks
2023-01-18  6:45   ` John Johansen

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