Re: [PATCH] um: hostfs: avoid issues on inode number reuse by host

["Mickaël Salaün" <mic@digikod.net>]

On Fri, Feb 14, 2025 at 10:28:22AM +0100, Benjamin Berg wrote:
> From: Benjamin Berg <benjamin.berg@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.berg@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-mic@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 <mic@digikod.net>
Tested-by: Mickaël Salaün <mic@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
>