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

Al Viro
2025-11-10T06:37:00Z

On Sun, Nov 09, 2025 at 02:18:04PM -0800, Linus Torvalds wrote:

> @@ -143,7 +162,7 @@ getname_flags(const char __user *filename, int flags)

> - const size_t size = offsetof(struct filename, iname[1]);
> - kname = (char *)result;
> -
> - /*
> - * size is chosen that way we to guarantee that
> - * result->iname[0] is within the same object and that
> - * kname can't be equal to result->iname, no matter what.
> - */
> - result = kzalloc(size, GFP_KERNEL);
> - if (unlikely(!result)) {
> - __putname(kname);
> + kname = kmalloc(PATH_MAX, GFP_KERNEL);
> + if (unlikely(!kname)) {
> + free_filename(result);
> return ERR_PTR(-ENOMEM);
> }
> - result->name = kname;
> - len = strncpy_from_user(kname, filename, PATH_MAX);
> + memcpy(kname, result->iname, EMBEDDED_NAME_MAX);
> +
> + // Copy remaining part of the name
> + len = strncpy_from_user(kname + EMBEDDED_NAME_MAX,
> + filename + EMBEDDED_NAME_MAX,
> + PATH_MAX-EMBEDDED_NAME_MAX);
> + if (unlikely(len == PATH_MAX-EMBEDDED_NAME_MAX))
> + len = -ENAMETOOLONG;
> if (unlikely(len < 0)) {
> - __putname(kname);
> - kfree(result);
> + free_filename(result);
> + kfree(kname);
> return ERR_PTR(len);
> }
> - /* The empty path is special. */
> - if (unlikely(!len) && !(flags & LOOKUP_EMPTY)) {
> - __putname(kname);
> - kfree(result);
> - return ERR_PTR(-ENOENT);
> - }
> - if (unlikely(len == PATH_MAX)) {
> - __putname(kname);
> - kfree(result);
> - return ERR_PTR(-ENAMETOOLONG);
> - }
> + result->name = kname;

FWIW, I would take that part into an out-of-line helper - it does
not need flags and it doesn't really need to bother with
free_filename() on failure - that can be left to the caller,
where we would have
len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
if (unlikely(len == 0) && !(flags & LOOKUP_EMPTY))
len = -ENOENT;
if (unlikely(len == EMBEDDED_NAME_MAX))
len = getname_long_name(result, filename);
if (unlikely(len < 0)) {
free_filename(result);
return ERR_PTR(len);
}
// we are fine, finish setting it up
...
return result;

> @@ -246,7 +252,7 @@ struct filename *getname_kernel(const char * filename)
> struct filename *result;
> int len = strlen(filename) + 1;
>
> - result = __getname();
> + result = alloc_filename();
> if (unlikely(!result))
> return ERR_PTR(-ENOMEM);
>
> @@ -258,13 +264,13 @@ struct filename *getname_kernel(const char * filename)
>
> tmp = kmalloc(size, GFP_KERNEL);
> if (unlikely(!tmp)) {
> - __putname(result);
> + free_filename(result);
> return ERR_PTR(-ENOMEM);
> }
> tmp->name = (char *)result;
> result = tmp;

That's wrong - putname() will choke on that (free_filename() on result of
kmalloc()). Something like
if (len <= EMBEDDED_NAME_MAX) {
result->name = (char *)result->iname;
memcpy(result->name, filename, len);
} else if (len <= PATH_MAX) {
result->name = kmemdup(filename, len);
if (unlikely(!result->name))
len = -ENOMEM;
} else {
len = -ENAMETOOLONG;
}
if (unlikely(len < 0) {
free_filename(result);
return ERR_PTR(len);
}
// finish setting up
...
return result;
perhaps?

And if we are going for that kind of changes, I would rather take
that stuff to fs/filename.c, TBH...