[libvirt] [PATCH 0/6] Couple of important fixes
I think the most important fix is 2/6 as it might touch live user data. Also, I haven't run 'make check-access' in a while. If I did I would have noticed new paths being accessed. I'll try to update white list (in a separate patch set) and hopefully we can then enable by default and threat its errors as fatal. Michal Prívozník (6): qemuBuildSCSIiSCSIHostdevDrvStr: Don't leak @netsource and @srcprops qemuxml2argvtest: Don't spawn session daemon Forget last daemon/ dir artefacts virtestmock: Track connect() too check-file-access: Allow specifying action virtestmock: Track action Makefile.am | 2 +- run.in | 2 +- src/driver.h | 2 +- src/qemu/qemu_command.c | 6 ++--- tests/check-file-access.pl | 32 ++++++++++++++++++---- tests/file_access_whitelist.txt | 15 +++++++---- tests/libvirtd-fail | 4 +-- tests/qemuxml2argvmock.c | 7 +++++ tests/virtestmock.c | 59 ++++++++++++++++++++++++++++++----------- 9 files changed, 95 insertions(+), 34 deletions(-) -- 2.16.4
After 6b770f9a3bdabb1 both @netsource and @srcprops are leaked because of early return introduced in the commit. ==1812== 644 bytes in 4 blocks are definitely lost in loss record 835 of 885 ==1812== at 0x4C2F12F: realloc (vg_replace_malloc.c:785) ==1812== by 0x8846393: xmlSaveUriRealloc (in /usr/lib64/libxml2.so.2.9.8) ==1812== by 0x8846B1C: xmlSaveUri (in /usr/lib64/libxml2.so.2.9.8) ==1812== by 0x5DDA619: virURIFormat (viruri.c:256) ==1812== by 0x56E941B: qemuBuildNetworkDriveURI (qemu_command.c:781) ==1812== by 0x56E979A: qemuBuildNetworkDriveStr (qemu_command.c:859) ==1812== by 0x56F3A0B: qemuBuildSCSIiSCSIHostdevDrvStr (qemu_command.c:4664) ==1812== by 0x56F3D1F: qemuBuildSCSIHostdevDrvStr (qemu_command.c:4732) ==1812== by 0x56F57F7: qemuBuildHostdevCommandLine (qemu_command.c:5337) ==1812== by 0x570303A: qemuBuildCommandLine (qemu_command.c:10376) ==1812== by 0x57604EE: qemuProcessCreatePretendCmd (qemu_process.c:6649) ==1812== by 0x11352A: testCompareXMLToArgv (qemuxml2argvtest.c:566) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 32eb59b6ab..86970f3d91 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4644,8 +4644,8 @@ static char * qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps) { + char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *netsource = NULL; virJSONValuePtr srcprops = NULL; virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; @@ -4672,13 +4672,13 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, if (virBufferCheckError(&buf) < 0) goto cleanup; - return virBufferContentAndReset(&buf); + ret = virBufferContentAndReset(&buf); cleanup: VIR_FREE(netsource); virJSONValueFree(srcprops); virBufferFreeAndReset(&buf); - return NULL; + return ret; } char * -- 2.16.4
On Mon, Jul 09, 2018 at 11:56:09 +0200, Michal Privoznik wrote:
After 6b770f9a3bdabb1 both @netsource and @srcprops are leaked because of early return introduced in the commit.
I'd revert this commit completely. The change seems pointless to me.
==1812== 644 bytes in 4 blocks are definitely lost in loss record 835 of 885 ==1812== at 0x4C2F12F: realloc (vg_replace_malloc.c:785) ==1812== by 0x8846393: xmlSaveUriRealloc (in /usr/lib64/libxml2.so.2.9.8)
On Mon, Jul 09, 2018 at 12:54:41 +0200, Peter Krempa wrote:
On Mon, Jul 09, 2018 at 11:56:09 +0200, Michal Privoznik wrote:
After 6b770f9a3bdabb1 both @netsource and @srcprops are leaked because of early return introduced in the commit.
I'd revert this commit completely. The change seems pointless to me.
Oh, disregard this. The comit seems pointless but is a preparation for a bugfix.
==1812== 644 bytes in 4 blocks are definitely lost in loss record 835 of 885 ==1812== at 0x4C2F12F: realloc (vg_replace_malloc.c:785) ==1812== by 0x8846393: xmlSaveUriRealloc (in /usr/lib64/libxml2.so.2.9.8)
ACK to this patch then.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
After f14c37ce4c2ccd111 the cleanup path for qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() tries to connect to nwfilter driver in order to tear down any NWFilter that was brought up during cmd line construction. Since we also have negative test cases where errors during cmd line build are expected the cleanup paths are executed and NWFilter removal is attempted. Fortunately, there is another bug that by pure luck prevented us from actually spawning the daemon and thus modifying actual user data. See next commit for explanation. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/driver.h | 2 +- tests/qemuxml2argvmock.c | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/driver.h b/src/driver.h index 0b1f7a2269..0a0d8facee 100644 --- a/src/driver.h +++ b/src/driver.h @@ -113,7 +113,7 @@ int virDriverLoadModule(const char *name, virConnectPtr virGetConnectInterface(void); virConnectPtr virGetConnectNetwork(void); -virConnectPtr virGetConnectNWFilter(void); +virConnectPtr virGetConnectNWFilter(void) ATTRIBUTE_NOINLINE; virConnectPtr virGetConnectNodeDev(void); virConnectPtr virGetConnectSecret(void); virConnectPtr virGetConnectStorage(void); diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 4df92cf396..13ccfb855d 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -233,3 +233,10 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev ATTRIBUTE_UNUSED) abort(); return 1729; } + + +virConnectPtr +virGetConnectNWFilter(void) +{ + return NULL; +} -- 2.16.4
On Mon, Jul 09, 2018 at 11:56:10AM +0200, Michal Privoznik wrote:
After f14c37ce4c2ccd111 the cleanup path for qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() tries to connect to nwfilter driver in order to tear down any NWFilter that was brought up during cmd line construction. Since we also have negative test cases where errors during cmd line build are expected the cleanup paths are executed and NWFilter removal is attempted.
Fortunately, there is another bug that by pure luck prevented us from actually spawning the daemon and thus modifying actual user data. See next commit for explanation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/driver.h | 2 +- tests/qemuxml2argvmock.c | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/driver.h b/src/driver.h index 0b1f7a2269..0a0d8facee 100644 --- a/src/driver.h +++ b/src/driver.h @@ -113,7 +113,7 @@ int virDriverLoadModule(const char *name,
virConnectPtr virGetConnectInterface(void); virConnectPtr virGetConnectNetwork(void); -virConnectPtr virGetConnectNWFilter(void); +virConnectPtr virGetConnectNWFilter(void) ATTRIBUTE_NOINLINE; virConnectPtr virGetConnectNodeDev(void); virConnectPtr virGetConnectSecret(void); virConnectPtr virGetConnectStorage(void); diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 4df92cf396..13ccfb855d 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -233,3 +233,10 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev ATTRIBUTE_UNUSED) abort(); return 1729; } + + +virConnectPtr +virGetConnectNWFilter(void) +{ + return NULL; +}
In qemuxml2argvtest.c we actally set a fake shared connection, but I only set it for two of the drivers. We should just register it for all the drivers. eg expand these lines: virSetConnectSecret(conn); virSetConnectStorage(conn); Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 07/09/2018 12:44 PM, Daniel P. Berrangé wrote:
On Mon, Jul 09, 2018 at 11:56:10AM +0200, Michal Privoznik wrote:
After f14c37ce4c2ccd111 the cleanup path for qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() tries to connect to nwfilter driver in order to tear down any NWFilter that was brought up during cmd line construction. Since we also have negative test cases where errors during cmd line build are expected the cleanup paths are executed and NWFilter removal is attempted.
Fortunately, there is another bug that by pure luck prevented us from actually spawning the daemon and thus modifying actual user data. See next commit for explanation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/driver.h | 2 +- tests/qemuxml2argvmock.c | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/driver.h b/src/driver.h index 0b1f7a2269..0a0d8facee 100644 --- a/src/driver.h +++ b/src/driver.h @@ -113,7 +113,7 @@ int virDriverLoadModule(const char *name,
virConnectPtr virGetConnectInterface(void); virConnectPtr virGetConnectNetwork(void); -virConnectPtr virGetConnectNWFilter(void); +virConnectPtr virGetConnectNWFilter(void) ATTRIBUTE_NOINLINE; virConnectPtr virGetConnectNodeDev(void); virConnectPtr virGetConnectSecret(void); virConnectPtr virGetConnectStorage(void); diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 4df92cf396..13ccfb855d 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -233,3 +233,10 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev ATTRIBUTE_UNUSED) abort(); return 1729; } + + +virConnectPtr +virGetConnectNWFilter(void) +{ + return NULL; +}
In qemuxml2argvtest.c we actally set a fake shared connection, but I only set it for two of the drivers. We should just register it for all the drivers. eg expand these lines:
virSetConnectSecret(conn); virSetConnectStorage(conn);
Oh, that means I have to provide some basic implementation. I can't just set conn->nwFilterDriver = NULL; because then virGetConnectNWFilte() would still try to connect. Okay, I think I can do it. Michal
On Mon, Jul 09, 2018 at 02:00:06PM +0200, Michal Privoznik wrote:
On 07/09/2018 12:44 PM, Daniel P. Berrangé wrote:
On Mon, Jul 09, 2018 at 11:56:10AM +0200, Michal Privoznik wrote:
After f14c37ce4c2ccd111 the cleanup path for qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() tries to connect to nwfilter driver in order to tear down any NWFilter that was brought up during cmd line construction. Since we also have negative test cases where errors during cmd line build are expected the cleanup paths are executed and NWFilter removal is attempted.
Fortunately, there is another bug that by pure luck prevented us from actually spawning the daemon and thus modifying actual user data. See next commit for explanation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/driver.h | 2 +- tests/qemuxml2argvmock.c | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/driver.h b/src/driver.h index 0b1f7a2269..0a0d8facee 100644 --- a/src/driver.h +++ b/src/driver.h @@ -113,7 +113,7 @@ int virDriverLoadModule(const char *name,
virConnectPtr virGetConnectInterface(void); virConnectPtr virGetConnectNetwork(void); -virConnectPtr virGetConnectNWFilter(void); +virConnectPtr virGetConnectNWFilter(void) ATTRIBUTE_NOINLINE; virConnectPtr virGetConnectNodeDev(void); virConnectPtr virGetConnectSecret(void); virConnectPtr virGetConnectStorage(void); diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 4df92cf396..13ccfb855d 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -233,3 +233,10 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev ATTRIBUTE_UNUSED) abort(); return 1729; } + + +virConnectPtr +virGetConnectNWFilter(void) +{ + return NULL; +}
In qemuxml2argvtest.c we actally set a fake shared connection, but I only set it for two of the drivers. We should just register it for all the drivers. eg expand these lines:
virSetConnectSecret(conn); virSetConnectStorage(conn);
Oh, that means I have to provide some basic implementation. I can't just set conn->nwFilterDriver = NULL; because then virGetConnectNWFilte() would still try to connect.
You shouldn't have to provide any impl - virGetConnectNWFilter() merely cares about the virConnectPtr being non-NULL - it doesn't check if the conn->nwfilterDriver pointer is set. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 07/09/2018 02:05 PM, Daniel P. Berrangé wrote:
On Mon, Jul 09, 2018 at 02:00:06PM +0200, Michal Privoznik wrote:
On 07/09/2018 12:44 PM, Daniel P. Berrangé wrote:
On Mon, Jul 09, 2018 at 11:56:10AM +0200, Michal Privoznik wrote:
In qemuxml2argvtest.c we actally set a fake shared connection, but I only set it for two of the drivers. We should just register it for all the drivers. eg expand these lines:
virSetConnectSecret(conn); virSetConnectStorage(conn);
Oh, that means I have to provide some basic implementation. I can't just set conn->nwFilterDriver = NULL; because then virGetConnectNWFilte() would still try to connect.
You shouldn't have to provide any impl - virGetConnectNWFilter() merely cares about the virConnectPtr being non-NULL - it doesn't check if the conn->nwfilterDriver pointer is set.
Well, in that case there are couple of errors reported (not sure why they are being printed into stderr rather than respecting log settings?) libvirt.git/tests $ VIR_TEST_RANGE=460 ./qemuxml2argvtest TEST: qemuxml2argvtest libvirt: Network Filter Driver error : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev libvirt: Network Filter Driver error : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev libvirt: Network Filter Driver error : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev . 1602 OK Michal
The most important part is LIBVIRTD_PATH env var fix. It is used in virFileFindResourceFull() from tests. The libvirtd no longer lives under daemon/. Then, libvirtd-fail test was still failing (as expected) but not because of missing config file but because it was trying to execute (nonexistent) top_builddir/daemon/libvirtd which fulfilled expected outcome and thus test did not fail. Thirdly, lcov was told to generate coverage for daemon/ dir too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Makefile.am | 2 +- run.in | 2 +- tests/libvirtd-fail | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index 1926e21b7a..709064c6a6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -80,7 +80,7 @@ check-access: cov: clean-cov $(MKDIR_P) $(top_builddir)/coverage $(LCOV) -c -o $(top_builddir)/coverage/libvirt.info.tmp \ - -d $(top_builddir)/src -d $(top_builddir)/daemon \ + -d $(top_builddir)/src \ -d $(top_builddir)/tests $(LCOV) -r $(top_builddir)/coverage/libvirt.info.tmp \ -o $(top_builddir)/coverage/libvirt.info diff --git a/run.in b/run.in index cbef61a674..06ad54b62b 100644 --- a/run.in +++ b/run.in @@ -63,7 +63,7 @@ export PKG_CONFIG_PATH export LIBVIRT_DRIVER_DIR="$b/src/.libs" export LIBVIRT_LOCK_MANAGER_PLUGIN_DIR="$b/src/.libs" export VIRTLOCKD_PATH="$b/src" -export LIBVIRTD_PATH="$b/daemon" +export LIBVIRTD_PATH="$b/src" # This is a cheap way to find some use-after-free and uninitialized # read problems when using glibc. diff --git a/tests/libvirtd-fail b/tests/libvirtd-fail index 6c61b892cb..f9e927b61f 100755 --- a/tests/libvirtd-fail +++ b/tests/libvirtd-fail @@ -5,12 +5,12 @@ if test "$VERBOSE" = yes; then set -x - $abs_top_builddir/daemon/libvirtd --version + $abs_top_builddir/src/libvirtd --version fi fail=0 -$abs_top_builddir/daemon/libvirtd --config=no-such-conf --timeout=5 2> log +$abs_top_builddir/src/libvirtd --config=no-such-conf --timeout=5 2> log RET=$? test "$RET" != "0" && exit 0 || exit 1 -- 2.16.4
The aim of this mock is to track if a test doesn't touch anything in live system. Well, connect() definitely falls into that category. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virtestmock.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/virtestmock.c b/tests/virtestmock.c index 9b91adec77..654af24a10 100644 --- a/tests/virtestmock.c +++ b/tests/virtestmock.c @@ -27,6 +27,10 @@ #include <execinfo.h> #include <sys/file.h> #include <sys/stat.h> +#include <sys/socket.h> +#ifdef HAVE_SYS_UN_H +# include <sys/un.h> +#endif #include "internal.h" #include "configmake.h" @@ -61,6 +65,7 @@ static int (*real_lstat)(const char *path, struct stat *sb); static int (*real_lstat64)(const char *path, void *sb); static int (*real___lxstat)(int ver, const char *path, struct stat *sb); static int (*real___lxstat64)(int ver, const char *path, void *sb); +static int (*real_connect)(int fd, const struct sockaddr *addr, socklen_t addrlen); static const char *progname; const char *output; @@ -79,6 +84,7 @@ static void init_syms(void) VIR_MOCK_REAL_INIT_ALT(stat64, __xstat64); VIR_MOCK_REAL_INIT_ALT(lstat, __lxstat); VIR_MOCK_REAL_INIT_ALT(lstat64, __lxstat64); + VIR_MOCK_REAL_INIT(connect); } static void @@ -321,3 +327,19 @@ __lxstat64(int ver, const char *path, struct stat64 *sb) return real___lxstat64(ver, path, sb); } #endif + + +int connect(int sockfd, const struct sockaddr *addr, socklen_t addrlen) +{ + init_syms(); + +#ifdef HAVE_SYS_UN_H + if (addrlen == sizeof(struct sockaddr_un)) { + struct sockaddr_un *tmp = (struct sockaddr_un *) addr; + if (tmp->sun_family == AF_UNIX) + checkPath(tmp->sun_path); + } +#endif + + return real_connect(sockfd, addr, addrlen); +} -- 2.16.4
The check-file-access.pl script is used to match access list generated by virtestmock against whitelisted rules stored in file_access_whitelist.txt. So far the rules are in form: $path: $progname: $testname This is not sufficient because the rule does not take into account 'action' that caused $path to appear in the list of accessed files. After this commit the rule can be in new form: $path: $action: $progname: $testname where $action is one from ("open", "fopen", "access", "stat", "lstat", "connect"). This way the white list can be fine tuned to allow say access() but not connect(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/check-file-access.pl | 32 +++++++++++++++++++++++++++----- tests/file_access_whitelist.txt | 15 ++++++++++----- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/tests/check-file-access.pl b/tests/check-file-access.pl index 977a2bc533..ea0b7a18a2 100755 --- a/tests/check-file-access.pl +++ b/tests/check-file-access.pl @@ -27,18 +27,21 @@ use warnings; my $access_file = "test_file_access.txt"; my $whitelist_file = "file_access_whitelist.txt"; +my @known_actions = ("open", "fopen", "access", "stat", "lstat", "connect"); + my @files; my @whitelist; open FILE, "<", $access_file or die "Unable to open $access_file: $!"; while (<FILE>) { chomp; - if (/^(\S*):\s*(\S*)(\s*:\s*(.*))?$/) { + if (/^(\S*):\s*(\S*):\s*(\S*)(\s*:\s*(.*))?$/) { my %rec; ${rec}{path} = $1; - ${rec}{progname} = $2; - if (defined $4) { - ${rec}{testname} = $4; + ${rec}{action} = $2; + ${rec}{progname} = $3; + if (defined $5) { + ${rec}{testname} = $5; } push (@files, \%rec); } else { @@ -52,7 +55,21 @@ while (<FILE>) { chomp; if (/^\s*#.*$/) { # comment + } elsif (/^(\S*):\s*(\S*)(:\s*(\S*)(\s*:\s*(.*))?)?$/ and + grep /^$2$/, @known_actions) { + # $path: $action: $progname: $testname + my %rec; + ${rec}{path} = $1; + ${rec}{action} = $3; + if (defined $4) { + ${rec}{progname} = $4; + } + if (defined $6) { + ${rec}{testname} = $6; + } + push (@whitelist, \%rec); } elsif (/^(\S*)(:\s*(\S*)(\s*:\s*(.*))?)?$/) { + # $path: $progname: $testname my %rec; ${rec}{path} = $1; if (defined $3) { @@ -79,6 +96,11 @@ for my $file (@files) { next; } + if (defined %${rule}{action} and + not %${file}{action} =~ m/^$rule->{action}$/) { + next; + } + if (defined %${rule}{progname} and not %${file}{progname} =~ m/^$rule->{progname}$/) { next; @@ -95,7 +117,7 @@ for my $file (@files) { if (not $match) { $error = 1; - print "$file->{path}: $file->{progname}"; + print "$file->{path}: $file->{action}: $file->{progname}"; print ": $file->{testname}" if defined %${file}{testname}; print "\n"; } diff --git a/tests/file_access_whitelist.txt b/tests/file_access_whitelist.txt index 850b28506e..3fb318cbab 100644 --- a/tests/file_access_whitelist.txt +++ b/tests/file_access_whitelist.txt @@ -1,14 +1,17 @@ # This is a whitelist that allows accesses to files not in our # build directory nor source directory. The records are in the -# following format: +# following formats: # # $path: $progname: $testname +# $path: $action: $progname: $testname # -# All these three are evaluated as perl RE. So to allow /dev/sda -# and /dev/sdb, you can just '/dev/sd[a-b]', or to allow +# All these variables are evaluated as perl RE. So to allow +# /dev/sda and /dev/sdb, you can just '/dev/sd[a-b]', or to allow # /proc/$pid/status you can '/proc/\d+/status' and so on. -# Moreover, $progname and $testname can be empty, in which which -# case $path is allowed for all tests. +# Moreover, $action, $progname and $testname can be empty, in which +# which case $path is allowed for all tests. However, $action (if +# specified) must be one of "open", "fopen", "access", "stat", +# "lstat", "connect". /bin/cat: sysinfotest /bin/dirname: sysinfotest: x86 sysinfo @@ -19,5 +22,7 @@ /etc/hosts /proc/\d+/status +/etc/passwd: fopen + # This is just a dummy example, DO NOT USE IT LIKE THAT! .*: nonexistent-test-touching-everything -- 2.16.4
As advertised in the previous commit, we need the list of accessed files to also contain action that caused the $path to appear on the list. Not only this enables us to fine tune our white list rules it also helps us to see why $path is reported. For instance: /run/user/1000/libvirt/libvirt-sock: connect: qemuxml2argvtest: QEMU XML-2-ARGV net-vhostuser-multiq Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/virtestmock.c | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/tests/virtestmock.c b/tests/virtestmock.c index 654af24a10..25aadf8aea 100644 --- a/tests/virtestmock.c +++ b/tests/virtestmock.c @@ -88,7 +88,8 @@ static void init_syms(void) } static void -printFile(const char *file) +printFile(const char *file, + const char *func) { FILE *fp; const char *testname = getenv("VIR_TEST_MOCK_TESTNAME"); @@ -116,9 +117,9 @@ printFile(const char *file) } /* Now append the following line into the output file: - * $file: $progname $testname */ + * $file: $progname: $func: $testname */ - fprintf(fp, "%s: %s", file, progname); + fprintf(fp, "%s: %s: %s", file, func, progname); if (testname) fprintf(fp, ": %s", testname); @@ -128,8 +129,12 @@ printFile(const char *file) fclose(fp); } +#define CHECK_PATH(path) \ + checkPath(path, __FUNCTION__) + static void -checkPath(const char *path) +checkPath(const char *path, + const char *func) { char *fullPath = NULL; char *relPath = NULL; @@ -160,7 +165,7 @@ checkPath(const char *path) if (!STRPREFIX(path, abs_topsrcdir) && !STRPREFIX(path, abs_topbuilddir)) { - printFile(path); + printFile(path, func); } VIR_FREE(crippledPath); @@ -180,7 +185,7 @@ int open(const char *path, int flags, ...) init_syms(); - checkPath(path); + CHECK_PATH(path); if (flags & O_CREAT) { va_list ap; @@ -199,7 +204,7 @@ FILE *fopen(const char *path, const char *mode) { init_syms(); - checkPath(path); + CHECK_PATH(path); return real_fopen(path, mode); } @@ -209,7 +214,7 @@ int access(const char *path, int mode) { init_syms(); - checkPath(path); + CHECK_PATH(path); return real_access(path, mode); } @@ -239,7 +244,7 @@ int stat(const char *path, struct stat *sb) { init_syms(); - checkPath(path); + checkPath(path, "stat"); return real_stat(path, sb); } @@ -250,7 +255,7 @@ int stat64(const char *path, struct stat64 *sb) { init_syms(); - checkPath(path); + checkPath(path, "stat"); return real_stat64(path, sb); } @@ -262,7 +267,7 @@ __xstat(int ver, const char *path, struct stat *sb) { init_syms(); - checkPath(path); + checkPath(path, "stat"); return real___xstat(ver, path, sb); } @@ -274,7 +279,7 @@ __xstat64(int ver, const char *path, struct stat64 *sb) { init_syms(); - checkPath(path); + checkPath(path, "stat"); return real___xstat64(ver, path, sb); } @@ -286,7 +291,7 @@ lstat(const char *path, struct stat *sb) { init_syms(); - checkPath(path); + checkPath(path, "lstat"); return real_lstat(path, sb); } @@ -298,7 +303,7 @@ lstat64(const char *path, struct stat64 *sb) { init_syms(); - checkPath(path); + checkPath(path, "lstat"); return real_lstat64(path, sb); } @@ -310,7 +315,7 @@ __lxstat(int ver, const char *path, struct stat *sb) { init_syms(); - checkPath(path); + checkPath(path, "lstat"); return real___lxstat(ver, path, sb); } @@ -322,7 +327,7 @@ __lxstat64(int ver, const char *path, struct stat64 *sb) { init_syms(); - checkPath(path); + checkPath(path, "lstat"); return real___lxstat64(ver, path, sb); } @@ -337,7 +342,7 @@ int connect(int sockfd, const struct sockaddr *addr, socklen_t addrlen) if (addrlen == sizeof(struct sockaddr_un)) { struct sockaddr_un *tmp = (struct sockaddr_un *) addr; if (tmp->sun_family == AF_UNIX) - checkPath(tmp->sun_path); + CHECK_PATH(tmp->sun_path); } #endif -- 2.16.4
participants (3)
-
Daniel P. Berrangé -
Michal Privoznik -
Peter Krempa