On Fri, Feb 14, 2025 at 10:28:22AM +0100, Benjamin Berg wrote:
> From: Benjamin Berg <benjamin.b...@intel.com>
> 
> 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.b...@intel.com>

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-...@digikod.net/
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 <m...@digikod.net>
Tested-by: Mickaël Salaün <m...@digikod.net>

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

Reply via email to