2:21 a.m.
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?
2:21 a.m.
New subject: [PATCH] audit: allow audit_reusename to check kernel path
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(a)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
7:32 a.m.
On Mon, May 25, 2020 at 3:22 AM Yiwen Gu <guyiwen(a)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
7:36 p.m.
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(a)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?
8:23 a.m.
On Tue, May 26, 2020 at 8:37 PM Alexander Viro <aviro(a)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(a)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-guy...
> What kernel are you using? Is this an Android kernel?
>
> Do you have a reproducer you can share?
--
paul moore
www.paul-moore.com
1669
days inactive
1671
days old
linux-audit@lists.linux-audit.osci.io
4 comments
3 participants
participants (3)
-
Alexander Viro -
Paul Moore -
Yiwen Gu