Re: [PATCH v2] qemu-sockets: add abstract UNIX domain socket support

[xiaoqiang.zhao]

在 2020/4/23 下午5:00, Daniel P. Berrangé 写道:
> Adding Eric & Markus for QAPI modelling questions
>
> On Thu, Apr 23, 2020 at 11:56:40AM +0800, xiaoqiang zhao wrote:
> > unix_connect_saddr now support abstract address type
> >
> > By default qemu does not support abstract UNIX domain
> > socket address. Add this ability to make qemu handy
> > when abstract address is needed.
> Was that a specific app you're using with QEMU that needs this ?

Thanks for your reply !

I once use qemu to connect a unix domain socket server (with abstract 
type address written by android java code)

> > Abstract address is marked by prefixing the address name with a '@'.
> For full support of the abstract namespace we would ned to allow
> the "sun_path" to contain an arbitrary mix of NULs and non-NULs
> characters, and allow connect() @addrlen to be an arbitrary size.
>
> This patch only allows a single initial NUL, and reqiures @addrlen
> to be the full size of sun_path, padding with trailing NULs. This
> limitation is impossible to lift with QEMU's current approach to
> UNIX sockets, as it relies on passing around a NULL terminated
> string, so there's no way to have embedded NULs. Since there's
> no explicit length, we have to chooose between forcing the full
> sun_path size as @addrlen, or forcing the string length as the
> @addrlen value.
>
> IIUC, socat makes the latter decision by default, but has a
> flag to switch to the former.
>
>    [man socat]
>    unix-tightsocklen=[0|1]
>    On  socket  operations,  pass  a  socket  address  length that does not
>    include the whole struct sockaddr_un record but (besides  other  compo‐
>    nents)  only  the  relevant  part  of  the filename or abstract string.
>    Default is 1.
>    [/man]
>
> This actually is supported for both abstract and non-abstract
> sockets, though IIUC this doesn't make a semantic difference
> for non-abstract sockets.
>
> The point is we have four possible combinations
>
>   NON-ABSTRACT + FULL SIZE
>   NON-ABSTRACT + MINIMAL SIZE  (default)
>   ABSTRACT + FULL SIZE
>   ABSTRACT + MINIMAL SIZE  (default)
>
> With your patch doing the latter, it means QEMU supports
> only two combinations
>
>    NON+ABSTRACT + FULL SIZE
>    ABSTRACT + MINIMAL SIZE
>
> and also can't use "@somerealpath" for a non-abstract
> socket, though admittedly this is unlikely.
>
> Socat uses a special option to request use of abstract
> sockets. eg ABSTRACT:somepath, and automatically adds
> the leading NUL, so there's no need for a special "@"
> character. This means that UNIX:@somepath still resolves
> to a filesystem path and not a abstract socket path.
>
> Finally, the patch as only added support for connect()
> not listen(). I think if QEMU wants to support abstract
> sockets we must do both, and also have unit tests added
> to tests/test-util-sockets.c
Yes , I missed these parts.
> The question is whether we're ok with this simple
> approach in QEMU, or should do a full approach with
> more explicit modelling.
Agree,  more comments is welcome.
> ie should we change QAPI thus:
>
> { 'struct': 'UnixSocketAddress',
>    'data': {
>      'path': 'str',
>      'tight': 'bool',
>      'abstract': 'bool' } }
>
> where 'tight' is a flag indicating whether to set @addrlen
> to the minimal string length, or the maximum sun_path length.
> And 'abstract' indicates that we automagically add a leading
> NUL.
>
> This would *not* allow for NULs in the middle of path,
> but I'm not so bothered about that, since I can't see that
> being widely used. If we really did need that it could be
> added via a 'base64': 'bool' flag, to indicate that @path
> is base64 encoded and thus may contain NULs
>
>  From a CLI POV, this could be mapped to QAPI thus
>
>   *  -chardev unix:somepath
>
>        @path==somepath
>        @tight==false
>        @abstract==false
>
>   *  -chardev unix:somepath,tight
>
>        @path==somepath
>        @tight==true
>        @abstract==false
>
>   *  -chardev unix-abstract:somepath
>
>        @path==somepath
>        @tight==false
>        @abstract==true
>
>   *  -chardev unix-abstract:somepath,tight
>
>        @path==somepath
>        @tight==true
>        @abstract==true
>
>
>
> Regards,
> Daniel