RFC: utility of 'orphansbuild' patch to mock-helper (BZ#221351)?

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Michael E Brown wrote:
> On Tue, Jul 10, 2007 at 01:13:33PM -0400, Mike Bonnet wrote:
>> On Tue, 2007-07-10 at 11:53 -0500, Clark Williams wrote:
>>> Jan Kratochvil has submitted a patch to mock that adds the 'orphanskill' command to
>>> mock-helper (a setuid root program used by mock). The patch traverses the /proc
>>> directory, looking for tasks with a "root" link that matches the chroot currently in
>>> use, and sends a SIGKILL to each matching task.
>>>
>>> As far as I can tell this is only useful to the GDB build. The testsuite for GDB
>>> seems to have some either abnormal terminations or so other oddity that leaves jobs
>>> hanging. I've looked at the C code and it looks well written, without obvious
>>> security holes.
>>>
>>> I've mixed feelings regarding adding the command. Michael and I have been fairly
>>> resistant to adding things to mock-helper, on the general principle that adding
>>> features to a setuid root program is fraught with peril. I see the utility of the
>>> code, but I'm torn as to whether the 'orphanskill' command is sufficiently useful to
>>> the general community.
>>>
>>> So, that's the question. Is 'orphanskill' worth adding to mock?
>> GDB is not the only build that leaves orphaned processes lying around.
>> I've seen similar behavior when building gcc, glibc, and mysql, to name
>> a few.  The problems are usually caused by test suites called during the
>> build process, and leaving them around after a build has completed (or
>> failed) can tie up system resources or in some cases cause subsequent
>> builds to fail.
>>
>> Just as mock cleans up the filesystem after a build, it should probably
>> be cleaning up the process list as well.  I'd be in favor of adding this
>> patch.  Koji could certainly make use of it.
> 
> It looks to me that, no matter the design of mock, we need to have this
> functionality.
> 
> As long as the patch looks reasonable, we probably need to include it
> for now, until we get to the point where we drop the suid binary.
> 

That makes sense. I've got Jan's patch and I'll apply it to the current head
(locally) and make sure it still works, then I'll push it to my public tree for you
guys to review.

Clark
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFGlAO0Hyuj/+TTEp0RAtpNAKC5K52QYeCDq/2kVFWixd5NPy5McACgvK3h
wVcqiMvrKdMjer3yL5RuPmM=
=pVg6
-----END PGP SIGNATURE-----