[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/6] qemu-bridge-helper: Fix misuse of isspace()

From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-devel] [PATCH 1/6] qemu-bridge-helper: Fix misuse of isspace()
Date: Tue, 14 May 2019 18:03:40 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1

On 5/14/19 2:18 PM, Markus Armbruster wrote:
> Peter Maydell <address@hidden> writes:
>> On Mon, 13 May 2019 at 14:21, Markus Armbruster <address@hidden> wrote:
>>> Perhaps I should do it just for this file while I touch it anyway.  The
>>> question to ask: should parse_acl_file() obey the locale for whitespace
>>> recognition?
>> I vote for "no".
>> Q: do we document the format of the ACL file anywhere ?
> Support for it was added in commit bdef79a2994, v1.1.  Just code, no
> documentation.
> Grepping for qemu-bridge-helper finds just qemu-options.hx.  Contains
> -help output and some .texi that goes into qemu-doc and the manual page.
> None of it mentions how qemu-bridge-helper is run, or the ACL file
> feature, let alone what its format might be.
> I'm afraid all we have is the commit message.  Which doesn't really
> define the file format, it merely gives a bunch of examples.
> As far as I can tell, qemu-bridge-helper is for use with -netdev tap and
> -netdev bridge.
> Both variations of -netdev call net_bridge_run_helper() to run the
> helper.  First argument is -netdev parameter "helper", default usually
> "$prefix/libexec/qemu-bridge-helper".  Second argument is parameter
> "br", default "br0".
> If @helper contains space or tab, net_bridge_run_helper() guesses its a
> full command, else it guesses its the name of the executable.  Bad
> magic.
> If it guesses name of executable, it execv()s this executable with
> arguments "--use-vnet", "--fd=FD", "address@hidden".
> If it guesses full command, it appends "--use-vnet --fd=FD", where FD is
> the helper's half of the socketpair used to connect QEMU and the helper.
> It further appends "address@hidden", unless @helper contains "--br=".
> More bad magic.
> It executes the resulting string with sh -c.  Magic cherry on top.
> When the helper fails, netdev creation fails.
> The helper we ship with QEMU unconditionally tries to read
> "$prefix/etc/bridge.conf".  Fatal error if this file doesn't exist.
> Errors in this file are fatal.  Errors in files it includes are not
> fatal; instead, the remainder of the erroneous file is ignored.
> *Boggle*
> As far as I can tell, libvirt runs qemu-bridge-helper itself (Paolo's
> commit 2d80fbb14df).  Makes sense, because running QEMU with the
> necessary privileges would be unwise, and so would be letting it execute
> setuid helpers.  Also bypasses the bad magic in QEMU's
> net_bridge_run_helper().
> qemu-bridge-helper should have a manual page, and its handling of errors
> in ACL include files needs work.  There's probably more; I just glanced
> at it.  I'm not volunteering, though.  It lacks a maintainer.  Should we
> add it to Jason's "Network device backends"?
> -netdev's helper parameter is seriously underdocumented.  Document or
> deprecate?

I understood the project policy is "deprecate until maintained or
tested"... If not, we might start to think about it :)

reply via email to

[Prev in Thread] Current Thread [Next in Thread]