All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Schoenebeck <qemu_oss@crudebyte.com>
To: qemu-devel@nongnu.org
Cc: "Laurent Vivier" <lvivier@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	qemu-devel@nongnu.org, "Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	hi@alyssa.is, "Michael Roitzsch" <reactorcontrol@icloud.com>,
	"Will Cohen" <wwcohen@gmail.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Keno Fischer" <keno@juliacomputing.com>,
	"Greg Kurz" <groug@kaod.org>,
	"Akihiko Odaki" <akihiko.odaki@gmail.com>
Subject: Re: [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat
Date: Tue, 12 Apr 2022 14:19:50 +0200	[thread overview]
Message-ID: <17933734.zYzKuhC07K@silver> (raw)
In-Reply-To: <20220408170059.1134a039@bahia>

On Freitag, 8. April 2022 17:00:59 CEST Greg Kurz wrote:
> On Fri, 08 Apr 2022 15:52:25 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Sonntag, 27. Februar 2022 23:35:20 CEST Will Cohen wrote:
> > > From: Keno Fischer <keno@juliacomputing.com>
> > > 
> > > Darwin does not support mknodat. However, to avoid race conditions
> > > with later setting the permissions, we must avoid using mknod on
> > > the full path instead. We could try to fchdir, but that would cause
> > > problems if multiple threads try to call mknodat at the same time.
> > > However, luckily there is a solution: Darwin includes a function
> > > that sets the cwd for the current thread only.
> > > This should suffice to use mknod safely.
> > 
> > [...]
> > 
> > > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > > index cdb4c9e24c..bec0253474 100644
> > > --- a/hw/9pfs/9p-util-darwin.c
> > > +++ b/hw/9pfs/9p-util-darwin.c
> > > @@ -7,6 +7,8 @@
> > > 
> > >  #include "qemu/osdep.h"
> > >  #include "qemu/xattr.h"
> > > 
> > > +#include "qapi/error.h"
> > > +#include "qemu/error-report.h"
> > > 
> > >  #include "9p-util.h"
> > >  
> > >  ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const
> > >  char
> > > 
> > > *name, @@ -62,3 +64,34 @@ int fsetxattrat_nofollow(int dirfd, const char
> > > *filename, const char *name, close_preserve_errno(fd);
> > > 
> > >      return ret;
> > >  
> > >  }
> > > 
> > > +
> > > +/*
> > > + * As long as mknodat is not available on macOS, this workaround
> > > + * using pthread_fchdir_np is needed.
> > > + *
> > > + * Radar filed with Apple for implementing mknodat:
> > > + * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
> > > + */
> > > +#if defined CONFIG_PTHREAD_FCHDIR_NP
> > > +
> > > +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t
> > > dev)
> > > +{
> > > +    int preserved_errno, err;
> > > +    if (!pthread_fchdir_np) {
> > > +        error_report_once("pthread_fchdir_np() not available on this
> > > version of macOS"); +        return -ENOTSUP;
> > > +    }
> > > +    if (pthread_fchdir_np(dirfd) < 0) {
> > > +        return -1;
> > > +    }
> > > +    err = mknod(filename, mode, dev);
> > 
> > I just tested this on macOS Monterey and realized mknod() seems to require
> > admin privileges on macOS to work. So if you run QEMU as ordinary user on
> > macOS then mknod() would fail with errno=1 (Operation not permitted).
> > 
> > That means a lot of stuff would simply not work on macOS, unless you
> > really
> > want to run QEMU with super user privileges, which does not sound
> > appealing to me. :/
> > 
> > Should we introduce another fake behaviour here, i.e. remapping this on
> > macOS hosts as regular file and make guest believe it would create a
> > device, similar as we already do for mapped links?
> 
> I'd rather keep that for the mapped security mode only to avoid
> confusion, but qemu_mknodat() is also used in passthrough mode.
> 
> Anyway, it seems that macOS's mknod() only creates device files,
> unlike linux (POSIX) which is also used to create FIFOs, sockets
> and regular files. And it also requires elevated privileges,
> CAP_MKNOD, in order to create device files.
> 
> It seems that this implementation of qemu_mknodat() is just missing
> some features that can be implemented with unprivileged syscalls like
> mkfifo(), socket() and open().

+Akihiko on CC.

Like always when it comes to POSIX APIs, Apple's documentation on this is far 
from being great. I actually had to test out what's supported with mknod() on 
macOS, in which way, and what is not (tested on macOS 12 "Monterey" only):

* S_IFIFO: works, even as regular user.

* S_IFREG: doesn't work, neither as regular user (ERRNO 1, Operation not 
  permitted), nor as super-user (ERRNO 22, Invalid argument). So we should 
  divert that to a regular open() call on macOS.

* S_IFCHR and S_IFBLK: works as super-user, doesn't work for regular user 
  (ERRNO 1, Operation not permitted). So if 9p is used with passthrough 
  permissions, we should probably stick to the direct mknod() call and that 
  the user would need to run QEMU as super-user to get this working. Whereas 
  if 9p is used with mapped permissions, we should fake those devices by 
  creating regular files, store their type and major, minor numbers there and 
  that's it. We don't expect that guest ever tries to read/write such block/
  character devices, right? I.e. I'm assuming that any read/write is handled 
  as an overlay by Linux kernel on its guest level, correct?

* S_IFSOCK: doesn't work, neither as regular user (ERRNO 1, Operation not 
  permitted), nor as super-user (ERRNO 22, Invalid argument). So we should 
  divert that to a socket() call on macOS.

