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(a)huawei.com&gt; > --- > 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(a)redhat.com > https://www.redhat.com/mailman/listinfo/linux-audit - RGB -- Richard Guy Briggs <rgb(a)redhat.com&gt; 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