From nobody Sun May 5 18:04:52 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1505906839421230.68073567557633; Wed, 20 Sep 2017 04:27:19 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2EDF5C053FA2; Wed, 20 Sep 2017 11:27:17 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id ACDCB60BE2; Wed, 20 Sep 2017 11:27:16 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id ABF621855944; Wed, 20 Sep 2017 11:27:15 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v8KBBmcl017522 for ; Wed, 20 Sep 2017 07:11:48 -0400 Received: by smtp.corp.redhat.com (Postfix) id 471C66046B; Wed, 20 Sep 2017 11:11:48 +0000 (UTC) Received: from lettuce.camlab.fab.redhat.com (dhcp-42.gsslab.fab.redhat.com [10.33.9.42]) by smtp.corp.redhat.com (Postfix) with ESMTP id B82866017B; Wed, 20 Sep 2017 11:11:44 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 2EDF5C053FA2 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com From: "Daniel P. Berrange" To: libvir-list@redhat.com Date: Wed, 20 Sep 2017 12:08:59 +0100 Message-Id: <1505905739-30542-1-git-send-email-berrange@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH] Stop linking tests/commandhelper to libvirt code X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 20 Sep 2017 11:27:17 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" The commandhelper binary is a helper for commandtest that validates what file handles were inherited. For this to work reliably we must not have any libraries that leak file descriptors into commandhelper. Unfortunately some versions of gnutls will intentionally open file handles at library load time via a constructor function. We previously hacked around this in commit 4cbc15d037e1cd8abf5c4aa6acc30d83ae13e34d Author: Martin Kletzander Date: Fri May 2 09:55:52 2014 +0200 tests: don't fail with newer gnutls gnutls-3.3.0 and newer leaves 2 FDs open in order to be backwards compatible when it comes to chrooted binaries [1]. Linking commandhelper with gnutls then leaves these two FDs open and commandtest fails thanks to that. This patch does not link commandhelper with libvirt.la, but rather only the utilities making the test pass. Based on suggestion from Daniel [2]. [1] http://lists.gnutls.org/pipermail/gnutls-help/2014-April/003429.html [2] https://www.redhat.com/archives/libvir-list/2014-April/msg01119.html That fix relied on fact that while libvirt.so linked with gnutls, libvirt_util.la did not link to it. With the introduction of the util/vircrypto.c file that assumption is no longer valid. We must not link to libvirt_util.la at all - only gnulib and libc can (hopefully) be relied on not to open random file descriptors in constructors. Signed-off-by: Daniel P. Berrange Reviewed-by: Martin Kletzander --- cfg.mk | 8 ++++---- tests/Makefile.am | 6 ++++-- tests/commandhelper.c | 29 +++++++++++++---------------- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/cfg.mk b/cfg.mk index 56cb14b..f2a053f 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1111,7 +1111,7 @@ $(srcdir)/src/admin/admin_client.h: $(srcdir)/src/adm= in/admin_protocol.x exclude_file_name_regexp--sc_avoid_strcase =3D ^tools/vsh\.h$$ =20 _src1=3Dlibvirt-stream|qemu/qemu_monitor|util/vir(command|file|fdstream)|x= en/xend_internal|rpc/virnetsocket|lxc/lxc_controller|locking/lock_daemon|lo= gging/log_daemon -_test1=3Dshunloadtest|virnettlscontexttest|virnettlssessiontest|vircgroupm= ock +_test1=3Dshunloadtest|virnettlscontexttest|virnettlssessiontest|vircgroupm= ock|commandhelper exclude_file_name_regexp--sc_avoid_write =3D \ ^(src/($(_src1))|daemon/libvirtd|tools/virsh-console|tests/($(_test1)))\= .c$$ =20 @@ -1146,10 +1146,10 @@ exclude_file_name_regexp--sc_prohibit_asprintf =3D \ ^(cfg\.mk|bootstrap.conf$$|examples/|src/util/virstring\.[ch]$$|tests/vi= rcgroupmock\.c$$) =20 exclude_file_name_regexp--sc_prohibit_strdup =3D \ - ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup= )mock.c$$) + ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup= )mock.c|tests/commandhelper\.c$$) =20 exclude_file_name_regexp--sc_prohibit_close =3D \ - (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\= .c|tests/vir.+mock\.c)$$) + (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\= .c|tests/vir.+mock\.c|tests/commandhelper\.c)$$) =20 exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF =3D \ (^tests/(qemuhelp|virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/= .*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) @@ -1173,7 +1173,7 @@ exclude_file_name_regexp--sc_prohibit_select =3D \ ^cfg\.mk$$ =20 exclude_file_name_regexp--sc_prohibit_raw_allocation =3D \ - ^(docs/hacking\.html\.in|src/util/viralloc\.[ch]|examples/.*|tests/(secu= rityselinuxhelper|(vircgroup|nss)mock)\.c|tools/wireshark/src/packet-libvir= t\.c)$$ + ^(docs/hacking\.html\.in|src/util/viralloc\.[ch]|examples/.*|tests/(secu= rityselinuxhelper|(vircgroup|nss)mock|commandhelper)\.c|tools/wireshark/src= /packet-libvirt\.c)$$ =20 exclude_file_name_regexp--sc_prohibit_readlink =3D \ ^src/(util/virutil|lxc/lxc_container)\.c$$ diff --git a/tests/Makefile.am b/tests/Makefile.am index 8138885..0b2305d 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -946,12 +946,14 @@ commandtest_SOURCES =3D \ commandtest.c testutils.h testutils.c commandtest_LDADD =3D $(LDADDS) =20 +# Must not link to any libvirt modules - libc / gnulib only +# otherwise external libraries might unexpectedly leak +# file descriptors into commandhelper invalidating the +# test logic assumptions commandhelper_SOURCES =3D \ commandhelper.c commandhelper_LDADD =3D \ $(NO_INDIRECT_LDFLAGS) \ - $(PROBES_O) \ - ../src/libvirt_util.la \ $(GNULIB_LIBS) =20 commandhelper_LDFLAGS =3D -static diff --git a/tests/commandhelper.c b/tests/commandhelper.c index 015efda..e9e6353 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -28,11 +28,6 @@ #include =20 #include "internal.h" -#include "virutil.h" -#include "viralloc.h" -#include "virfile.h" -#include "testutils.h" -#include "virstring.h" =20 #ifndef WIN32 =20 @@ -50,11 +45,13 @@ static int envsort(const void *a, const void *b) char *bkey; int ret; =20 - ignore_value(VIR_STRNDUP_QUIET(akey, astr, aeq - astr)); - ignore_value(VIR_STRNDUP_QUIET(bkey, bstr, beq - bstr)); + if (!(akey =3D strndup(astr, aeq - astr))) + abort(); + if (!(bkey =3D strndup(bstr, beq - bstr))) + abort(); ret =3D strcmp(akey, bkey); - VIR_FREE(akey); - VIR_FREE(bkey); + free(akey); + free(bkey); return ret; } =20 @@ -80,8 +77,8 @@ int main(int argc, char **argv) { origenv++; } =20 - if (VIR_ALLOC_N_QUIET(newenv, n) < 0) - goto cleanup; + if (!(newenv =3D malloc(sizeof(*newenv) * n))) + abort(); =20 origenv =3D environ; n =3D i =3D 0; @@ -120,7 +117,7 @@ int main(int argc, char **argv) { STREQ(cwd + strlen(cwd) - strlen("/commanddata"), "/commanddata")) strcpy(cwd, ".../commanddata"); fprintf(log, "CWD:%s\n", cwd); - VIR_FREE(cwd); + free(cwd); =20 fprintf(log, "UMASK:%04o\n", umask(0)); =20 @@ -144,9 +141,9 @@ int main(int argc, char **argv) { goto cleanup; if (got =3D=3D 0) break; - if (safewrite(STDOUT_FILENO, buf, got) !=3D got) + if (write(STDOUT_FILENO, buf, got) !=3D got) goto cleanup; - if (safewrite(STDERR_FILENO, buf, got) !=3D got) + if (write(STDERR_FILENO, buf, got) !=3D got) goto cleanup; } =20 @@ -158,8 +155,8 @@ int main(int argc, char **argv) { ret =3D EXIT_SUCCESS; =20 cleanup: - VIR_FORCE_FCLOSE(log); - VIR_FREE(newenv); + fclose(log); + free(newenv); return ret; } =20 --=20 2.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list