Bugzilla will be upgraded to version 5.0 on a still to be determined date in the near future. The original upgrade date has been delayed.
Bug 620368 - More libcgroup services not LSB-compliant
More libcgroup services not LSB-compliant
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: libcgroup (Show other bugs)
6.0
All Linux
low Severity high
: rc
: ---
Assigned To: Jan Safranek
Mike Gahagan
:
Depends On:
Blocks: 672300
  Show dependency treegraph
 
Reported: 2010-08-02 06:47 EDT by Caspar Zhang
Modified: 2013-07-03 03:25 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-05-19 09:13:08 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
patch against insufficient privilege use to run service (604 bytes, patch)
2010-08-03 01:49 EDT, Caspar Zhang
no flags Details | Diff


External Trackers
Tracker ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2011:0577 normal SHIPPED_LIVE libcgroup bug fix and enhancement update 2011-05-18 13:56:57 EDT

  None (edit)
Description Caspar Zhang 2010-08-02 06:47:58 EDT
Description of problem:

The testcase is located at tests/libcgroup/Sanity/initscript, the beaker link is: https://beaker.engineering.redhat.com/jobs/10077

From the result, we can see the following items are not LSB-compliant:

service cgconfig:
1) no pid file when cgconfig is running
2) pid file exists, but service not started (Expected 1, got 3)
3) lock file exists, but service not started (Expected 2, got 0)
4) Insufficient rights, like user `testcgconfigqa', restarting resrvice under nonprivileged user must fail (Expected 4, got 1)
5) Testing proper return code when nonexisting function, like `service cgconfig nonexistfunc' (Expected 2, got 1)
6) When no arguments added to service, it must fail with proper return code  (Expected 2, got 1)

service cgred:
1) Insufficient rights, restarting resrvice under nonprivileged user must fail (Expected 4, got 1)
2) Testing proper return code when nonexisting function (Expected 2, got 0)
3) When no arguments added to service, it must fail with proper return code  (Expected 2, got 0)

Version-Release number of selected component (if applicable):
libcgroup-0.36.1-6.el6

How reproducible:
always

Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:
Comment 2 RHEL Product and Program Management 2010-08-02 07:07:57 EDT
This issue has been proposed when we are only considering blocker
issues in the current Red Hat Enterprise Linux release.

** If you would still like this issue considered for the current
release, ask your support representative to file as a blocker on
your behalf. Otherwise ask that it be considered for the next
Red Hat Enterprise Linux release. **
Comment 3 Jan Safranek 2010-08-02 10:10:59 EDT
(In reply to comment #0)
> Description of problem:
> 
> service cgconfig:
> 1) no pid file when cgconfig is running
> 2) pid file exists, but service not started (Expected 1, got 3)

There is no deamon running, therefore there is no pid file.

cgconfig service just creates the groups on service start. It does not start any daemon which would need a pid file.

> 3) lock file exists, but service not started (Expected 2, got 0)

dtto, there is no daemon running, which we could check. The lock file is the only way, how to see if the service was started or not. If you remove the lock file, the service looses its only indicator and it assumes it was not started.

> 4) Insufficient rights, like user `testcgconfigqa', restarting resrvice under
> nonprivileged user must fail (Expected 4, got 1)

It is impossible to do it in init script. cgconfig service does not know, if it failed because of insufficient privileges or because e.g. the config file cannot be parsed. Many other services fail similar way, IMHO exit code 1 is acceptable in this case.

> 5) Testing proper return code when nonexisting function, like `service cgconfig
> nonexistfunc' (Expected 2, got 1)
> 6) When no arguments added to service, it must fail with proper return code 
> (Expected 2, got 1)

I'll fix those two.


> service cgred:
> 1) Insufficient rights, restarting resrvice under nonprivileged user must fail
> (Expected 4, got 1)

same as cgconfig service... 1 is IMHO acceptable.

> 2) Testing proper return code when nonexisting function (Expected 2, got 0)
> 3) When no arguments added to service, it must fail with proper return code 
> (Expected 2, got 0)

I'll fix those two.
Comment 4 Caspar Zhang 2010-08-03 01:49:12 EDT
Created attachment 436189 [details]
patch against insufficient privilege use to run service

(In reply to comment #3)
> 
> > 3) lock file exists, but service not started (Expected 2, got 0)
> 
> dtto, there is no daemon running, which we could check. The lock file is the
> only way, how to see if the service was started or not. If you remove the lock
> file, the service looses its only indicator and it assumes it was not started.

OK, I'll remove this test for now.

> 
> > 4) Insufficient rights, like user `testcgconfigqa', restarting resrvice under
> > nonprivileged user must fail (Expected 4, got 1)
> 
> It is impossible to do it in init script. cgconfig service does not know, if it
> failed because of insufficient privileges or because e.g. the config file
> cannot be parsed. Many other services fail similar way, IMHO exit code 1 is
> acceptable in this case.

How about the attached patch? check the current user id before run the service. These codes can be found in rdma and some other service packages.

> > service cgred:
> > 1) Insufficient rights, restarting resrvice under nonprivileged user must fail
> > (Expected 4, got 1)
> 
> same as cgconfig service... 1 is IMHO acceptable.

ditto. the patch attached.

Thanks,
Caspar
Comment 5 Jan Safranek 2010-08-03 01:57:20 EDT
(In reply to comment #4)
> (In reply to comment #3)
> > > 4) Insufficient rights, like user `testcgconfigqa', restarting resrvice under
> > > nonprivileged user must fail (Expected 4, got 1)
> > 
> > It is impossible to do it in init script. cgconfig service does not know, if it
> > failed because of insufficient privileges or because e.g. the config file
> > cannot be parsed. Many other services fail similar way, IMHO exit code 1 is
> > acceptable in this case.
> 
> How about the attached patch? check the current user id before run the service.
> These codes can be found in rdma and some other service packages.

