[libvirt] [PATCH] nwfilter: Partly initialize driver even for non-privileged users

[Laine Stump]

On 04/16/2015 04:45 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1211436
>
> This reverts commit b7829f959b33c6e32422222a9ed745c0da7dc696.

Well, not exactly - it adds something too :-)

>
> The previous fix was not correct. Like everywhere else, a driver is a
> global variable allocated in stateInitialize function (or something
> similar for stateless drivers). Later, when a driver API is called,
> it's possible that the global variable is accessed and dereferenced.
> Now, some drivers require root privileges because they undertake some
> actions reserved only for the system admin (e.g. manipulating host
> firewall). And here's the trouble, the NWFilter state initializer
> exited too early when finding out it's running unprivileged, leaving
> the global NWFilter driver variable uninitialized. Any subsequent
> API call that tried to lock the driver resulted in dereferencing the
> driver and thus crash.
>
> On the other hand, in order to not resurrect the bug the original
> commit was fixing, Let's forbid the nwfilter define in session mode.

Sure, makes sense. The other APIs will just return "nothing found" since
there are no existing filters when the driver is started and none can be
defined.


>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>
> Conflicts:
> 	src/nwfilter/nwfilter_driver.c: Context. Code changed a bit
>         since 2013.

I don't know how useful this info is, since it's not an exact revert anyway.

ACK.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/nwfilter/nwfilter_driver.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
> index 8e3db43..1a868b6 100644
> --- a/src/nwfilter/nwfilter_driver.c
> +++ b/src/nwfilter/nwfilter_driver.c
> @@ -176,9 +176,6 @@ nwfilterStateInitialize(bool privileged,
>      char *base = NULL;
>      DBusConnection *sysbus = NULL;
>  
> -    if (!privileged)
> -        return 0;
> -
>      if (virDBusHasSystemBus() &&
>          !(sysbus = virDBusGetSystemBus()))
>          return -1;
> @@ -193,6 +190,9 @@ nwfilterStateInitialize(bool privileged,
>      driver->watchingFirewallD = (sysbus != NULL);
>      driver->privileged = privileged;
>  
> +    if (!privileged)
> +        return 0;
> +
>      nwfilterDriverLock();
>  
>      if (virNWFilterIPAddrMapInit() < 0)
> @@ -535,6 +535,12 @@ nwfilterDefineXML(virConnectPtr conn,
>      virNWFilterObjPtr nwfilter = NULL;
>      virNWFilterPtr ret = NULL;
>  
> +    if (!driver->privileged) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("Can't define NWFilters in session mode"));
> +        return NULL;
> +    }
> +
>      nwfilterDriverLock();
>      virNWFilterWriteLockFilterUpdates();
>      virNWFilterCallbackDriversLock();