On Fri, Feb 14, 2025 at 10:28:22AM +0100, Benjamin Berg wrote:
> From: Benjamin Berg <benjamin.berg@xxxxxxxxx>
>
> Some file systems (e.g. ext4) may reuse inode numbers once the inode is
> not in use anymore. Usually hostfs will keep an FD open for each inode,
> but this is not always the case. In the case of sockets, this cannot
> even be done properly.
>
> As such, the following sequence of events was possible:
> * application creates and deletes a socket
> * hostfs creates/deletes the socket on the host
> * inode is still in the hostfs cache
> * hostfs creates a new file
> * ext4 on the outside reuses the inode number
> * hostfs finds the socket inode for the newly created file
> * application receives -ENXIO when opening the file
>
> As mentioned, this can only happen if the deleted file is a special file
> that is never opened on the host (i.e. no .open fop).
>
> As such, to prevent issues, it is sufficient to check that the inode
> has the expected type. That said, also add a check for the inode birth
> time, just to be on the safe side.
>
> Fixes: 74ce793bcbde ("hostfs: Fix ephemeral inodes")
> Signed-off-by: Benjamin Berg <benjamin.berg@xxxxxxxxx>
Thanks! This indeed fix an issue that is at least visible when running
Landlock kselftests with an UML kernel built with Landlock audit support
(probably because of a race condition now being more consistent):
https://lore.kernel.org/all/20250308184422.2159360-1-mic@xxxxxxxxxxx/
FYI, I plan to merge this patch series with v6.15
I guess this patch should fix some other use of UML anyway. Please
merge it, if possible before v6.15 .
Reviewed-by: Mickaël Salaün <mic@xxxxxxxxxxx>
Tested-by: Mickaël Salaün <mic@xxxxxxxxxxx>
> ---
> fs/hostfs/hostfs.h | 2 +-
> fs/hostfs/hostfs_kern.c | 7 ++++-
> fs/hostfs/hostfs_user.c | 59 ++++++++++++++++++++++++-----------------
> 3 files changed, 41 insertions(+), 27 deletions(-)
>
> diff --git a/fs/hostfs/hostfs.h b/fs/hostfs/hostfs.h
> index 8b39c15c408c..15b2f094d36e 100644
> --- a/fs/hostfs/hostfs.h
> +++ b/fs/hostfs/hostfs.h
> @@ -60,7 +60,7 @@ struct hostfs_stat {
> unsigned int uid;
> unsigned int gid;
> unsigned long long size;
> - struct hostfs_timespec atime, mtime, ctime;
> + struct hostfs_timespec atime, mtime, ctime, btime;
> unsigned int blksize;
> unsigned long long blocks;
> struct {
> diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
> index e0741e468956..e6e247235728 100644
> --- a/fs/hostfs/hostfs_kern.c
> +++ b/fs/hostfs/hostfs_kern.c
> @@ -33,6 +33,7 @@ struct hostfs_inode_info {
> struct inode vfs_inode;
> struct mutex open_mutex;
> dev_t dev;
> + struct hostfs_timespec btime;
> };
>
> static inline struct hostfs_inode_info *HOSTFS_I(struct inode *inode)
> @@ -547,6 +548,7 @@ static int hostfs_inode_set(struct inode *ino, void *data)
> }
>
> HOSTFS_I(ino)->dev = dev;
> + HOSTFS_I(ino)->btime = st->btime;
> ino->i_ino = st->ino;
> ino->i_mode = st->mode;
> return hostfs_inode_update(ino, st);
> @@ -557,7 +559,10 @@ static int hostfs_inode_test(struct inode *inode, void *data)
> const struct hostfs_stat *st = data;
> dev_t dev = MKDEV(st->dev.maj, st->dev.min);
>
> - return inode->i_ino == st->ino && HOSTFS_I(inode)->dev == dev;
> + return inode->i_ino == st->ino && HOSTFS_I(inode)->dev == dev &&
> + (inode->i_mode & S_IFMT) == (st->mode & S_IFMT) &&
> + HOSTFS_I(inode)->btime.tv_sec == st->btime.tv_sec &&
> + HOSTFS_I(inode)->btime.tv_nsec == st->btime.tv_nsec;
> }
>
> static struct inode *hostfs_iget(struct super_block *sb, char *name)
> diff --git a/fs/hostfs/hostfs_user.c b/fs/hostfs/hostfs_user.c
> index 97e9c40a9448..3bcd9f35e70b 100644
> --- a/fs/hostfs/hostfs_user.c
> +++ b/fs/hostfs/hostfs_user.c
> @@ -18,39 +18,48 @@
> #include "hostfs.h"
> #include <utime.h>
>
> -static void stat64_to_hostfs(const struct stat64 *buf, struct hostfs_stat *p)
> +static void statx_to_hostfs(const struct statx *buf, struct hostfs_stat *p)
> {
> - p->ino = buf->st_ino;
> - p->mode = buf->st_mode;
> - p->nlink = buf->st_nlink;
> - p->uid = buf->st_uid;
> - p->gid = buf->st_gid;
> - p->size = buf->st_size;
> - p->atime.tv_sec = buf->st_atime;
> - p->atime.tv_nsec = 0;
> - p->ctime.tv_sec = buf->st_ctime;
> - p->ctime.tv_nsec = 0;
> - p->mtime.tv_sec = buf->st_mtime;
> - p->mtime.tv_nsec = 0;
> - p->blksize = buf->st_blksize;
> - p->blocks = buf->st_blocks;
> - p->rdev.maj = os_major(buf->st_rdev);
> - p->rdev.min = os_minor(buf->st_rdev);
> - p->dev.maj = os_major(buf->st_dev);
> - p->dev.min = os_minor(buf->st_dev);
> + p->ino = buf->stx_ino;
> + p->mode = buf->stx_mode;
> + p->nlink = buf->stx_nlink;
> + p->uid = buf->stx_uid;
> + p->gid = buf->stx_gid;
> + p->size = buf->stx_size;
> + p->atime.tv_sec = buf->stx_atime.tv_sec;
> + p->atime.tv_nsec = buf->stx_atime.tv_nsec;
> + p->ctime.tv_sec = buf->stx_ctime.tv_sec;
> + p->ctime.tv_nsec = buf->stx_ctime.tv_nsec;
> + p->mtime.tv_sec = buf->stx_mtime.tv_sec;
> + p->mtime.tv_nsec = buf->stx_mtime.tv_nsec;
> + if (buf->stx_mask & STATX_BTIME) {
> + p->btime.tv_sec = buf->stx_btime.tv_sec;
> + p->btime.tv_nsec = buf->stx_btime.tv_nsec;
> + } else {
> + memset(&p->btime, 0, sizeof(p->btime));
> + }
> + p->blksize = buf->stx_blksize;
> + p->blocks = buf->stx_blocks;
> + p->rdev.maj = buf->stx_rdev_major;
> + p->rdev.min = buf->stx_rdev_minor;
> + p->dev.maj = buf->stx_dev_major;
> + p->dev.min = buf->stx_dev_minor;
> }
>
> int stat_file(const char *path, struct hostfs_stat *p, int fd)
> {
> - struct stat64 buf;
> + struct statx buf;
> + int flags = AT_SYMLINK_NOFOLLOW;
>
> if (fd >= 0) {
> - if (fstat64(fd, &buf) < 0)
> - return -errno;
> - } else if (lstat64(path, &buf) < 0) {
> - return -errno;
> + flags |= AT_EMPTY_PATH;
> + path = "";
> }
> - stat64_to_hostfs(&buf, p);
> +
> + if ((statx(fd, path, flags, STATX_BASIC_STATS | STATX_BTIME, &buf)) < 0)
> + return -errno;
> +
> + statx_to_hostfs(&buf, p);
> return 0;
> }
>
> --
> 2.48.1
>
