[Merge] lp:~stgraber/upstart/upstart-initgroups into lp:upstart

[James Hunt]

Review: Needs Fixing

> > The first call to initgroups for user jobs looks fine, but the 2nd call for
> > system jobs (that specify setuid/setgid stanzas) has a few problems:
> >
> > (1) Shouldn't we only be calling this if we're root _and_ 'if (class->setuid
> > || class->setgid)' and then performing the password and group lookup for
> > job_setuid and job_setgid?
> 
> No, in the case where the root user is in additional groups, we need the initgroups call to populate the group list.

Ah - I did wonder about that. We've survived for a long time without calling initgroups for system jobs, but that appears to have been an oversight from way back :)

> 
> > (2) The checks on pwd and grp for near the 2nd call to initgroups are not
> > currently legitimate - pwd and grp should be initialised to NULL to avoid
> > undefined behaviour.
> 
> Oops, indeed, fixed.
> 
> > (3) You don't need those two errno resets just before the 2nd call since the
> > code never explicitly checks errno there.
> 
> Fixed.

The final change required is to update job_process_error_read() for the new JobProcessErrorTypes you've added. Once you've done this, please can you force such an error in a manual test, to ensure expected behaviour (currently, you should find that init will assert).

I've raised bug 1085836 to document the requirement to create a new test program which will embody all tests that need to be run as the root user.
-- 
https://code.launchpad.net/~stgraber/upstart/upstart-initgroups/+merge/136794
Your team Upstart Reviewers is subscribed to branch lp:upstart.