Re: [RFC][PATCH 10/13] get rid of audit_reusename()

Jan Kara
2025-11-13T10:29:32Z

On Sun 09-11-25 06:37:42, Al Viro wrote:
> Originally we tried to avoid multiple insertions into audit names array
> during retry loop by a cute hack - memorize the userland pointer and
> if there already is a match, just grab an extra reference to it.
>
> Cute as it had been, it had problems - two identical pointers had
> audit aux entries merged, two identical strings did not. Having
> different behaviour for syscalls that differ only by addresses of
> otherwise identical string arguments is obviously wrong - if nothing
> else, compiler can decide to merge identical string literals.
>
> Besides, this hack does nothing for non-audited processes - they get
> a fresh copy for retry. It's not time-critical, but having behaviour
> subtly differ that way is bogus.
>
> These days we have very few places that import filename more than once
> (9 functions total) and it's easy to massage them so we get rid of all
> re-imports. With that done, we don't need audit_reusename() anymore.
> There's no need to memorize userland pointer either.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

Honza

> ---
> fs/namei.c | 11 +++--------
> include/linux/audit.h | 11 -----------
> include/linux/fs.h | 1 -
> kernel/auditsc.c | 23 -----------------------
> 4 files changed, 3 insertions(+), 43 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 7377020a2cba..dd86e41deeeb 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -125,9 +125,8 @@
>
> #define EMBEDDED_NAME_MAX (PATH_MAX - offsetof(struct filename, iname))
>
> -static inline void initname(struct filename *name, const char __user *uptr)
> +static inline void initname(struct filename *name)
> {
> - name->uptr = uptr;
> name->aname = NULL;
> atomic_set(&name->refcnt, 1);
> }
> @@ -139,10 +138,6 @@ getname_flags(const char __user *filename, int flags)
> char *kname;
> int len;
>
> - result = audit_reusename(filename);
> - if (result)
> - return result;
> -
> result = __getname();
> if (unlikely(!result))
> return ERR_PTR(-ENOMEM);
> @@ -210,7 +205,7 @@ getname_flags(const char __user *filename, int flags)
> return ERR_PTR(-ENAMETOOLONG);
> }
> }
> - initname(result, filename);
> + initname(result);
> audit_getname(result);
> return result;
> }
> @@ -268,7 +263,7 @@ struct filename *getname_kernel(const char * filename)
> return ERR_PTR(-ENAMETOOLONG);
> }
> memcpy((char *)result->name, filename, len);
> - initname(result, NULL);
> + initname(result);
> audit_getname(result);
> return result;
> }
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 536f8ee8da81..d936a604d056 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -316,7 +316,6 @@ extern void __audit_uring_exit(int success, long code);
> 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 void __audit_getname(struct filename *name);
> extern void __audit_inode(struct filename *name, const struct dentry *dentry,
> unsigned int flags);
> @@ -380,12 +379,6 @@ 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)
> -{
> - if (unlikely(!audit_dummy_context()))
> - return __audit_reusename(name);
> - return NULL;
> -}
> static inline void audit_getname(struct filename *name)
> {
> if (unlikely(!audit_dummy_context()))
> @@ -624,10 +617,6 @@ static inline struct audit_context *audit_context(void)
> {
> return NULL;
> }
> -static inline struct filename *audit_reusename(const __user char *name)
> -{
> - return NULL;
> -}
> static inline void audit_getname(struct filename *name)
> { }
> static inline void audit_inode(struct filename *name,
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index c895146c1444..bbae3cfdc338 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2835,7 +2835,6 @@ extern struct kobject *fs_kobj;
> struct audit_names;
> struct filename {
> const char *name; /* pointer to actual string */
> - const __user char *uptr; /* original userland pointer */
> atomic_t refcnt;
> struct audit_names *aname;
> const char iname[];
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index d1966144bdfe..e59a094bb9f7 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2169,29 +2169,6 @@ static struct audit_names *audit_alloc_name(struct audit_context *context,
> return aname;
> }
>
> -/**
> - * __audit_reusename - fill out filename with info from existing entry
> - * @uptr: userland ptr to pathname
> - *
> - * Search the audit_names list for the current audit context. If there is an
> - * existing entry with a matching "uptr" then return the filename
> - * associated with that audit_name. If not, return NULL.
> - */
> -struct filename *
> -__audit_reusename(const __user char *uptr)
> -{
> - struct audit_context *context = audit_context();
> - struct audit_names *n;
> -
> - list_for_each_entry(n, &context->names_list, list) {
> - if (!n->name)
> - continue;
> - if (n->name->uptr == uptr)
> - return refname(n->name);
> - }
> - return NULL;
> -}
> -
> /**
> * __audit_getname - add a name to the list
> * @name: name to add
> --
> 2.47.3
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR