* [RFC] audit: allow audit_reusename to check kernel path @ 2020-05-25 7:21 Yiwen Gu 2020-05-25 7:21 ` [PATCH] " Yiwen Gu 2020-05-26 12:32 ` [RFC] " Paul Moore 0 siblings, 2 replies; 5+ messages in thread From: Yiwen Gu @ 2020-05-25 7:21 UTC (permalink / raw) To: paul, eparis; +Cc: linux-audit For now, we met a situation where the audit_reusename checking function returns the same filename structure for files sharing the same uptr. However, these files are different, and we are trying to open them in a loop where the names are loaded into the same address. Therefore, the function returns the same structure for different files. By the way, may I ask in what situation would the audit_list be kept across syscalls? -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] audit: allow audit_reusename to check kernel path 2020-05-25 7:21 [RFC] audit: allow audit_reusename to check kernel path Yiwen Gu @ 2020-05-25 7:21 ` Yiwen Gu 2020-05-26 12:32 ` [RFC] " Paul Moore 1 sibling, 0 replies; 5+ messages in thread From: Yiwen Gu @ 2020-05-25 7:21 UTC (permalink / raw) To: paul, eparis; +Cc: linux-audit 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. Add kname into audit_reusename function to check file names from the audit_names list. Signed-off-by: Yiwen Gu <guyiwen@huawei.com> --- fs/namei.c | 11 +++++++---- include/linux/audit.h | 11 +++++++---- kernel/auditsc.c | 7 ++++--- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index db6565c99825..c5623858f3e2 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,12 @@ getname_flags(const char __user *filename, int flags, int *empty) } } + result_audit = audit_reusename(filename, kname); + if (result_audit) { + putname(result); + 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@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC] audit: allow audit_reusename to check kernel path 2020-05-25 7:21 [RFC] audit: allow audit_reusename to check kernel path Yiwen Gu 2020-05-25 7:21 ` [PATCH] " Yiwen Gu @ 2020-05-26 12:32 ` Paul Moore 2020-05-27 0:36 ` Alexander Viro 1 sibling, 1 reply; 5+ messages in thread From: Paul Moore @ 2020-05-26 12:32 UTC (permalink / raw) To: Yiwen Gu; +Cc: linux-audit On Mon, May 25, 2020 at 3:22 AM Yiwen Gu <guyiwen@huawei.com> wrote: > For now, we met a situation where the audit_reusename checking > function returns the same filename structure for files sharing > the same uptr. However, these files are different, and we are trying > to open them in a loop where the names are loaded into the same address. > Therefore, the function returns the same structure for different files. > By the way, may I ask in what situation would the audit_list be kept > across syscalls? What kernel are you using? Is this an Android kernel? Do you have a reproducer you can share? -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] audit: allow audit_reusename to check kernel path 2020-05-26 12:32 ` [RFC] " Paul Moore @ 2020-05-27 0:36 ` Alexander Viro 2020-05-27 13:23 ` Paul Moore 0 siblings, 1 reply; 5+ messages in thread From: Alexander Viro @ 2020-05-27 0:36 UTC (permalink / raw) To: Paul Moore; +Cc: linux-audit On Tue, May 26, 2020 at 08:32:06AM -0400, Paul Moore wrote: > On Mon, May 25, 2020 at 3:22 AM Yiwen Gu <guyiwen@huawei.com> wrote: > > For now, we met a situation where the audit_reusename checking > > function returns the same filename structure for files sharing > > the same uptr. However, these files are different, and we are trying > > to open them in a loop where the names are loaded into the same address. > > Therefore, the function returns the same structure for different files. > > By the way, may I ask in what situation would the audit_list be kept > > across syscalls? Never. "reuse" is strictly within the same syscall, so e.g. -ESTALE retry logics doesn't have to worry about extra instances of struct filename. > What kernel are you using? Is this an Android kernel? > > Do you have a reproducer you can share? -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] audit: allow audit_reusename to check kernel path 2020-05-27 0:36 ` Alexander Viro @ 2020-05-27 13:23 ` Paul Moore 0 siblings, 0 replies; 5+ messages in thread From: Paul Moore @ 2020-05-27 13:23 UTC (permalink / raw) To: Alexander Viro; +Cc: linux-audit On Tue, May 26, 2020 at 8:37 PM Alexander Viro <aviro@redhat.com> wrote: > On Tue, May 26, 2020 at 08:32:06AM -0400, Paul Moore wrote: > > On Mon, May 25, 2020 at 3:22 AM Yiwen Gu <guyiwen@huawei.com> wrote: > > > For now, we met a situation where the audit_reusename checking > > > function returns the same filename structure for files sharing > > > the same uptr. However, these files are different, and we are trying > > > to open them in a loop where the names are loaded into the same address. > > > Therefore, the function returns the same structure for different files. > > > By the way, may I ask in what situation would the audit_list be kept > > > across syscalls? > > Never. "reuse" is strictly within the same syscall, so e.g. -ESTALE > retry logics doesn't have to worry about extra instances of struct > filename. I think there is something odd with the kernel that Yiwen Gu is running; they posted a similar patch back in April (link below) and we talked about it then. The patch didn't make sense in April and it still doesn't make sense to me now. If they can provide more information and a reproducer that works on a kernel from Linus' tree we can take a look, but as things currently stand I think this may be due to a wonky Android/Huawei kernel. * https://lore.kernel.org/linux-audit/1587536907-63272-1-git-send-email-guyiwen@huawei.com > > What kernel are you using? Is this an Android kernel? > > > > Do you have a reproducer you can share? -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-27 13:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-25 7:21 [RFC] audit: allow audit_reusename to check kernel path Yiwen Gu 2020-05-25 7:21 ` [PATCH] " Yiwen Gu 2020-05-26 12:32 ` [RFC] " Paul Moore 2020-05-27 0:36 ` Alexander Viro 2020-05-27 13:23 ` Paul Moore
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).