Re: [PATCH 3/3] tomoyo: robustify handling of mm->exe_file

Tetsuo Handa
2015-02-24T11:35:37Z

Davidlohr Bueso wrote:
> On Fri, 2015-02-20 at 07:11 +0900, Tetsuo Handa wrote:
> > Davidlohr Bueso wrote:
> > > On Thu, 2015-02-19 at 20:07 +0900, Tetsuo Handa wrote:
> > > > Why do we need to let the caller call path_put() ?
> > > > There is no need to do like proc_exe_link() does, for
> > > > tomoyo_get_exe() returns pathname as "char *".
> > >
> > > Having the pathname doesn't guarantee anything later, and thus doesn't
> > > seem very robust in the manager call if it can be dropped during the
> > > call... or can this never occur in this context?
> > >
> > tomoyo_get_exe() returns the pathname of executable of current thread.
> > The executable of current thread cannot be changed while current thread
> > is inside the manager call. Although the pathname of executable of
> > current thread could be changed by other threads via namespace manipulation
> > like pivot_root(), holding a reference guarantees nothing. Your patch helps
> > for avoiding memory allocation with mmap_sem held, but does not robustify
> > handling of mm->exe_file for tomoyo.
>
> Fair enough, I won't argue. This is beyond the scope if what I'm trying
> to accomplish here anyway. Are you ok with this instead?
>
I prefer cleanups excluded from this patch, like shown below.
Would you please resend?

> diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
> index e0fb750..73ce629 100644
> --- a/security/tomoyo/common.c
> +++ b/security/tomoyo/common.c
> @@ -908,6 +908,31 @@ static void tomoyo_read_manager(struct tomoyo_io_buffer *head)
> }
>
> /**
> + * tomoyo_get_exe - Get tomoyo_realpath() of current process.
> + *
> + * Returns the tomoyo_realpath() of current process on success, NULL otherwise.
> + *
> + * This function uses kzalloc(), so the caller must call kfree()
> + * if this function didn't return NULL.
> + */
> +static const char *tomoyo_get_exe(void)
> +{
> + struct file *exe_file;
> + const char *cp = NULL;

No need to initialize cp here.

> + struct mm_struct *mm = current->mm;
> +
> + if (!mm)
> + return NULL;
> + exe_file = get_mm_exe_file(mm);
> + if (!exe_file)
> + return NULL;
> +
> + cp = tomoyo_realpath_from_path(&exe_file->f_path);
> + fput(exe_file);
> + return cp;
> +}
> +
> +/**
> * tomoyo_manager - Check whether the current process is a policy manager.
> *
> * Returns true if the current process is permitted to modify policy
> diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h
> index b897d48..fc89eba 100644
> --- a/security/tomoyo/common.h
> +++ b/security/tomoyo/common.h
> @@ -947,7 +947,6 @@ char *tomoyo_init_log(struct tomoyo_request_info *r, int len, const char *fmt,
> char *tomoyo_read_token(struct tomoyo_acl_param *param);
> char *tomoyo_realpath_from_path(struct path *path);
> char *tomoyo_realpath_nofollow(const char *pathname);
> -const char *tomoyo_get_exe(void);
> const char *tomoyo_yesno(const unsigned int value);
> const struct tomoyo_path_info *tomoyo_compare_name_union
> (const struct tomoyo_path_info *name, const struct tomoyo_name_union *ptr);
> diff --git a/security/tomoyo/util.c b/security/tomoyo/util.c
> index 2952ba5..7eff479 100644
> --- a/security/tomoyo/util.c
> +++ b/security/tomoyo/util.c
> @@ -939,28 +939,6 @@ bool tomoyo_path_matches_pattern(const struct tomoyo_path_info *filename,
> }
>
> /**
> - * tomoyo_get_exe - Get tomoyo_realpath() of current process.
> - *
> - * Returns the tomoyo_realpath() of current process on success, NULL otherwise.
> - *
> - * This function uses kzalloc(), so the caller must call kfree()
> - * if this function didn't return NULL.
> - */
> -const char *tomoyo_get_exe(void)
> -{
> - struct mm_struct *mm = current->mm;
> - const char *cp = NULL;
> -
> - if (!mm)
> - return NULL;
> - down_read(&mm->mmap_sem);
> - if (mm->exe_file)
> - cp = tomoyo_realpath_from_path(&mm->exe_file->f_path);
> - up_read(&mm->mmap_sem);
> - return cp;
> -}
> -
> -/**
> * tomoyo_get_mode - Get MAC mode.
> *
> * @ns: Pointer to "struct tomoyo_policy_namespace".
> --
> 2.1.4

You can post TOMOYO patches to tomoyo-dev-en@lists.sourceforge.jp .
I'll add your mail address to ML's accept list.