[libvirt] [PATCH] tests: don't abort in fopen(/proc/mounts)
The mock fopen() function will abort if "/proc/mounts" is requested with "r" permissions and VIR_CGROUP_MOCK_FILENAME env var is not set. Unfortunately this is triggering by the libselinux library constructor when it tries to read /proc/mounts to find out if selinuxfs is mounted in an unusual place. This, however, only affects libselinux in Debian as that opens with "r", while in Fedora / RHEL it opens "re" and thus luckily never triggered the abort(), instead getting an EACCESS. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/vircgroupmock.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index 06bd0a5f29..9c67a44b0d 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -460,8 +460,10 @@ FILE *fopen(const char *path, const char *mode) } if (type) { - if (!filename) - abort(); + if (!filename) { + errno = EACCES; + return NULL; + } if (virAsprintfQuiet(&filepath, "%s/vircgroupdata/%s.%s", abs_srcdir, filename, type) < 0) { abort(); -- 2.20.1
On 3/27/19 11:31 AM, Daniel P. Berrangé wrote:
The mock fopen() function will abort if "/proc/mounts" is requested with "r" permissions and VIR_CGROUP_MOCK_FILENAME env var is not set.
Unfortunately this is triggering by the libselinux library constructor when it tries to read /proc/mounts to find out if selinuxfs is mounted in an unusual place.
This, however, only affects libselinux in Debian as that opens with "r", while in Fedora / RHEL it opens "re" and thus luckily never triggered the abort(), instead getting an EACCESS.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/vircgroupmock.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index 06bd0a5f29..9c67a44b0d 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -460,8 +460,10 @@ FILE *fopen(const char *path, const char *mode) }
if (type) { - if (!filename) - abort(); + if (!filename) { + errno = EACCES; + return NULL; + } if (virAsprintfQuiet(&filepath, "%s/vircgroupdata/%s.%s", abs_srcdir, filename, type) < 0) { abort();
How about: diff --git i/tests/vircgroupmock.c w/tests/vircgroupmock.c index 06bd0a5f29..83f43e72bf 100644 --- i/tests/vircgroupmock.c +++ w/tests/vircgroupmock.c @@ -459,9 +459,7 @@ FILE *fopen(const char *path, const char *mode) } } - if (type) { - if (!filename) - abort(); + if (filename && type) { if (virAsprintfQuiet(&filepath, "%s/vircgroupdata/%s.%s", abs_srcdir, filename, type) < 0) { abort(); Michal
On Wed, Mar 27, 2019 at 12:55:18PM +0100, Michal Privoznik wrote:
On 3/27/19 11:31 AM, Daniel P. Berrangé wrote:
The mock fopen() function will abort if "/proc/mounts" is requested with "r" permissions and VIR_CGROUP_MOCK_FILENAME env var is not set.
Unfortunately this is triggering by the libselinux library constructor when it tries to read /proc/mounts to find out if selinuxfs is mounted in an unusual place.
This, however, only affects libselinux in Debian as that opens with "r", while in Fedora / RHEL it opens "re" and thus luckily never triggered the abort(), instead getting an EACCESS.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/vircgroupmock.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index 06bd0a5f29..9c67a44b0d 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -460,8 +460,10 @@ FILE *fopen(const char *path, const char *mode) } if (type) { - if (!filename) - abort(); + if (!filename) { + errno = EACCES; + return NULL; + } if (virAsprintfQuiet(&filepath, "%s/vircgroupdata/%s.%s", abs_srcdir, filename, type) < 0) { abort();
How about:
diff --git i/tests/vircgroupmock.c w/tests/vircgroupmock.c index 06bd0a5f29..83f43e72bf 100644 --- i/tests/vircgroupmock.c +++ w/tests/vircgroupmock.c @@ -459,9 +459,7 @@ FILE *fopen(const char *path, const char *mode) } }
- if (type) { - if (!filename) - abort(); + if (filename && type) { if (virAsprintfQuiet(&filepath, "%s/vircgroupdata/%s.%s", abs_srcdir, filename, type) < 0) { abort();
That will cause it to try to open the real /proc/mounts, which feels like an undesirable thing as it means the test could silently start relying on host state by mistake. 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 3/27/19 12:57 PM, Daniel P. Berrangé wrote:
On Wed, Mar 27, 2019 at 12:55:18PM +0100, Michal Privoznik wrote:
On 3/27/19 11:31 AM, Daniel P. Berrangé wrote:
The mock fopen() function will abort if "/proc/mounts" is requested with "r" permissions and VIR_CGROUP_MOCK_FILENAME env var is not set.
Unfortunately this is triggering by the libselinux library constructor when it tries to read /proc/mounts to find out if selinuxfs is mounted in an unusual place.
This, however, only affects libselinux in Debian as that opens with "r", while in Fedora / RHEL it opens "re" and thus luckily never triggered the abort(), instead getting an EACCESS.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/vircgroupmock.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index 06bd0a5f29..9c67a44b0d 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -460,8 +460,10 @@ FILE *fopen(const char *path, const char *mode) } if (type) { - if (!filename) - abort(); + if (!filename) { + errno = EACCES; + return NULL; + } if (virAsprintfQuiet(&filepath, "%s/vircgroupdata/%s.%s", abs_srcdir, filename, type) < 0) { abort();
How about:
diff --git i/tests/vircgroupmock.c w/tests/vircgroupmock.c index 06bd0a5f29..83f43e72bf 100644 --- i/tests/vircgroupmock.c +++ w/tests/vircgroupmock.c @@ -459,9 +459,7 @@ FILE *fopen(const char *path, const char *mode) } }
- if (type) { - if (!filename) - abort(); + if (filename && type) { if (virAsprintfQuiet(&filepath, "%s/vircgroupdata/%s.%s", abs_srcdir, filename, type) < 0) { abort();
That will cause it to try to open the real /proc/mounts, which feels like an undesirable thing as it means the test could silently start relying on host state by mistake.
Ah, good point. ACK to your version then. Michal
participants (2)
-
Daniel P. Berrangé -
Michal Privoznik