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 <[email protected]>
Looks good. Feel free to add: Reviewed-by: Jan Kara <[email protected]> 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 <[email protected]> SUSE Labs, CR
