Re: [PATCH v2 3/5] 9pfs: fix wrong encoding of rdev field in Rgetattr on macOS

[Greg Kurz]

On Thu, 21 Apr 2022 17:07:46 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> The 'rdev' field in 9p reponse 'Rgetattr' is of type dev_t,
> which is actually a system dependant type and therefore both the
> size and encoding of dev_t differ between macOS and Linux.
> 
> So far we have sent 'rdev' to guest in host's dev_t format as-is,
> which caused devices to appear with wrong device numbers on
> guests running on macOS hosts, eventually leading to various
> misbehaviours on guest in conjunction with device files.
> 
> This patch fixes this issue by converting the device number from
> host's dev_t format to Linux dev_t format. As 9p request
> 'Tgettattr' is exclusive to protocol version 9p2000.L, it should
> be fair to assume that 'rdev' field is assumed to be in Linux dev_t
> format by client as well.
> 
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> Link: https://lore.kernel.org/qemu-devel/20220421093056.5ab1e7ed@bahia/
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---

Reviewed-again-by: Greg Kurz <groug@kaod.org>

>  hw/9pfs/9p-util.h | 39 +++++++++++++++++++++++++++++++++++++++
>  hw/9pfs/9p.c      |  2 +-
>  2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> index 97e681e167..2cc9a5dbfb 100644
> --- a/hw/9pfs/9p-util.h
> +++ b/hw/9pfs/9p-util.h
> @@ -19,6 +19,45 @@
>  #define O_PATH_9P_UTIL 0
>  #endif
>  
> +#if !defined(CONFIG_LINUX)
> +
> +/*
> + * Generates a Linux device number (a.k.a. dev_t) for given device major
> + * and minor numbers.
> + *
> + * To be more precise: it generates a device number in glibc's format
> + * (MMMM_Mmmm_mmmM_MMmm, 64 bits) actually, which is compatible with
> + * Linux's format (mmmM_MMmm, 32 bits), as described in <bits/sysmacros.h>.
> + */
> +static inline uint64_t makedev_dotl(uint32_t dev_major, uint32_t dev_minor)
> +{
> +    uint64_t dev;
> +
> +    // from glibc sysmacros.h:
> +    dev  = (((uint64_t) (dev_major & 0x00000fffu)) <<  8);
> +    dev |= (((uint64_t) (dev_major & 0xfffff000u)) << 32);
> +    dev |= (((uint64_t) (dev_minor & 0x000000ffu)) <<  0);
> +    dev |= (((uint64_t) (dev_minor & 0xffffff00u)) << 12);
> +    return dev;
> +}
> +
> +#endif
> +
> +/*
> + * Converts given device number from host's device number format to Linux
> + * device number format. As both the size of type dev_t and encoding of
> + * dev_t is system dependant, we have to convert them for Linux guests if
> + * host is not running Linux.
> + */
> +static inline uint64_t host_dev_to_dotl_dev(dev_t dev)
> +{
> +#ifdef CONFIG_LINUX
> +    return dev;
> +#else
> +    return makedev_dotl(major(dev), minor(dev));
> +#endif
> +}
> +
>  #ifdef CONFIG_DARWIN
>  #define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
>  #define qemu_lgetxattr(...) getxattr(__VA_ARGS__, 0, XATTR_NOFOLLOW)
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 225f31fc31..4a296a0b94 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1327,7 +1327,7 @@ static int stat_to_v9stat_dotl(V9fsPDU *pdu, const 
> struct stat *stbuf,
>      v9lstat->st_nlink = stbuf->st_nlink;
>      v9lstat->st_uid = stbuf->st_uid;
>      v9lstat->st_gid = stbuf->st_gid;
> -    v9lstat->st_rdev = stbuf->st_rdev;
> +    v9lstat->st_rdev = host_dev_to_dotl_dev(stbuf->st_rdev);
>      v9lstat->st_size = stbuf->st_size;
>      v9lstat->st_blksize = stat_to_iounit(pdu, stbuf);
>      v9lstat->st_blocks = stbuf->st_blocks;