Bug 556358

Summary: Various tests give illusory results
Product: [Fedora] Fedora Reporter: JW <ohtmvyyn>
Component: coreutilsAssignee: Ondrej Vasik <ovasik>
Status: CLOSED UPSTREAM QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: kdudka, meyering, ovasik, twaugh
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-11-05 04:46:47 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description JW 2010-01-17 22:01:38 EST
Description of problem:
Several of the tests after build invoke a utility 'setuidgid'.  Unfortunately in doing so it is most likely that the wrong program executes.

Version-Release number of selected component (if applicable):

How reproducible:

Steps to Reproduce:
1.  create an executable file, say 'rm', in your path so that it executes by default and always produces a syntax error
2. run the tests/no-give-up script
Actual results:
2. You will see the syntax error

Expected results:
2. You should see the src/rm run and complete successfully

Additional info:
The problem occurs when the following is invoked:
"setuidgid nobody env PATH=/path/to/src:<oldpath> rm --version".
Now setting a new PATH is a brilliant idea except what if user 'nobody' doesn't have read permission on the compiler's home directory?  I know that I never allow user 'nobody' to read in any of my home directories.  The consequence of this is that a path search will never match the src/rm instance.

But the silly thing is that the PATH is prefixed with /path/to/src so why not forget about the path and simple invoke /path/to/src/rm directly?  That is guaranteed to work since the src/rm and build directories give others execute permission (this is ensured in various scripts).

In any case the consequence of this bugs is that often the test scripts will actually be testing the already installed programs which exists somewhere else in the PATH rather than the newly compiled programs!

Question is: Who tests the test suites?  Who tests the testers?

This is a fantastic example which demonstrates that having automated test suites are generally no better than not having them.  Because bugs in the test suite will give people a false sense that the test suites are actually proving something.  You would need to create new test-suites that test the test-suites ad infinitum.
Comment 1 JW 2010-01-17 23:03:37 EST
The solution is to patch every test script that utilizes setuidgid in the following manner:

-setuidgid $NON_ROOT_USERNAME env PATH="$PATH" cp -p c c2 || fail=1
+setuidgid $NON_ROOT_USERNAME env PATH="$PATH" $abs_top_builddir/src/cp -p c c2 || fail=1

Just to ensure that the recently compiled program is really the one being tested!

But unfortunately one still needs to ensure that execute permission is granted to 'others' on ones home directory.  One wouldn't want to automate this in a script!

I think the only correct solution is to create a temporary directory in /tmp, give it the correct permissions, and copy all the compiled programs there.  Then run the tests with the tmp directory using an absolute path to the binary and without a PATH change.
Comment 2 JW 2010-01-17 23:09:22 EST
... or better still execute the programs which have already been copied into the BUILDROOT subdirectory - but that would only be appropriate for rpm builds.

Additionally setuidgid should complain loudly if a target cannot be executed (and not bother executing via 'env').
Comment 3 Jim Meyering 2010-04-18 06:13:21 EDT
Thanks for the report.
We take pains *not* to invoke via absolute names, because that often leads to false positive test failures due to differences in argv[0] that usually appears in diagnostics.

The advice in README should be enough to solve your problem:

Running tests as root:

If you run the tests as root, note that a few of them create files
and/or run programs as a non-root user, `nobody' by default.
If you want to use some other non-root username, specify it via
the NON_ROOT_USERNAME environment variable.  Depending on the
permissions with which the working directories have been created,
using `nobody' may fail, because that user won't have the required
read and write access to the build and test directories.
I find that it is best to unpack and build as a non-privileged
user, and then to run the following command as that user in order
to run the privilege-requiring tests:

  sudo env PATH="$PATH" NON_ROOT_USERNAME=$USER make -k check-root

If you can run the tests as root, please do so and report any
problems.  We get much less test coverage in that mode, and it's
arguably more important that these tools work well when run by
root than when run by less privileged users.
Comment 4 JW 2010-04-18 07:06:15 EDT
No, the advice in README is not sufficient for reliable building of rpms.  When one builds an rpm one does not read README's and manually invoke special commands depending on a number of vague environmental considerations.

Any solution you propose must work when ANY user builds an rpm.

Wouldn't it be true to say that computers can relieve us of tedious tasks and perform lots of wonderful things for us? But not so in your world.  Your world requires us to laboriously invoke arcane voodoo and read lots of README's and manually do what our computers are supposed to do for us.  Because apparently in your world the solution to improper computer function is to not fix the actual problem but to recommend that users resort to complicated work-arounds (if they manage to detect the malfunction in the first place).

So back to square one, Jim.

Come up with a solution that doesn't require rpm builders to read bundled README's and invoke arcane command-line magic.

Make your rpms build successfully under a broad range of conditions. And don't get into user profiling and start weighing against users who dare to be root.  Treat all users as equal and make your rpms build properly and reliably even for root!

The fact is that the tests invoked during an rpm build are unreliable and their apparent successful execution can sometimes be just an illusion.

Fix the real problem.  No excuses please.
Comment 5 Ondrej Vasik 2010-04-19 04:43:01 EDT
Let's clarify few things:
1) Jim is upstream maintainer of coreutils, not maintainer in Fedora/RHEL - he has almost nothing to do with rpms/srpms we ship in Fedora.
2) Coreutils rpms we ship in Fedora are built on koji build system - with user/group mockbuild - and tests are run in that case correctly. As only binaries are shipped, users can not run (or get confused by) tests. So only case is rebuilding coreutils srpm under very special conditions on your machine.

Any solution for that must be accepted by upstream - as this is not Fedora specific thing. If you think that tests with setuidgid might be in risk of unreliability when directory with binaries is unreadable for NON_ROOT_USERNAME, maybe some additional check && skip for those 9 tests with setuidgid might be added into test-lib.sh (require_readablesrcdir_nonroot_() ?) to improve reliability in those very special cases...

Jim, what do you think?
Comment 6 Jim Meyering 2010-04-19 11:24:56 EDT
Hi Ondrej,

That sounds fine.  It would be even better if you can make make it so that any root-only test automatically determines whether it uses setuidgid, and if so, runs this additional test.  Thanks!
Comment 7 Bug Zapper 2010-11-03 20:41:38 EDT
This message is a reminder that Fedora 12 is nearing its end of life.
Approximately 30 (thirty) days from now Fedora will stop maintaining
and issuing updates for Fedora 12.  It is Fedora's policy to close all
bug reports from releases that are no longer maintained.  At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '12'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 12's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that 
we may not be able to fix it before Fedora 12 is end of life.  If you 
would still like to see this bug fixed and are able to reproduce it 
against a later version of Fedora please change the 'version' of this 
bug to the applicable version.  If you are unable to change the version, 
please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events.  Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

The process we are following is described here: 
Comment 8 JW 2010-11-03 20:46:26 EDT
Still an issue with fedora 14 (coreutils-8.5-6)
Comment 9 Ondrej Vasik 2012-11-05 04:46:47 EST
This should be resolved upstream as a part of coreutils-8.20 release ( http://git.savannah.gnu.org/cgit/coreutils.git/commit/init.cfg?id=51a4b04954ad5ad12de1d1b82a3603fc350a3bfa and http://git.savannah.gnu.org/cgit/coreutils.git/commit/init.cfg?id=ad5eeacc6740fd98ebe641b6fa88ec4019295b4e ) - this release is now build in Fedora Rawhide. Closing upstream.