Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> writes:
> Eric W. Biederman wrote:
>> Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> writes:
>> > diff --git a/fs/exec.c b/fs/exec.c
>> > index 8875dd1..89f0479 100644
>> > --- a/fs/exec.c
>> > +++ b/fs/exec.c
>> > @@ -1172,6 +1172,7 @@ void free_bprm(struct linux_binprm *bprm)
>> > {
>> > free_arg_pages(bprm);
>> > if (bprm->cred) {
>> > + security_bprm_aborting_creds(bprm);
>>
>> Can you move this look outside of the cred_guard_mutex? It looks like
>> you can and I expect not unnecessarily extending the scope of the mutex
>> would be a good idea.
>>
>> > mutex_unlock(¤t->signal->cred_guard_mutex);
>> > abort_creds(bprm->cred);
>> > }
>>
>
> Sure. Here is the updated patch. Does this look OK to you?
Yes. I don't know about the semantics of the patch but moving
oustide of the cred_guard_mutex looks good.
Eric
>>From b226aaf96303495a04d615cda6f8a06af3e822c3 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> Date: Fri, 18 Oct 2013 21:32:16 +0900
> Subject: [PATCHv2 1/4] LSM: Add security_bprm_aborting_creds() hook.
>
> Add a LSM hook which is called only when an execve operation failed after
> prepare_bprm_creds() succeeded. This hook is used by TOMOYO for synchronously
> cleaning up resources allocated during an execve operation.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> ---
> fs/exec.c | 1 +
> include/linux/security.h | 11 +++++++++++
> security/capability.c | 5 +++++
> security/security.c | 5 +++++
> 4 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 8875dd1..b229f22 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1173,6 +1173,7 @@ void free_bprm(struct linux_binprm *bprm)
> free_arg_pages(bprm);
> if (bprm->cred) {
> mutex_unlock(¤t->signal->cred_guard_mutex);
> + security_bprm_aborting_creds(bprm);
> abort_creds(bprm->cred);
> }
> /* If a binfmt changed the interp, free it. */
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 9d37e2b..e9fff76 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -236,6 +236,11 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
> * linux_binprm structure. This hook is a good place to perform state
> * changes on the process such as clearing out non-inheritable signal
> * state. This is called immediately after commit_creds().
> + * @bprm_aborting_creds:
> + * This hook is called when an execve operation failed after
> + * prepare_bprm_creds() succeeded so that we can synchronously clean up
> + * resources used by an execve operation.
> + * @bprm points to the linux_binprm structure.
> * @bprm_secureexec:
> * Return a boolean value (0 or 1) indicating whether a "secure exec"
> * is required. The flag is passed in the auxiliary table
> @@ -1446,6 +1451,7 @@ struct security_operations {
> int (*bprm_secureexec) (struct linux_binprm *bprm);
> void (*bprm_committing_creds) (struct linux_binprm *bprm);
> void (*bprm_committed_creds) (struct linux_binprm *bprm);
> + void (*bprm_aborting_creds) (struct linux_binprm *bprm);
>
> int (*sb_alloc_security) (struct super_block *sb);
> void (*sb_free_security) (struct super_block *sb);
> @@ -1741,6 +1747,7 @@ int security_bprm_set_creds(struct linux_binprm *bprm);
> int security_bprm_check(struct linux_binprm *bprm);
> void security_bprm_committing_creds(struct linux_binprm *bprm);
> void security_bprm_committed_creds(struct linux_binprm *bprm);
> +void security_bprm_aborting_creds(struct linux_binprm *bprm);
> int security_bprm_secureexec(struct linux_binprm *bprm);
> int security_sb_alloc(struct super_block *sb);
> void security_sb_free(struct super_block *sb);
> @@ -1988,6 +1995,10 @@ static inline void security_bprm_committed_creds(struct linux_binprm *bprm)
> {
> }
>
> +static inline void security_bprm_aborting_creds(struct linux_binprm *bprm)
> +{
> +}
> +
> static inline int security_bprm_secureexec(struct linux_binprm *bprm)
> {
> return cap_bprm_secureexec(bprm);
> diff --git a/security/capability.c b/security/capability.c
> index dbeb9bc..44ca607 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -40,6 +40,10 @@ static void cap_bprm_committed_creds(struct linux_binprm *bprm)
> {
> }
>
> +static void cap_bprm_aborting_creds(struct linux_binprm *bprm)
> +{
> +}
> +
> static int cap_sb_alloc_security(struct super_block *sb)
> {
> return 0;
> @@ -931,6 +935,7 @@ void __init security_fixup_ops(struct security_operations *ops)
> set_to_cap_if_null(ops, bprm_set_creds);
> set_to_cap_if_null(ops, bprm_committing_creds);
> set_to_cap_if_null(ops, bprm_committed_creds);
> + set_to_cap_if_null(ops, bprm_aborting_creds);
> set_to_cap_if_null(ops, bprm_check_security);
> set_to_cap_if_null(ops, bprm_secureexec);
> set_to_cap_if_null(ops, sb_alloc_security);
> diff --git a/security/security.c b/security/security.c
> index 4dc31f4..064da04 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -236,6 +236,11 @@ void security_bprm_committed_creds(struct linux_binprm *bprm)
> security_ops->bprm_committed_creds(bprm);
> }
>
> +void security_bprm_aborting_creds(struct linux_binprm *bprm)
> +{
> + security_ops->bprm_aborting_creds(bprm);
> +}
> +
> int security_bprm_secureexec(struct linux_binprm *bprm)
> {
> return security_ops->bprm_secureexec(bprm);
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
