On 12/20/2013 09:34 AM, Daniel P. Berrange wrote:
> On Fri, Dec 20, 2013 at 08:24:41PM +0400, Reco wrote:
>> Implement virProcessRunInMountNamespace, which runs callback of type
>> virProcessNamespaceCallback in a container namespace.
>>
>> Hope it'll nail it this time.
Fails 'make syntax-check':
prohibit_fork_wrappers
src/util/virprocess.c:874: switch (cpid = fork()) {
maint.mk: use virCommand for child processes
make: *** [sc_prohibit_fork_wrappers] Error 1
>> +
>> + switch (cpid = fork()) {
>> + case 0:
>> + if (setns(fd, 0) == -1) {
>> + _exit(-1);
exit(-1) is wrong; the user will see $? of 255 (not -1), and it is not
our typical exit status for failure (where we normally want to be < 128
so that it is not confused with exit due to signal). Besides, I hate
magic numbers; better would be _exit(EXIT_FAILURE).
>> + default:
>> + if (virProcessWait(cpid, &status) < 0 || status < 0) {
No need to pass &status to virProcessWait() if you are going to insist
on 0 status anyways; better to pass NULL and let virProcessWait do the
validation on your behalf.
>> + virReportSystemError(errno,
>> + _("Callback failed with status %i"),
>> + status);
This overwrites the (better) error message that virProcessWait() already
generated.
>> + ret = 1;
This returns a number > 0 on failure, even though our normal conventions
is to return a negative number on failure.
>> +++ b/src/util/virprocess.h
>> @@ -60,4 +60,10 @@ int virProcessSetNamespaces(size_t nfdlist,
>> int virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes);
>> int virProcessSetMaxProcesses(pid_t pid, unsigned int procs);
>> int virProcessSetMaxFiles(pid_t pid, unsigned int files);
>> +
>> +typedef int (*virProcessNamespaceCallback)(pid_t pid, void *opaque);
Worth documenting that the return value of this callback is passed on to
_exit() (and thus that errors should be EXIT_FAILURE rather than our
typical -1 on failure).
I'll repost a full v5 series with my proposed fixes to these issues in a
separate thread, and wait for a review (since this fixes a CVE, it needs
a second set of eyes).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list