[PATCH] audit: allow audit_reusename to check kernel path

[Richard Guy Briggs]

On 2020-04-22 14:28, Yiwen Gu wrote:
> Currently, audit_reusename check file path only by comparing
> userspace pointer "uptr", without checking the situation where
> the file name is different with the same uptr.

Has this been found to actually be a problem?  If so, is there a
reproducer to demonstrate this?

The reason it was originally set up like this was to make the check as
efficient as possible, comparing only a pointer, rather than needing to
compare the entire string.

> Add kname into audit_reusename function to check file names
> from the audit_names list.
> 
> Signed-off-by: Yiwen Gu <guyiwen at huawei.com>
> ---
>  fs/namei.c            |  9 +++++----
>  include/linux/audit.h | 11 +++++++----
>  kernel/auditsc.c      |  7 ++++---
>  3 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index db6565c99825..d5fb4bd12aec 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -128,13 +128,10 @@ struct filename *
>  getname_flags(const char __user *filename, int flags, int *empty)
>  {
>  	struct filename *result;
> +	struct filename *result_audit;
>  	char *kname;
>  	int len;
>  
> -	result = audit_reusename(filename);
> -	if (result)
> -		return result;
> -
>  	result = __getname();
>  	if (unlikely(!result))
>  		return ERR_PTR(-ENOMEM);
> @@ -197,6 +194,10 @@ getname_flags(const char __user *filename, int flags, int *empty)
>  		}
>  	}
>  
> +	result_audit = audit_reusename(filename, kname);
> +	if (result_audit)
> +		return result_audit;
> +
>  	result->uptr = filename;
>  	result->aname = NULL;
>  	audit_getname(result);
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index f9ceae57ca8d..71fb783f14c4 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -266,7 +266,8 @@ extern void __audit_free(struct task_struct *task);
>  extern void __audit_syscall_entry(int major, unsigned long a0, unsigned long a1,
>  				  unsigned long a2, unsigned long a3);
>  extern void __audit_syscall_exit(int ret_success, long ret_value);
> -extern struct filename *__audit_reusename(const __user char *uptr);
> +extern struct filename *__audit_reusename(const __user char *uptr,
> +				const char *kname);
>  extern void __audit_getname(struct filename *name);
>  
>  extern void __audit_inode(struct filename *name, const struct dentry *dentry,
> @@ -316,10 +317,11 @@ static inline void audit_syscall_exit(void *pt_regs)
>  		__audit_syscall_exit(success, return_code);
>  	}
>  }
> -static inline struct filename *audit_reusename(const __user char *name)
> +static inline struct filename *audit_reusename(const __user char *name,
> +						const char *kname)
>  {
>  	if (unlikely(!audit_dummy_context()))
> -		return __audit_reusename(name);
> +		return __audit_reusename(name, kname);
>  	return NULL;
>  }
>  static inline void audit_getname(struct filename *name)
> @@ -539,7 +541,8 @@ static inline struct audit_context *audit_context(void)
>  {
>  	return NULL;
>  }
> -static inline struct filename *audit_reusename(const __user char *name)
> +static inline struct filename *audit_reusename(const __user char *name,
> +						const char *kname)
>  {
>  	return NULL;
>  }
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 4effe01ebbe2..62ffc02abb98 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1843,13 +1843,14 @@ static struct audit_names *audit_alloc_name(struct audit_context *context,
>  /**
>   * __audit_reusename - fill out filename with info from existing entry
>   * @uptr: userland ptr to pathname
> + * @kname: kernel pathname string
>   *
>   * Search the audit_names list for the current audit context. If there is an
> - * existing entry with a matching "uptr" then return the filename
> + * existing entry with matching "uptr" and "kname" then return the filename
>   * associated with that audit_name. If not, return NULL.
>   */
>  struct filename *
> -__audit_reusename(const __user char *uptr)
> +__audit_reusename(const __user char *uptr, const char *kname)
>  {
>  	struct audit_context *context = audit_context();
>  	struct audit_names *n;
> @@ -1857,7 +1858,7 @@ __audit_reusename(const __user char *uptr)
>  	list_for_each_entry(n, &context->names_list, list) {
>  		if (!n->name)
>  			continue;
> -		if (n->name->uptr == uptr) {
> +		if (n->name->uptr == uptr && !strcmp(kname, n->name->name)) {
>  			n->name->refcnt++;
>  			return n->name;
>  		}
> -- 
> 2.17.1
> 
> 
> 
> --
> Linux-audit mailing list
> Linux-audit at redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb at redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635