Thoughts?

Best regards,
Christian Schoenebeck




  reply	other threads:[~2022-04-12 12:23 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-27 22:35 [PATCH v9 00/11] 9p: Add support for darwin Will Cohen
2022-02-27 22:35 ` [PATCH v9 01/11] 9p: linux: Fix a couple Linux assumptions Will Cohen
2022-02-27 22:35 ` [PATCH v9 02/11] 9p: Rename 9p-util -> 9p-util-linux Will Cohen
2022-02-27 22:35 ` [PATCH v9 03/11] 9p: darwin: Handle struct stat(fs) differences Will Cohen
2022-02-27 22:35 ` [PATCH v9 04/11] 9p: darwin: Handle struct dirent differences Will Cohen
2022-02-27 22:35 ` [PATCH v9 05/11] 9p: darwin: Ignore O_{NOATIME, DIRECT} Will Cohen
2022-02-27 22:35 ` [PATCH v9 06/11] 9p: darwin: Move XATTR_SIZE_MAX->P9_XATTR_SIZE_MAX Will Cohen
2022-02-27 22:35 ` [PATCH v9 07/11] 9p: darwin: *xattr_nofollow implementations Will Cohen
2022-02-27 22:35 ` [PATCH v9 08/11] 9p: darwin: Compatibility for f/l*xattr Will Cohen
2022-02-27 22:35 ` [PATCH v9 09/11] 9p: darwin: Implement compatibility for mknodat Will Cohen
2022-02-28 13:20   ` Christian Schoenebeck
2022-02-28 13:36     ` Thomas Huth
2022-02-28 13:51       ` Christian Schoenebeck
2022-02-28 14:06         ` Peter Maydell
2022-02-28 15:45           ` Christian Schoenebeck
2022-02-28 13:37     ` Will Cohen
2022-02-28 13:41       ` Greg Kurz
2022-04-08 13:52   ` Christian Schoenebeck
2022-04-08 15:00     ` Greg Kurz
2022-04-12 12:19       ` Christian Schoenebeck [this message]
2022-02-27 22:35 ` [PATCH v9 10/11] 9p: darwin: Adjust assumption on virtio-9p-test Will Cohen
2022-02-27 22:35 ` [PATCH v9 11/11] 9p: darwin: meson: Allow VirtFS on Darwin Will Cohen
2022-02-28 13:11   ` Christian Schoenebeck
2022-02-28 13:43     ` Will Cohen
2022-03-01 19:25 ` [PATCH v9 00/11] 9p: Add support for darwin Christian Schoenebeck
2022-03-01 20:09   ` Will Cohen
2022-03-02 18:15     ` Christian Schoenebeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=17933734.zYzKuhC07K@silver \
    --to=qemu_oss@crudebyte.com \
    --cc=akihiko.odaki@gmail.com \
    --cc=f4bug@amsat.org \
    --cc=groug@kaod.org \
    --cc=hi@alyssa.is \
    --cc=keno@juliacomputing.com \
    --cc=lvivier@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=reactorcontrol@icloud.com \
    --cc=thuth@redhat.com \
    --cc=wwcohen@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.