Bug 600636
Summary: | fails lvm-snapshot schroot (device-lock) | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Oron Peled <oron> | ||||||||
Component: | lockdev | Assignee: | Jiri Popelka <jpopelka> | ||||||||
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||||
Severity: | medium | Docs Contact: | |||||||||
Priority: | low | ||||||||||
Version: | 19 | CC: | damianatorrpm, jpopelka, os, pbonzini, sergio | ||||||||
Target Milestone: | --- | Keywords: | Reopened | ||||||||
Target Release: | --- | ||||||||||
Hardware: | All | ||||||||||
OS: | Linux | ||||||||||
Whiteboard: | |||||||||||
Fixed In Version: | lockdev-1.0.4-0.18.20111007git.fc22 | Doc Type: | Bug Fix | ||||||||
Doc Text: | Story Points: | --- | |||||||||
Clone Of: | Environment: | ||||||||||
Last Closed: | 2014-09-18 13:54:06 UTC | Type: | --- | ||||||||
Regression: | --- | Mount Type: | --- | ||||||||
Documentation: | --- | CRM: | |||||||||
Verified Versions: | Category: | --- | |||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||
Embargoed: | |||||||||||
Attachments: |
|
Description
Oron Peled
2010-06-05 10:43:04 UTC
After applying the workaround attached to this bug report, the lvm-snapshot has another (trivial) failure which is reported in bug #600639 Fixing also #600639 by adding a BR of libuuid-devel, results in a working lvm-snapshot schroot. I reproduced this with schroot 1.4.2. I will report this upstream. Comment from upstream: Hi Zach, This is not a bug in schroot. It's a bug in lockdev, specifically a patch to lockdev added in RedHat/Fedora which breaks setuid programs. schroot works fine using the vanilla upstream/Debian lockdev without this patch. This is the patch: http://lists.alioth.debian.org/pipermail/lockdev-devel/2010-February/000013.html and this is my review with the rationale: http://lists.alioth.debian.org/pipermail/lockdev-devel/2010-February/000039.html and here's a previous finder of this issue in CentOS: http://www.nikhef.nl/~dennisvd/schroot.html To summarise the above: this patch uses access(2) to see if the user has permission to use the device. access(2) is racy, but more importantly uses the real UID, not the effective UID of the calling process. Normally this makes no difference, but in a setuid program such as schroot, this completely breaks things. The suggested fix of the setuid call is AFAICT just working around this problem; schroot itself only has a single setuid call, and that's when switching to user context prior to invoking a program or shell on their behalf. All the locking and other setup is always done as root with EUID=0. I'm not sure why this patch was introduced, but as it stands it's lockdev at fault here, not schroot. Due to lack of upstream maintenance of lockdev, the Debian and Fedora lockdev sources have diverged with the accumulation of different patches over the last few years. Debian was the upstream for the last 10 years or so since upstream passed away. Over the last six months, mainly with the assistance of Ludwig Nussel (though Karel Zak at Red Hat was also involved initiating this), we have merged all the outstanding RedHat patches into upstream. You can find the git repo and mailing list at alioth.debian.org. Some bits were not included or were reworked , and the access(2) checks were one of those. We haven't yet made a new upstream release of this unified tree, but I would hope this is possible soon (the work is basically done, it just needs review and testing). Regards, Roger Changing component to lockdev, as this is not considered to be a bug in schroot. (In reply to comment #4) > Some bits were not included or > were reworked , and the access(2) checks were one of those. Upstream commit of this "rework" is here http://git.debian.org/?p=lockdev/lockdev.git;a=commitdiff;h=5a6928053ff6d88dcb3da765748720924f9424fc This message is a reminder that Fedora 13 is nearing its end of life. Approximately 30 (thirty) days from now Fedora will stop maintaining and issuing updates for Fedora 13. 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 '13'. 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 13'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 13 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: http://fedoraproject.org/wiki/BugZappers/HouseKeeping We were hopefull for the last year... alas :-( * No new commits in the git tree mentioned in comment #6 * We still cannot use schroot with lvm-snapshot due to this bug Any chance of sorting this out? (If you need help, just say what) Created attachment 576972 [details] Replace access() calls with euidaccess() * See comment #4 for details about access() problems * The euidaccess() behaves as expected and schroot can be used by non-root users on LVM snapshots (after applying another minor build fix to schroot [bugzilla #811856]) * The addition of the access() test specifically to Red-Hat (~2001) was not explained (it's not in the upstream source): - I think it was done to prevent from users who do not have write access to a device to lock it via lockdev (i.e: prevent denial of service attack) - I added a comment to this effect as part of the patch - Is there any other reason for this? * New release of schroot(1): https://admin.fedoraproject.org/updates/FEDORA-2012-7764/schroot-1.4.25-4.fc16 * This release contains again the 'lvm-snapshot' feature. However, this lockdev bug prevent this feature from working. * What about my proposed fix? Good? Bad? Something else? Wow, that's really strange - I never got any email from Bugzilla since my last comment. In F-17 we have lockdev-1.0.4-0.4.20111007git.fc17 which is the upstream git snapshot containing all the latest upstream changes including the one mentioned in comment #6. Can you test that one ? I think you can safely rebuild&use it in F-16 (I remember testing it in F-16 too). Does that make any change ? Do we still need your patch ? I appreciate your work and I'm sorry for all the inconvenience. I tested an lvm-snapshot in a new build of schroot with the latest lockdev in F-17, and it worked just fine. Currently lvm-snapshot feature is not enabled in F-17, but once I get my new build in, it should be working again. To test lvm-snapshot in F-17 with newest lockdev, use this build: http://koji.fedoraproject.org/koji/buildinfo?buildID=320564 Zach, there must be some error in your f17 test. I've just tested it in f17 virtual machine and it has the same (lockdev) problem. - Installed your new f17 build from: http://koji.fedoraproject.org/koji/buildinfo?buildID=320564 - Verified lockdev version: $ rpm -q lockdev lockdev-1.0.4-0.4.20111007git.fc17.i686 - Created a logical volume (/dev/data/squeeze) - Mounted it temporarily on /mnt - debootstrap squeeze /mnt # so I have Debian/squeeze OS inside - umount /mnt - Created /etc/schroot/chroot.d/squeeze-i386.conf: [squeeze-i386] type=lvm-snapshot description=Debian squeeze LVM snapshot users=oron source-root-users=oron device=/dev/data/squeeze mount-options=-o atime,sync,user_xattr lvm-snapshot-options=--size 0.5G - Tried using as *root* user -- no problem: schroot -c squeeze-i386 - Tried using as myself -- permission denied Analayzing the problem: - Minor issue: one has to be in the 'lock' group to use lockdev. This isn't clearly documented in the man-page, but was easy to find. After adding myself to the 'lock' group, logout, login, id (shows I'm in the lock group) - strace schroot -c squeeze-i386: ... umask(02) = 02 stat64("/dev/data/squeeze", {st_mode=S_IFBLK|0660, st_rdev=makedev(253, 2), ...}) = 0 access("/dev/data/squeeze", W_OK) = -1 EACCES (Permission denied) umask(02) = 02 ... - This obviously fails, because normal users should not have write (or read) access to disk devices. Although schroot is suid, it doesn't help, because access() check against 'ruid' and not 'euid'. To debug lockdev in isolation, I created the following suid-root wrapper, testlockdev.c: #include <stdio.h> #include <unistd.h> int main() { execlp("lockdev", "lockdev", "-l", "/dev/data/squeeze", NULL); perror("lockdev"); return 1; } Testing the wrapper: $ gcc -Wall -o testlockdev testlockdevs.c $ su -c 'chown root testlockdev' Password: $ su -c 'chmod u+s testlockdev' Password: $ ./testlockdev $ echo $? 2 $ strace ./testlockdev stat64("/dev/data/squeeze", {st_mode=S_IFBLK|0660, st_rdev=makedev(253, 2), ...}) = 0 access("/dev/data/squeeze", W_OK) = -1 EACCES (Permission denied) umask(02) = 02 exit_group(2) = ? Fails :-( The following upstream change may shed some light: - http://lists.alioth.debian.org/pipermail/lockdev-devel/2010-March/000058.html - It specifically tries to address setuid programs - But that code only applies to a small function that is used only for tty-locks (and also enclosed in some #ifdef) Looking at the code make it clean that current lockdev cannot work for the general case of setuid programs. Hasn't anybody else bumped into it now that upstream version includes these buggy access() calls? Dang it, you are right. I was testing it as root. Sorry for the false results. [zcarter@nudj ~]$ schroot -c test-sch W: line 12 [test-sch]: Deprecated key ‘run-setup-scripts’ used I: This option will be removed in the future; please update your configuration W: line 13 [test-sch]: Deprecated key ‘run-exec-scripts’ used I: This option will be removed in the future; please update your configuration W: line 4 [test-sch]: Deprecated key ‘priority’ used I: This option will be removed in the future; please update your configuration E: test-sch-fae690ab-3884-44f4-9cc4-e1b46b61bb82: Failed to lock chroot: /dev/fastvg/schtestlv: Failed to lock device: Failed to lock device Maybe this is related but since the yesterday most fedora packages fail for me in the open build service with: error: unpacking of archive failed on file /var/lock/lockdev: cpio: mkdir failed - No such file or directory error: lockdev-1.0.4-0.4.20111007git.fc17.i686: install failed nevermind this was an OBS issue https://bugzilla.novell.com/show_bug.cgi?id=769756 Had the same issue with schroot command on F-17. Did changed the 'access' call to 'euidaccess' in lockdev, and it works just fine. Thanks to Roger for the hint! ;) This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle. Changing version to '19'. (As we did not run this process for some time, it could affect also pre-Fedora 19 development cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.) More information and reason for this action is here: https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19 Summarizing: The problem was discussed upstream http://lists.alioth.debian.org/pipermail/lockdev-devel/2010-March/000052.html and this commit http://lists.alioth.debian.org/pipermail/lockdev-devel/2010-March/000058.html was supposed to fix it. or Laszlo Nagy 2012-08-09 14:12:58 EDT Had the same issue with schroot command on F-17. Did changed the 'access' call to 'euidaccess' in lockdev, and it works just fine. Thanks to Roger for the hint! ;) any of this is applied to lockdev ? this bug still not fixed ? * At least regarding schroot, there's a light in the end of the tunnel: - The new upstream schroot-1.7.x does not depend on lockdev anymore!!! (It also dropped libuuid dependency) - I've started testing it so I can ask Zach to bump it (but currently still have some problems that need resolving) * Other than that: - Let's see who use lockdev in Fedora: [root@f19 ~]# repoquery --whatrequires lockdev ckermit-0:9.0.302-4.fc19.i686 dchroot-0:1.4.25-13.fc19.i686 libcec-0:2.1.3-1.fc19.i686 libgphoto2-0:2.5.2-1.fc19.i686 lockdev-devel-0:1.0.4-0.6.20111007git.fc19.i686 mgetty-0:1.1.36-21.fc19.i686 mgetty-sendfax-0:1.1.36-21.fc19.i686 mgetty-voice-0:1.1.36-21.fc19.i686 minicom-0:2.6.2-1.fc19.i686 schroot-0:1.4.25-13.fc19.i686 ttywatch-0:0.14-17.fc19.i686 uucp-0:1.07-36.fc19.i686 wvdial-0:1.61-7.fc19.i686 - IIRC, the bad code-path in lockdev only affect storage devices, so if I'm correct, without schroot it only leaves: libgphoto2 * So far this lockdev saga: - Doesn't seem to end in my lifetime :-( - The buggy patches seem to be some RHEL legacy stuff - As a result, probably nobody want to fix them (maintainer had 2 relevant comments in 3 years). - Let's nominate it "abandonware of the year" ;-) but if new schroot solves the problem let focus on update it # repoquery --releasever=rawhide --disablerepo=\*updates\* -q schroot schroot-0:1.4.25-13.fc20.x86_64 # repoquery --releasever=rawhide --disablerepo=\*updates\* -q schroot --whatrequires (nothing) if say schroot 1.7 seems 1.4 is pretty old opened bug #969885 (In reply to Sergio Monteiro Basto from comment #21) > or Laszlo Nagy 2012-08-09 14:12:58 EDT > Had the same issue with schroot command on F-17. Did changed the 'access' > call to 'euidaccess' in lockdev, and it works just fine. Thanks to Roger for > the hint! ;) > > any of this is applied to lockdev ? Sorry. I just applied patch from comment #9. http://koji.fedoraproject.org/koji/taskinfo?taskID=5458117 lockdev-1.0.4-0.7.20111007git.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/lockdev-1.0.4-0.7.20111007git.fc19 Tested schroot in F19 with the new lockdev update and it seems to be working. Cool! Package lockdev-1.0.4-0.7.20111007git.fc19: * should fix your issue, * was pushed to the Fedora 19 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing lockdev-1.0.4-0.7.20111007git.fc19' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2013-10002/lockdev-1.0.4-0.7.20111007git.fc19 then log in and leave karma (feedback). (In reply to Zach Carter from comment #27) > Tested schroot in F19 with the new lockdev update and it seems to be > working. Cool! fix the initial problem ? (schroot fails entering an lvm-snapshot type chroot (Fails to lock device) ) I think we also should build this package for F18 lockdev-1.0.4-0.6.20111007git.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/lockdev-1.0.4-0.6.20111007git.fc18 lockdev-1.0.4-0.6.20111007git.fc18 has been pushed to the Fedora 18 stable repository. If problems still persist, please make note of it in this bug report. lockdev-1.0.4-0.7.20111007git.fc19 has been pushed to the Fedora 19 stable repository. If problems still persist, please make note of it in this bug report. Created attachment 938865 [details]
patch to support ACLs in the helper
Unfortunately, the fix breaks another case. The problem is that euidaccess does not honor ACLs; it just checks against the file uid and gid.
In my setup, I'm /dev/ttyS0 is granted console user access via udev, so that it has these ACLs:
# file: dev/ttyUSB0
# owner: root
# group: dialout
user::rw-
user:pbonzini:rw-
group::rw-
mask::rw-
other::---
When ttylock is called, the real and effective uids are pbonzini. However, euidaccess returns false because it doesn't look at ACLs, and /usr/sbin/lockdev fails.
I think you should use "access" from the helper, since the caller helpfully swaps the real and effective uids for it. The attached patch (a replacement for the existing lockdev-euidaccess.patch) does it by embedding the lockdev library into the helper, instead of linking to the shared library.
With this patch,
minicom -D /dev/ttyUSB0
works for me instead of failing with EACCES.
Alternative approaches include:
- instead of euidaccess, make dev_lock/dev_unlock fork a child process and do setreuid+setregid+access. Communicate the result via the exit code; remove the now redundant setreuid+retregid pair in _spawn_helper. Same result as euidaccess, but with correct handling of ACLs. But much more invasive.
- add a function pointer that defaults to euidaccess(), set it to access() from the helper's main().
Thanks Paolo ! |