| Message ID | 9db9268aac4eb87e1471ab8240109c7c29be3bef.1650370027.git.qemu_oss@crudebyte.com |
|---|---|
| State | New |
| Headers | show |
| Series | 9pfs: macOS host fixes | expand |
On Tue, 19 Apr 2022 13:41:15 +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. > For the sake of accuracy : this is converting the host's dev_t to 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>. > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > --- > hw/9pfs/9p.c | 37 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 36 insertions(+), 1 deletion(-) > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > index 225f31fc31..d953035e1c 100644 > --- a/hw/9pfs/9p.c > +++ b/hw/9pfs/9p.c > @@ -1318,6 +1318,41 @@ static int32_t stat_to_iounit(const V9fsPDU *pdu, const struct stat *stbuf) > return blksize_to_iounit(pdu, stbuf->st_blksize); > } > > +#if !defined(CONFIG_LINUX) > + > +/* > + * Generates a Linux device number (a.k.a. dev_t) for given device major > + * and minor numbers. > + */ > +static 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 uint64_t host_dev_to_dotl_dev(dev_t dev) > +{ > +#ifdef CONFIG_LINUX > + return dev; > +#else > + return makedev_dotl(major(dev), minor(dev)); > +#endif > +} > + It is a bit unfortunate to inflate 9p.c, which is large enough, with glue code. Even if they are only used here, I'd personally put them in 9p-util.h. No big deal. Reviewed-by: Greg Kurz <groug@kaod.org> > static int stat_to_v9stat_dotl(V9fsPDU *pdu, const struct stat *stbuf, > V9fsStatDotl *v9lstat) > { > @@ -1327,7 +1362,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;
On Donnerstag, 21. April 2022 09:30:56 CEST Greg Kurz wrote: > On Tue, 19 Apr 2022 13:41:15 +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. > > For the sake of accuracy : this is converting the host's dev_t to 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>. You want me to put this to the commit log? > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > > --- > > > > hw/9pfs/9p.c | 37 ++++++++++++++++++++++++++++++++++++- > > 1 file changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > > index 225f31fc31..d953035e1c 100644 > > --- a/hw/9pfs/9p.c > > +++ b/hw/9pfs/9p.c > > @@ -1318,6 +1318,41 @@ static int32_t stat_to_iounit(const V9fsPDU *pdu, > > const struct stat *stbuf)> > > return blksize_to_iounit(pdu, stbuf->st_blksize); > > > > } > > > > +#if !defined(CONFIG_LINUX) > > + > > +/* > > + * Generates a Linux device number (a.k.a. dev_t) for given device major > > + * and minor numbers. > > + */ > > +static 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 uint64_t host_dev_to_dotl_dev(dev_t dev) > > +{ > > +#ifdef CONFIG_LINUX > > + return dev; > > +#else > > + return makedev_dotl(major(dev), minor(dev)); > > +#endif > > +} > > + > > It is a bit unfortunate to inflate 9p.c, which is large enough, with > glue code. Even if they are only used here, I'd personally put them > in 9p-util.h. No big deal. Makes sense, I'll move it. > Reviewed-by: Greg Kurz <groug@kaod.org> Thanks! > > static int stat_to_v9stat_dotl(V9fsPDU *pdu, const struct stat *stbuf, > > > > V9fsStatDotl *v9lstat) > > > > { > > > > @@ -1327,7 +1362,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;
On Thu, 21 Apr 2022 12:25:23 +0200 Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > On Donnerstag, 21. April 2022 09:30:56 CEST Greg Kurz wrote: > > On Tue, 19 Apr 2022 13:41:15 +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. > > > > For the sake of accuracy : this is converting the host's dev_t to 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>. > > You want me to put this to the commit log? > Maybe closer to the code that does the actual magic... > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > > > --- > > > > > > hw/9pfs/9p.c | 37 ++++++++++++++++++++++++++++++++++++- > > > 1 file changed, 36 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > > > index 225f31fc31..d953035e1c 100644 > > > --- a/hw/9pfs/9p.c > > > +++ b/hw/9pfs/9p.c > > > @@ -1318,6 +1318,41 @@ static int32_t stat_to_iounit(const V9fsPDU *pdu, > > > const struct stat *stbuf)> > > > return blksize_to_iounit(pdu, stbuf->st_blksize); > > > > > > } > > > > > > +#if !defined(CONFIG_LINUX) > > > + > > > +/* > > > + * Generates a Linux device number (a.k.a. dev_t) for given device major > > > + * and minor numbers. ... here ^^. > > > + */ > > > +static 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 uint64_t host_dev_to_dotl_dev(dev_t dev) > > > +{ > > > +#ifdef CONFIG_LINUX > > > + return dev; > > > +#else > > > + return makedev_dotl(major(dev), minor(dev)); > > > +#endif > > > +} > > > + > > > > It is a bit unfortunate to inflate 9p.c, which is large enough, with > > glue code. Even if they are only used here, I'd personally put them > > in 9p-util.h. No big deal. > > Makes sense, I'll move it. > > > Reviewed-by: Greg Kurz <groug@kaod.org> > > Thanks! > > > > static int stat_to_v9stat_dotl(V9fsPDU *pdu, const struct stat *stbuf, > > > > > > V9fsStatDotl *v9lstat) > > > > > > { > > > > > > @@ -1327,7 +1362,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; > >
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 225f31fc31..d953035e1c 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1318,6 +1318,41 @@ static int32_t stat_to_iounit(const V9fsPDU *pdu, const struct stat *stbuf) return blksize_to_iounit(pdu, stbuf->st_blksize); } +#if !defined(CONFIG_LINUX) + +/* + * Generates a Linux device number (a.k.a. dev_t) for given device major + * and minor numbers. + */ +static 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 uint64_t host_dev_to_dotl_dev(dev_t dev) +{ +#ifdef CONFIG_LINUX + return dev; +#else + return makedev_dotl(major(dev), minor(dev)); +#endif +} + static int stat_to_v9stat_dotl(V9fsPDU *pdu, const struct stat *stbuf, V9fsStatDotl *v9lstat) { @@ -1327,7 +1362,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;
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> --- hw/9pfs/9p.c | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)