This could work in the old good times, where only root could mount filesystems. Now we have extended capabilities - not only root can mount things. In addition, we have SELinux - sometimes even root itself can not mount :(. Checking for UID=0 does not work in modern Linux :(.
Comment 6 Jan Safranek 2010-08-05 04:15:37 EDT
My patches were accepted upstream, git commit be13bad0e41d8debde1e0c3c3345ee1f5158b72f
Comment 8 Mike Gahagan 2011-02-09 16:49:13 EST
With the current libcgroup-0.37-1.el6 build and some updates to selinux policy we should be seeing built in a few days, we are now down to this failure:

::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
:: [   LOG    ] :: cgred start - bad-configured
::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::

:: [   FAIL   ] :: Try start when cgconfig not running (Expected 7, got 1)
:: [   PASS   ] :: Running 'service cgred status'
:: [   PASS   ] :: Running 'rm -f /etc/cgrules.conf'
:: [   PASS   ] :: Try start when /etc/cgrules.conf not exist
:: [   PASS   ] :: Running 'service cgred status'
:: [   LOG    ] :: Duration: 1s
:: [   LOG    ] :: Assertions: 4 good, 1 bad
:: [   FAIL   ] :: RESULT: cgred start - bad-configured

in start() of the initscript we have:

        if ! grep "^cgroup" /proc/mounts &>/dev/null; then
                echo
                log_failure_msg $"Cannot find cgroups, is cgconfig service running?"
                return 1
        fi

When I read the LSB docs it seems to say that we should return 7 here (program is not running) where 1 is a generic error code.

 I am a bit concerned about that code because if some program other than cgconfig mounts the cgroup filesystem and happens to use the same name of cgroup then cgrulesengd will still start and might cause undesireable behavior if the groups it expects to see in /etc/cgrules.conf aren't actually there. I've confirmed the user can't confuse things with the mount command since those apperantly don't go into /proc/mounts so I don't know if we should really worry about this or not.
Comment 9 Jan Safranek 2011-03-03 09:50:06 EST
(In reply to comment #8)
> in start() of the initscript we have:
> 
>         if ! grep "^cgroup" /proc/mounts &>/dev/null; then
>                 echo
>                 log_failure_msg $"Cannot find cgroups, is cgconfig service
> running?"
>                 return 1
>         fi
> 
> When I read the LSB docs it seems to say that we should return 7 here (program
> is not running) where 1 is a generic error code.

Well, it depends which 'program' the LSB doc means. If you look at the LSB description of exit codes of 'service xxx status', 'program' clearly means the daemon which the service starts, therefore I think that 'program' refers to cgrulesengd in this context. And the scripts checks status of cgconfig service, not cgrulesengd.

I would expect exit code 7 e.g. on service cgred reload and the daemon not running.


>  I am a bit concerned about that code because if some program other than
> cgconfig mounts the cgroup filesystem and happens to use the same name of
> cgroup then cgrulesengd will still start and might cause undesireable behavior
> if the groups it expects to see in /etc/cgrules.conf aren't actually there.
> I've confirmed the user can't confuse things with the mount command since those
> apperantly don't go into /proc/mounts so I don't know if we should really worry
> about this or not.

cgrulesengd does not care who mounts the cgroups, if it is cgconfig service, different service or the user. It does not need cgconfig.conf at all, it finds the mount points by itself. I think my (simple) check is fine here.

I am confused regarding mounts created manually by mount command 'apperantly don't go into /proc/mounts'. Can you reproduce it? If so, it's serious kernel bug. *All* mounted filesystem, incl. cgroups, must be visible there, regardless how they were mounted. On the other hand, /etc/mtab may miss some, especially those mounted directly by applications (=cgconfig service) and not by /sbin/mount program.

(-> Moving back to ON_QA. Don't hesitate to move it back to me if you think I am wrong :)
Comment 10 Mike Gahagan 2011-03-11 11:27:43 EST
re: comment 9.. I think I misspoke... meant to say whatever's in /proc/mounts wasn't necessarily getting in mtab/getting displayed by the mount command.. which is OK for things that get mounted by programs other than mount.

I think we do have an issue with the kernel somewhere where a cgroup doesn't always umount properly, but that's a separate issue that might affect libcgroup services but doesn't necessarily indicate the bug lies with libcgroup.
Comment 11 Mike Gahagan 2011-03-11 11:28:32 EST
At any rate, I'll re-test once we get a beta tree.
Comment 12 Mike Gahagan 2011-04-08 16:09:15 EDT
Since no one feels strongly either way about what the return code should be (so long as it isn't zero) for comment 8. I think we can go ahead and consider this one fixed and the beaker test can be updated to allow 1 as a valid return code for that case.
Comment 13 errata-xmlrpc 2011-05-19 09:13:08 EDT
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2011-0577.html

Note You need to log in before you can comment on or make changes to this bug.