[ovs-dev] [PATCH V3] rhel: Fix support for root user using DPDK

[Ben Pfaff]

OK, I understand now that the goal is that passing "--user $UID" should
not call setuid() or setgid().  In that case, though, why pass the
--user option at all?  I believe that, with this patch, "--user $UID"
and "" yield equivalent behavior.

Thanks,

Ben.

On Thu, Feb 01, 2018 at 09:34:12PM -0200, Marcos Felipe Schwarz wrote:
> Hi Ben,
> 
> I agree that the purpose of the canges was not cleared in the patch. The
> full discussion is at
> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-January/046061.html.
> But in a nutshell, this change is a fancy way to avoid the actual problem,
> that's why it's not intuitive. The actual problem is the current method to
> change the ovs user at (daemon_become_new_user_linux), drops the
> CAP_SYS_ADMIN capability, even when you are "changing" from root to itself,
> which occurs in rhel based distros when you use OVS_USER="root:toot". My
> PoC solution [2] was to force the CAP_SYS_ADMIN during the user change, but
> this would give more permission to openvswitch process than necessary on
> most of the situations, for instance when DPDK and UIO are not required.
> The fix in the current patch was proposed by Aaron [3], which involves to
> avoid changing the user when it is already the desired user (and group),
> this fixes the problem without the downsides of the initial solution and
> avoids unnecessary calls.
> 
> [2]
> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-January/045959.html
> [3]
> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-January/045973.html
> Regards,
> 
> Marcos Schwarz
> 
> On Thu, Feb 1, 2018 at 7:27 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> > On Thu, Feb 01, 2018 at 03:42:03PM -0200, Marcos Felipe Schwarz wrote:
> > >  Since 2.8.0 OVS runs as non-root user on rhel distros, but the current
> > > implementation breaks the ability to run as root with DPDK and as a
> > > consequence there is no way possible to use UIO drivers on kernel 4.0 and
> > > newer [1, 2].
> > > [1] http://dpdk.org/browse/dpdk/commit/?id=cdc242f260e766
> > > bd95a658b5e0686a62ec04f5b0
> > > [2] https://www.kernel.org/doc/Documentation/vm/pagemap.txt
> > >
> > > Fixes: e3e738a3d058 ("redhat: allow dpdk to also run as non-root user")
> > > Signed-off-by: Marcos Schwarz <marcos.f.sch at gmail.com>
> > > Acked-by: Aaron Conole <aconole at redhat.com>
> >
> > Thanks for the patch.
> >
> > I understand from the commit message the ultimate goal of this patch.  I
> > don't yet understand what change it is actually making.  In particular,
> > what is the purpose of the following change; what does it do?
> >
> > > --- a/lib/daemon-unix.c
> > > +++ b/lib/daemon-unix.c
> > > @@ -1047,5 +1047,6 @@ daemon_set_new_user(const char *user_spec)
> > >          }
> > >      }
> > >
> > > -    switch_user = true;
> > > +    if (!uid_verify(uid) || !gid_verify(gid))
> > > +        switch_user = true;
> > >  }
> >
> > Thanks,
> >
> > Ben.
> >