linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Mateusz Guzik <mjguzik@gmail.com>
Cc: Xi Ruoyao <xry111@xry111.site>,
	Christian Brauner <brauner@kernel.org>,
	Miao Wang <shankerwangmiao@gmail.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH 2/2] vfs: Make sure {statx,fstatat}(..., AT_EMPTY_PATH | ..., NULL, ...) behave as (..., AT_EMPTY_PATH | ..., "", ...)
Date: Tue, 8 Oct 2024 05:27:51 +0100	[thread overview]
Message-ID: <20241008042751.GW4017910@ZenIV> (raw)
In-Reply-To: <20241008041621.GV4017910@ZenIV>

On Tue, Oct 08, 2024 at 05:16:21AM +0100, Al Viro wrote:

> Folks, please don't go there.  Really.  IMO vfs_empty_path() is a wrong API
> in the first place.  Too low-level and racy as well.
> 
> 	See the approach in #work.xattr; I'm going to lift that into fs/namei.c
> (well, the slow path - everything after "if path is NULL, we are done") and
> yes, fs/stat.c users get handled better that way.

FWIW, the intermediate (just after that commit) state of those functions is

int vfs_fstatat(int dfd, const char __user *filename,
                              struct kstat *stat, int flags)
{
        int ret;
        int statx_flags = flags | AT_NO_AUTOMOUNT;
        struct filename *name = getname_maybe_null(filename, flags);

        if (!name)
                return vfs_fstat(dfd, stat);

        ret = vfs_statx(dfd, name, statx_flags, stat, STATX_BASIC_STATS);
        putname(name); 

        return ret;  
}

and

SYSCALL_DEFINE5(statx,
                int, dfd, const char __user *, filename, unsigned, flags,
                unsigned int, mask,
                struct statx __user *, buffer)
{
        int ret;
        unsigned lflags;
        struct filename *name = getname_maybe_null(filename, flags);

        /*
         * Short-circuit handling of NULL and "" paths.
         *
         * For a NULL path we require and accept only the AT_EMPTY_PATH flag
         * (possibly |'d with AT_STATX flags).
         *
         * However, glibc on 32-bit architectures implements fstatat as statx
         * with the "" pathname and AT_NO_AUTOMOUNT | AT_EMPTY_PATH flags.
         * Supporting this results in the uglification below.
         */
        lflags = flags & ~(AT_NO_AUTOMOUNT | AT_STATX_SYNC_TYPE);
        if (!name)
                return do_statx_fd(dfd, flags & ~AT_NO_AUTOMOUNT, mask, buffer);

        ret = do_statx(dfd, name, flags, mask, buffer);
        putname(name);

        return ret;
}

static inline struct filename *getname_maybe_null(const char __user *name, int flags)
{
        if (!(flags & AT_EMPTY_PATH))
                return getname(name);

        if (!name)
                return NULL;
        return __getname_maybe_null(name);
}

struct filename *__getname_maybe_null(const char __user *pathname)
{
        struct filename *name;
        char c;

        /* try to save on allocations; loss on um, though */
        if (get_user(c, pathname))
                return ERR_PTR(-EFAULT);
        if (!c)
                return NULL;

        name = getname_flags(pathname, LOOKUP_EMPTY);
        if (!IS_ERR(name) && !(name->name[0])) {
                putname(name);
                name = NULL;
        }
        return name;   
}

  reply	other threads:[~2024-10-08  4:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-07 13:08 [PATCH 0/2] vfs: fstatat, statx: Consistently accept AT_EMPTY_PATH and NULL path Xi Ruoyao
2024-10-07 13:08 ` [PATCH 1/2] vfs: support fstatat(..., NULL, AT_EMPTY_PATH | AT_NO_AUTOMOUNT, ...) Xi Ruoyao
2024-10-07 13:08 ` [PATCH 2/2] vfs: Make sure {statx,fstatat}(..., AT_EMPTY_PATH | ..., NULL, ...) behave as (..., AT_EMPTY_PATH | ..., "", ...) Xi Ruoyao
2024-10-08  3:57   ` Mateusz Guzik
2024-10-08  4:16     ` Al Viro
2024-10-08  4:27       ` Al Viro [this message]
2024-10-08  4:52         ` Al Viro
2024-10-19  9:31         ` Xi Ruoyao

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=20241008042751.GW4017910@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=brauner@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjguzik@gmail.com \
    --cc=shankerwangmiao@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=xry111@xry111.site \
    /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;
as well as URLs for NNTP newsgroup(s).