Re: [PATCH] tests: don't abort in fopen(/proc/mounts)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> > ---
> >   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 :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux