public inbox for io-uring@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org, brauner@kernel.org, jack@suse.cz,
	mjguzik@gmail.com, paul@paul-moore.com, axboe@kernel.dk,
	audit@vger.kernel.org, io-uring@vger.kernel.org
Subject: Re: [RFC][PATCH 10/13] get rid of audit_reusename()
Date: Mon, 10 Nov 2025 06:36:58 +0000	[thread overview]
Message-ID: <20251110063658.GL2441659@ZenIV> (raw)
In-Reply-To: <CAHk-=wjA=iXRyu1-ABST43vdT60Md9zpQDJ4Kg14V3f_2Bf+BA@mail.gmail.com>

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...

  parent reply	other threads:[~2025-11-10  6:37 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-09  6:37 [RFC][PATCH 00/13] io_uring, struct filename and audit Al Viro
2025-11-09  6:37 ` [RFC][PATCH 01/13] do_faccessat(): import pathname only once Al Viro
2025-11-13 10:11   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 02/13] do_fchmodat(): " Al Viro
2025-11-13 10:12   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 03/13] do_fchownat(): " Al Viro
2025-11-13 10:13   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 04/13] do_utimes_path(): " Al Viro
2025-11-13 10:15   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 05/13] chdir(2): " Al Viro
2025-11-13 10:16   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 06/13] chroot(2): " Al Viro
2025-11-13 10:18   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 07/13] user_statfs(): " Al Viro
2025-11-13 10:18   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 08/13] do_sys_truncate(): " Al Viro
2025-11-13 10:18   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 09/13] do_readlinkat(): " Al Viro
2025-11-13 10:20   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 10/13] get rid of audit_reusename() Al Viro
2025-11-09 19:18   ` Linus Torvalds
2025-11-09 19:55     ` Mateusz Guzik
2025-11-09 20:22       ` Linus Torvalds
2025-11-09 22:18         ` Mateusz Guzik
2025-11-09 22:29           ` Linus Torvalds
2025-11-09 22:33             ` Mateusz Guzik
2025-11-09 22:39               ` Mateusz Guzik
2025-11-09 22:41               ` Linus Torvalds
2025-11-09 22:44                 ` Linus Torvalds
2025-11-09 23:07                   ` Linus Torvalds
2025-11-09 22:18     ` Linus Torvalds
2025-11-10  5:17       ` Al Viro
2025-11-10 16:41         ` Linus Torvalds
2025-11-10 19:58           ` Al Viro
2025-11-10 20:52             ` Linus Torvalds
2025-11-11  1:16               ` Al Viro
2025-11-12  9:26           ` Christian Brauner
2025-11-10  6:05       ` Al Viro
2025-11-10  6:36       ` Al Viro [this message]
2025-11-10 16:50         ` Linus Torvalds
2025-11-10 23:13   ` Paul Moore
2025-11-11  0:23     ` Paul Moore
2025-11-13 10:29   ` Jan Kara
2025-11-09  6:37 ` [RFC][PATCH 11/13] allow incomplete imports of filenames Al Viro
2025-11-11  0:45   ` Paul Moore
2025-11-11 14:41   ` Jens Axboe
2025-11-19  1:12     ` Al Viro
2025-11-19  1:14       ` Al Viro
2025-11-19  5:41         ` Al Viro
2025-11-09  6:37 ` [RFC][PATCH 12/13] fs: touch up predicts in putname() Al Viro
2025-11-09  6:37 ` [RFC][PATCH 13/13] struct filename ->refcnt doesn't need to be atomic Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251110063658.GL2441659@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=audit@vger.kernel.org \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=io-uring@vger.kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mjguzik@gmail.com \
    --cc=paul@paul-moore.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox