diff mbox series

[v5,2/6] 9pfs: fix qemu_mknodat(S_IFSOCK) on macOS

Message ID 2e7b5ecd7a6d83a538db4e8a22d8fb03e9e0f06e.1651228001.git.qemu_oss@crudebyte.com
State New
Headers show
Series 9pfs: macOS host fixes | expand

Commit Message

Christian Schoenebeck April 29, 2022, 10:25 a.m. UTC
mknod() on macOS does not support creating sockets, so divert to
call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
was passed with mode argument.

Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p-util-darwin.c | 42 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

Comments

Greg Kurz April 29, 2022, 12:56 p.m. UTC | #1
On Fri, 29 Apr 2022 12:25:11 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> mknod() on macOS does not support creating sockets, so divert to
> call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
> was passed with mode argument.
> 
> Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
>  hw/9pfs/9p-util-darwin.c | 42 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> index e24d09763a..619c403ba7 100644
> --- a/hw/9pfs/9p-util-darwin.c
> +++ b/hw/9pfs/9p-util-darwin.c
> @@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
>   */
>  #if defined CONFIG_PTHREAD_FCHDIR_NP
>  
> +static int create_socket_file_at_cwd(const char *filename, mode_t mode) {
> +    int fd, err;
> +    struct sockaddr_un addr = {
> +        .sun_family = AF_UNIX
> +    };
> +
> +    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s", filename);
> +    if (err < 0 || err >= sizeof(addr.sun_path)) {

According to POSIX [1]:

The snprintf() function shall fail if:

[EOVERFLOW]
[CX] [Option Start] The value of n is greater than {INT_MAX}. [Option End]

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.html

Since we're passing sizeof(addr.sun_path), I'm pretty sure snprintf()
cannot fail. No big deal.

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

> +        errno = ENAMETOOLONG;
> +        return -1;
> +    }
> +    fd = socket(PF_UNIX, SOCK_DGRAM, 0);
> +    if (fd == -1) {
> +        return fd;
> +    }
> +    err = bind(fd, (struct sockaddr *) &addr, sizeof(addr));
> +    if (err == -1) {
> +        goto out;
> +    }
> +    /*
> +     * FIXME: Should rather be using descriptor-based fchmod() on the
> +     * socket file descriptor above (preferably before bind() call),
> +     * instead of path-based fchmodat(), to prevent concurrent transient
> +     * state issues between creating the named FIFO file at bind() and
> +     * delayed adjustment of permissions at fchmodat(). However currently
> +     * macOS (12.x) does not support such operations on socket file
> +     * descriptors yet.
> +     *
> +     * Filed report with Apple: FB9997731
> +     */
> +    err = fchmodat(AT_FDCWD, filename, mode, AT_SYMLINK_NOFOLLOW_ANY);
> +out:
> +    close_preserve_errno(fd);
> +    return err;
> +}
> +
>  int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
>  {
>      int preserved_errno, err;
> @@ -93,7 +129,11 @@ int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
>      if (pthread_fchdir_np(dirfd) < 0) {
>          return -1;
>      }
> -    err = mknod(filename, mode, dev);
> +    if (S_ISSOCK(mode)) {
> +        err = create_socket_file_at_cwd(filename, mode);
> +    } else {
> +        err = mknod(filename, mode, dev);
> +    }
>      preserved_errno = errno;
>      /* Stop using the thread-local cwd */
>      pthread_fchdir_np(-1);
Christian Schoenebeck April 29, 2022, 1:50 p.m. UTC | #2
On Freitag, 29. April 2022 14:56:50 CEST Greg Kurz wrote:
> On Fri, 29 Apr 2022 12:25:11 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > mknod() on macOS does not support creating sockets, so divert to
> > call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
> > was passed with mode argument.
> > 
> > Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> > 
> >  hw/9pfs/9p-util-darwin.c | 42 +++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 41 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > index e24d09763a..619c403ba7 100644
> > --- a/hw/9pfs/9p-util-darwin.c
> > +++ b/hw/9pfs/9p-util-darwin.c
> > @@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char
> > *filename, const char *name,> 
> >   */
> >  
> >  #if defined CONFIG_PTHREAD_FCHDIR_NP
> > 
> > +static int create_socket_file_at_cwd(const char *filename, mode_t mode) {
> > +    int fd, err;
> > +    struct sockaddr_un addr = {
> > +        .sun_family = AF_UNIX
> > +    };
> > +
> > +    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s",
> > filename); +    if (err < 0 || err >= sizeof(addr.sun_path)) {
> 
> According to POSIX [1]:
> 
> The snprintf() function shall fail if:
> 
> [EOVERFLOW]
> [CX] [Option Start] The value of n is greater than {INT_MAX}. [Option End]
> 
> [1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.html
> 
> Since we're passing sizeof(addr.sun_path), I'm pretty sure snprintf()
> cannot fail. No big deal.

The question is whom you would want to trust on this? POSIX? ISO-C? Clang? 
BSD? Apple? And for how long into future? I mean in general yes, I would not 
expect it to fail with -1 here either, but there are various different API 
docs on snprintf() out there, and most of them don't even bother to enumarate 
which encoding errors may happen. And I'm pretty sure if I'd drop the negative 
err check here, then Akihiko would slap me for unforeseeable additional error 
cases on snprintf() that may be added in future.

Apple's documentation on snprintf() BTW just says:

  "These functions return a negative value if an error occurs."

So Apple does not even restrict the return value to -1 on errrors, you would 
also need to expect other negative values.

So on doubt, I leave this negative result check for now. ;-)

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

Thanks!

Best regards
Christian Schoenebeck

> > +        errno = ENAMETOOLONG;
> > +        return -1;
> > +    }
> > +    fd = socket(PF_UNIX, SOCK_DGRAM, 0);
> > +    if (fd == -1) {
> > +        return fd;
> > +    }
> > +    err = bind(fd, (struct sockaddr *) &addr, sizeof(addr));
> > +    if (err == -1) {
> > +        goto out;
> > +    }
> > +    /*
> > +     * FIXME: Should rather be using descriptor-based fchmod() on the
> > +     * socket file descriptor above (preferably before bind() call),
> > +     * instead of path-based fchmodat(), to prevent concurrent transient
> > +     * state issues between creating the named FIFO file at bind() and
> > +     * delayed adjustment of permissions at fchmodat(). However currently
> > +     * macOS (12.x) does not support such operations on socket file
> > +     * descriptors yet.
> > +     *
> > +     * Filed report with Apple: FB9997731
> > +     */
> > +    err = fchmodat(AT_FDCWD, filename, mode, AT_SYMLINK_NOFOLLOW_ANY);
> > +out:
> > +    close_preserve_errno(fd);
> > +    return err;
> > +}
> > +
> > 
> >  int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
> >  {
> >  
> >      int preserved_errno, err;
> > 
> > @@ -93,7 +129,11 @@ int qemu_mknodat(int dirfd, const char *filename,
> > mode_t mode, dev_t dev)> 
> >      if (pthread_fchdir_np(dirfd) < 0) {
> >      
> >          return -1;
> >      
> >      }
> > 
> > -    err = mknod(filename, mode, dev);
> > +    if (S_ISSOCK(mode)) {
> > +        err = create_socket_file_at_cwd(filename, mode);
> > +    } else {
> > +        err = mknod(filename, mode, dev);
> > +    }
> > 
> >      preserved_errno = errno;
> >      /* Stop using the thread-local cwd */
> >      pthread_fchdir_np(-1);
Greg Kurz April 29, 2022, 2:35 p.m. UTC | #3
On Fri, 29 Apr 2022 15:50:35 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Freitag, 29. April 2022 14:56:50 CEST Greg Kurz wrote:
> > On Fri, 29 Apr 2022 12:25:11 +0200
> > 
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > mknod() on macOS does not support creating sockets, so divert to
> > > call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
> > > was passed with mode argument.
> > > 
> > > Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
> > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > ---
> > > 
> > >  hw/9pfs/9p-util-darwin.c | 42 +++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 41 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > > index e24d09763a..619c403ba7 100644
> > > --- a/hw/9pfs/9p-util-darwin.c
> > > +++ b/hw/9pfs/9p-util-darwin.c
> > > @@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char
> > > *filename, const char *name,> 
> > >   */
> > >  
> > >  #if defined CONFIG_PTHREAD_FCHDIR_NP
> > > 
> > > +static int create_socket_file_at_cwd(const char *filename, mode_t mode) {
> > > +    int fd, err;
> > > +    struct sockaddr_un addr = {
> > > +        .sun_family = AF_UNIX
> > > +    };
> > > +
> > > +    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s",
> > > filename); +    if (err < 0 || err >= sizeof(addr.sun_path)) {
> > 
> > According to POSIX [1]:
> > 
> > The snprintf() function shall fail if:
> > 
> > [EOVERFLOW]
> > [CX] [Option Start] The value of n is greater than {INT_MAX}. [Option End]
> > 
> > [1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.html
> > 
> > Since we're passing sizeof(addr.sun_path), I'm pretty sure snprintf()
> > cannot fail. No big deal.
> 
> The question is whom you would want to trust on this? POSIX? ISO-C? Clang? 
> BSD? Apple? And for how long into future? I mean in general yes, I would not 

To improve overall portability across all possible hosts, I'd stick to
POSIX semantics but here this is macOS only code so you can assume
this is Apple's snprintf().

> expect it to fail with -1 here either, but there are various different API 
> docs on snprintf() out there, and most of them don't even bother to enumarate 
> which encoding errors may happen. And I'm pretty sure if I'd drop the negative 
> err check here, then Akihiko would slap me for unforeseeable additional error 
> cases on snprintf() that may be added in future.
> 

/o\ ;-)

> Apple's documentation on snprintf() BTW just says:
> 
>   "These functions return a negative value if an error occurs."
> 

How valuable this is !!! ;-)

> So Apple does not even restrict the return value to -1 on errrors, you would 
> also need to expect other negative values.
> 
> So on doubt, I leave this negative result check for now. ;-)
> 

Fair enough.

> > Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> Thanks!
> 
> Best regards
> Christian Schoenebeck
> 
> > > +        errno = ENAMETOOLONG;
> > > +        return -1;
> > > +    }
> > > +    fd = socket(PF_UNIX, SOCK_DGRAM, 0);
> > > +    if (fd == -1) {
> > > +        return fd;
> > > +    }
> > > +    err = bind(fd, (struct sockaddr *) &addr, sizeof(addr));
> > > +    if (err == -1) {
> > > +        goto out;
> > > +    }
> > > +    /*
> > > +     * FIXME: Should rather be using descriptor-based fchmod() on the
> > > +     * socket file descriptor above (preferably before bind() call),
> > > +     * instead of path-based fchmodat(), to prevent concurrent transient
> > > +     * state issues between creating the named FIFO file at bind() and
> > > +     * delayed adjustment of permissions at fchmodat(). However currently
> > > +     * macOS (12.x) does not support such operations on socket file
> > > +     * descriptors yet.
> > > +     *
> > > +     * Filed report with Apple: FB9997731
> > > +     */
> > > +    err = fchmodat(AT_FDCWD, filename, mode, AT_SYMLINK_NOFOLLOW_ANY);
> > > +out:
> > > +    close_preserve_errno(fd);
> > > +    return err;
> > > +}
> > > +
> > > 
> > >  int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
> > >  {
> > >  
> > >      int preserved_errno, err;
> > > 
> > > @@ -93,7 +129,11 @@ int qemu_mknodat(int dirfd, const char *filename,
> > > mode_t mode, dev_t dev)> 
> > >      if (pthread_fchdir_np(dirfd) < 0) {
> > >      
> > >          return -1;
> > >      
> > >      }
> > > 
> > > -    err = mknod(filename, mode, dev);
> > > +    if (S_ISSOCK(mode)) {
> > > +        err = create_socket_file_at_cwd(filename, mode);
> > > +    } else {
> > > +        err = mknod(filename, mode, dev);
> > > +    }
> > > 
> > >      preserved_errno = errno;
> > >      /* Stop using the thread-local cwd */
> > >      pthread_fchdir_np(-1);
> 
>
Christian Schoenebeck April 29, 2022, 3:20 p.m. UTC | #4
On Freitag, 29. April 2022 16:35:07 CEST Greg Kurz wrote:
> On Fri, 29 Apr 2022 15:50:35 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Freitag, 29. April 2022 14:56:50 CEST Greg Kurz wrote:
> > > On Fri, 29 Apr 2022 12:25:11 +0200
> > > 
> > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > mknod() on macOS does not support creating sockets, so divert to
> > > > call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
> > > > was passed with mode argument.
> > > > 
> > > > Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
> > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > ---
> > > > 
> > > >  hw/9pfs/9p-util-darwin.c | 42
> > > >  +++++++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 41 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > > > index e24d09763a..619c403ba7 100644
> > > > --- a/hw/9pfs/9p-util-darwin.c
> > > > +++ b/hw/9pfs/9p-util-darwin.c
> > > > @@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char
> > > > *filename, const char *name,>
> > > > 
> > > >   */
> > > >  
> > > >  #if defined CONFIG_PTHREAD_FCHDIR_NP
> > > > 
> > > > +static int create_socket_file_at_cwd(const char *filename, mode_t
> > > > mode) {
> > > > +    int fd, err;
> > > > +    struct sockaddr_un addr = {
> > > > +        .sun_family = AF_UNIX
> > > > +    };
> > > > +
> > > > +    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s",
> > > > filename); +    if (err < 0 || err >= sizeof(addr.sun_path)) {
> > > 
> > > According to POSIX [1]:
> > > 
> > > The snprintf() function shall fail if:
> > > 
> > > [EOVERFLOW]
> > > [CX] [Option Start] The value of n is greater than {INT_MAX}. [Option
> > > End]
> > > 
> > > [1]
> > > https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.htm
> > > l
> > > 
> > > Since we're passing sizeof(addr.sun_path), I'm pretty sure snprintf()
> > > cannot fail. No big deal.
> > 
> > The question is whom you would want to trust on this? POSIX? ISO-C? Clang?
> > BSD? Apple? And for how long into future? I mean in general yes, I would
> > not
> To improve overall portability across all possible hosts, I'd stick to
> POSIX semantics but here this is macOS only code so you can assume
> this is Apple's snprintf().
> 
> > expect it to fail with -1 here either, but there are various different API
> > docs on snprintf() out there, and most of them don't even bother to
> > enumarate which encoding errors may happen. And I'm pretty sure if I'd
> > drop the negative err check here, then Akihiko would slap me for
> > unforeseeable additional error cases on snprintf() that may be added in
> > future.
> 
> /o\ ;-)
> 
> > Apple's documentation on snprintf() BTW just says:
> >   "These functions return a negative value if an error occurs."
> 
> How valuable this is !!! ;-)
> 
> > So Apple does not even restrict the return value to -1 on errrors, you
> > would also need to expect other negative values.
> > 
> > So on doubt, I leave this negative result check for now. ;-)
> 
> Fair enough.

Hey, don't shoot the servant! I'm just trying to find compromises that aim to 
suit as many people as possible, as always. :)

Best regards,
Christian Schoenebeck
Akihiko Odaki April 29, 2022, 4:29 p.m. UTC | #5
On 2022/04/30 0:20, Christian Schoenebeck wrote:
> On Freitag, 29. April 2022 16:35:07 CEST Greg Kurz wrote:
>> On Fri, 29 Apr 2022 15:50:35 +0200
>>
>> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
>>> On Freitag, 29. April 2022 14:56:50 CEST Greg Kurz wrote:
>>>> On Fri, 29 Apr 2022 12:25:11 +0200
>>>>
>>>> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
>>>>> mknod() on macOS does not support creating sockets, so divert to
>>>>> call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
>>>>> was passed with mode argument.
>>>>>
>>>>> Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
>>>>> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
>>>>> ---
>>>>>
>>>>>   hw/9pfs/9p-util-darwin.c | 42
>>>>>   +++++++++++++++++++++++++++++++++++++++-
>>>>>   1 file changed, 41 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
>>>>> index e24d09763a..619c403ba7 100644
>>>>> --- a/hw/9pfs/9p-util-darwin.c
>>>>> +++ b/hw/9pfs/9p-util-darwin.c
>>>>> @@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char
>>>>> *filename, const char *name,>
>>>>>
>>>>>    */
>>>>>   
>>>>>   #if defined CONFIG_PTHREAD_FCHDIR_NP
>>>>>
>>>>> +static int create_socket_file_at_cwd(const char *filename, mode_t
>>>>> mode) {
>>>>> +    int fd, err;
>>>>> +    struct sockaddr_un addr = {
>>>>> +        .sun_family = AF_UNIX
>>>>> +    };
>>>>> +
>>>>> +    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s",
>>>>> filename); +    if (err < 0 || err >= sizeof(addr.sun_path)) {
>>>>
>>>> According to POSIX [1]:
>>>>
>>>> The snprintf() function shall fail if:
>>>>
>>>> [EOVERFLOW]
>>>> [CX] [Option Start] The value of n is greater than {INT_MAX}. [Option
>>>> End]
>>>>
>>>> [1]
>>>> https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.htm
>>>> l
>>>>
>>>> Since we're passing sizeof(addr.sun_path), I'm pretty sure snprintf()
>>>> cannot fail. No big deal.
>>>
>>> The question is whom you would want to trust on this? POSIX? ISO-C? Clang?
>>> BSD? Apple? And for how long into future? I mean in general yes, I would
>>> not
>> To improve overall portability across all possible hosts, I'd stick to
>> POSIX semantics but here this is macOS only code so you can assume
>> this is Apple's snprintf().
>>
>>> expect it to fail with -1 here either, but there are various different API
>>> docs on snprintf() out there, and most of them don't even bother to
>>> enumarate which encoding errors may happen. And I'm pretty sure if I'd
>>> drop the negative err check here, then Akihiko would slap me for
>>> unforeseeable additional error cases on snprintf() that may be added in
>>> future. >>
>> /o\ ;-)

I would rather throw the system which decided returning a negative value 
for snprintf() OK away, but it is always good to be cautious anyway. ;)

For the entire series:
Reviewed-by: Akihiko Odaki <akihiko.odaki@gmail.com>

>>
>>> Apple's documentation on snprintf() BTW just says:
>>>    "These functions return a negative value if an error occurs."
>>
>> How valuable this is !!! ;-)
>>
>>> So Apple does not even restrict the return value to -1 on errrors, you
>>> would also need to expect other negative values.
>>>
>>> So on doubt, I leave this negative result check for now. ;-)
>>
>> Fair enough.
> 
> Hey, don't shoot the servant! I'm just trying to find compromises that aim to
> suit as many people as possible, as always. :)
> 
> Best regards,
> Christian Schoenebeck
> 
>
Greg Kurz May 2, 2022, 6:45 a.m. UTC | #6
On Fri, 29 Apr 2022 17:20:26 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Freitag, 29. April 2022 16:35:07 CEST Greg Kurz wrote:
> > On Fri, 29 Apr 2022 15:50:35 +0200
> > 
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > On Freitag, 29. April 2022 14:56:50 CEST Greg Kurz wrote:
> > > > On Fri, 29 Apr 2022 12:25:11 +0200
> > > > 
> > > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > > mknod() on macOS does not support creating sockets, so divert to
> > > > > call sequence socket(), bind() and fchmodat() respectively if S_IFSOCK
> > > > > was passed with mode argument.
> > > > > 
> > > > > Link: https://lore.kernel.org/qemu-devel/17933734.zYzKuhC07K@silver/
> > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > > ---
> > > > > 
> > > > >  hw/9pfs/9p-util-darwin.c | 42
> > > > >  +++++++++++++++++++++++++++++++++++++++-
> > > > >  1 file changed, 41 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> > > > > index e24d09763a..619c403ba7 100644
> > > > > --- a/hw/9pfs/9p-util-darwin.c
> > > > > +++ b/hw/9pfs/9p-util-darwin.c
> > > > > @@ -74,6 +74,42 @@ int fsetxattrat_nofollow(int dirfd, const char
> > > > > *filename, const char *name,>
> > > > > 
> > > > >   */
> > > > >  
> > > > >  #if defined CONFIG_PTHREAD_FCHDIR_NP
> > > > > 
> > > > > +static int create_socket_file_at_cwd(const char *filename, mode_t
> > > > > mode) {
> > > > > +    int fd, err;
> > > > > +    struct sockaddr_un addr = {
> > > > > +        .sun_family = AF_UNIX
> > > > > +    };
> > > > > +
> > > > > +    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s",
> > > > > filename); +    if (err < 0 || err >= sizeof(addr.sun_path)) {
> > > > 
> > > > According to POSIX [1]:
> > > > 
> > > > The snprintf() function shall fail if:
> > > > 
> > > > [EOVERFLOW]
> > > > [CX] [Option Start] The value of n is greater than {INT_MAX}. [Option
> > > > End]
> > > > 
> > > > [1]
> > > > https://pubs.opengroup.org/onlinepubs/9699919799/functions/snprintf.htm
> > > > l
> > > > 
> > > > Since we're passing sizeof(addr.sun_path), I'm pretty sure snprintf()
> > > > cannot fail. No big deal.
> > > 
> > > The question is whom you would want to trust on this? POSIX? ISO-C? Clang?
> > > BSD? Apple? And for how long into future? I mean in general yes, I would
> > > not
> > To improve overall portability across all possible hosts, I'd stick to
> > POSIX semantics but here this is macOS only code so you can assume
> > this is Apple's snprintf().
> > 
> > > expect it to fail with -1 here either, but there are various different API
> > > docs on snprintf() out there, and most of them don't even bother to
> > > enumarate which encoding errors may happen. And I'm pretty sure if I'd
> > > drop the negative err check here, then Akihiko would slap me for
> > > unforeseeable additional error cases on snprintf() that may be added in
> > > future.
> > 
> > /o\ ;-)
> > 
> > > Apple's documentation on snprintf() BTW just says:
> > >   "These functions return a negative value if an error occurs."
> > 
> > How valuable this is !!! ;-)
> > 
> > > So Apple does not even restrict the return value to -1 on errrors, you
> > > would also need to expect other negative values.
> > > 
> > > So on doubt, I leave this negative result check for now. ;-)
> > 
> > Fair enough.
> 
> Hey, don't shoot the servant! I'm just trying to find compromises that aim to 
> suit as many people as possible, as always. :)
> 

Oh I wasn't criticizing your work... rather feeling sorry for the
poor documentation of snprintf() on the Apple side. Please go
on with the great job you're doing on 9p ! :-)

Cheers,

--
Greg

> Best regards,
> Christian Schoenebeck
> 
>
diff mbox series

Patch

diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
index e24d09763a..619c403ba7 100644
--- a/hw/9pfs/9p-util-darwin.c
+++ b/hw/9pfs/9p-util-darwin.c
@@ -74,6 +74,42 @@  int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
  */
 #if defined CONFIG_PTHREAD_FCHDIR_NP
 
+static int create_socket_file_at_cwd(const char *filename, mode_t mode) {
+    int fd, err;
+    struct sockaddr_un addr = {
+        .sun_family = AF_UNIX
+    };
+
+    err = snprintf(addr.sun_path, sizeof(addr.sun_path), "./%s", filename);
+    if (err < 0 || err >= sizeof(addr.sun_path)) {
+        errno = ENAMETOOLONG;
+        return -1;
+    }
+    fd = socket(PF_UNIX, SOCK_DGRAM, 0);
+    if (fd == -1) {
+        return fd;
+    }
+    err = bind(fd, (struct sockaddr *) &addr, sizeof(addr));
+    if (err == -1) {
+        goto out;
+    }
+    /*
+     * FIXME: Should rather be using descriptor-based fchmod() on the
+     * socket file descriptor above (preferably before bind() call),
+     * instead of path-based fchmodat(), to prevent concurrent transient
+     * state issues between creating the named FIFO file at bind() and
+     * delayed adjustment of permissions at fchmodat(). However currently
+     * macOS (12.x) does not support such operations on socket file
+     * descriptors yet.
+     *
+     * Filed report with Apple: FB9997731
+     */
+    err = fchmodat(AT_FDCWD, filename, mode, AT_SYMLINK_NOFOLLOW_ANY);
+out:
+    close_preserve_errno(fd);
+    return err;
+}
+
 int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
 {
     int preserved_errno, err;
@@ -93,7 +129,11 @@  int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
     if (pthread_fchdir_np(dirfd) < 0) {
         return -1;
     }
-    err = mknod(filename, mode, dev);
+    if (S_ISSOCK(mode)) {
+        err = create_socket_file_at_cwd(filename, mode);
+    } else {
+        err = mknod(filename, mode, dev);
+    }
     preserved_errno = errno;
     /* Stop using the thread-local cwd */
     pthread_fchdir_np(-1);