Bug 507397 - Directory permissions on volume group directory too restrictive
Summary: Directory permissions on volume group directory too restrictive
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: lvm2
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Peter Rajnoha
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 523009 (view as bug list)
Depends On:
Blocks: F12VirtBlocker
TreeView+ depends on / blocked
 
Reported: 2009-06-22 16:11 UTC by Daniel Berrangé
Modified: 2018-04-11 18:31 UTC (History)
19 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-09-30 09:04:06 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Daniel Berrangé 2009-06-22 16:11:24 UTC
Description of problem:
I have a volume group called 'HostVG', whose volumes appear under /dev/HostVG. I need to change ownership of one of the volume to allow a non-root KVM process to use it for its virtual disk. Unfortunately even if I chown() the volume /dev/HostVG/VirtTest, the non-root KVM process still can't access it because the permissions on the directory /dev/HostVG are "drwx------."

These restrictive permissions are bogus from a security POV, since that directory merely contains symbolic links to the real files under /dev/mapper which is "drwxr-xr-x.".

Thus the directory for the volume group symlink should have its permissions relaxed to match those of /dev/mapper

Version-Release number of selected component (if applicable):
lvm2-2.02.45-4.fc11.i586

How reproducible:
Always

Steps to Reproduce:
1. useradd test
2. chown 'test' /dev/HostVG/VirtTest
3. su test 
4. ls -l /dev/HostVG/VirtTest
  
Actual results:
$ ls -l /dev/HostVG/VirtTest
ls: cannot access /dev/HostVG/VirtTest: Permission denied

Expected results:
Can see the volume that was chown'd

Additional info:
This is a blocker for a virtualization feature in Fedora 12 where we run KVM with a non-root user ID and need to chown individual volumes

Comment 1 Milan Broz 2009-06-22 17:36:55 UTC
I hope that F12 will have there dirs & symlinks managed by udev rules, so the problem will be just about setting proper permissions in udev rules.

We should probably consolidate VG directory permissions to be the same as in /dev/mapper even in old code...

Comment 2 Peter Rajnoha 2009-06-30 14:30:50 UTC
Yes, exactly -- node/symlink permissions will be controlled by udev rules completely in a separate rule file with access to actual device name, its uuid, vg name, lv name and layer name that will be accessible via prepared environment variables, so one can easily use these values in the rules then.

However, the perms for dirs are set to 0755 statically by udevd. But that should be OK for everyone...

Comment 3 Daniel Berrangé 2009-06-30 14:42:38 UTC
Yes, 0755 directory permissions is fine for libvirt's needs because that's relaxed enough to let us open a file within it.

Comment 4 Daniel Berrangé 2009-08-04 15:31:15 UTC
This is still a problem on current rawhide

# ls -ld /dev/vg_dhcp0233/
drwx------. 2 root root 80 2009-08-04 11:28 /dev/vg_dhcp0233/



lvm2-2.02.50-2.fc12.x86_64
udev-145-3.fc12.x86_64

kernel 2.6.31-0.118.rc5.fc12.x86_64 


We really need this fixed in F12 beta, otherwise no one will be able to use LVM storage for their guests

Comment 5 Richard W.M. Jones 2009-08-04 15:32:56 UTC
Just for reference, since this bug didn't turn up in my
searches, the error you get from libvirt / virsh is:

# virsh start dom
error: Failed to start domain dom
error: internal error unable to start guest: qemu: could not open disk image /dev/VG/dom

Comment 6 Alasdair Kergon 2009-08-04 15:41:08 UTC
The udev code has been checked in upstream over the last couple of days and is undergoing basic upstream testing/bug-fixing prior to going into rawhide.

Comment 7 Mark McLoughlin 2009-08-10 14:19:00 UTC
Doesn't seem to be any change with 2.02.51, which seems to contain the udev code

$> rpm -q lvm2
lvm2-2.02.51-2.fc12.x86_64
$> dd if=/dev/zero of=//tmp/lvm2-test.img bs=1M count=0 seek=1024
$> losetup /dev/loop0 /tmp/lvm2-test.img 
$> pvcreate /dev/loop0 
$> vgcreate TestVG /dev/loop0 
$> lvcreate --extents 100 TestVG
$> ls -ld /dev/TestVG/
drwx------. 2 root root 60 2009-08-10 15:12 /dev/TestVG/

Comment 8 Peter Rajnoha 2009-08-10 15:53:08 UTC
It's not yet compiled in by default. But we will make an official rpm with udev synchronisation in rawhide soon, I promise.

If you want to give it a try on some testing machine, you can try building it yourself for now adding the '--enable-udev-sync' configure option. But it's without any warranty yet, be cautious! We're still adding some fallback and check code so the possible problems are minimised (detecting udev daemon is not running etc.).

Comment 9 Daniel Berrangé 2009-08-26 12:41:31 UTC
Is this switch over to udev going to land in F12 rawhide in the near future ? It is getting very late in dev cycle to be making such a change, so if it is not going to be done, then we really need the existing non-udev code to be fixed to use the correct permissions for creating VG  directories.

Comment 10 Peter Rajnoha 2009-08-26 13:05:29 UTC
The thing is that we're trying to synchronize the rules with the other packages as well (that use their own set of rules) - some modifications has to be done there and they should be synchronized properly. I've been arranging this exactly last couple of days.

However, a few problems have appeared lately concerning some utilities, which are a little bit confused now by using symlinks in /dev/mapper and nodes in /dev directly (/dev/dm-X).

But I beleive this will be in F12 rawhide finally. This week or next week, I guess (if there's no other problem discovered).

Comment 11 Mark McLoughlin 2009-09-04 10:17:15 UTC
(In reply to comment #10)
> But I beleive this will be in F12 rawhide finally. This week or next week, I
> guess (if there's no other problem discovered).  

Any further progress?

Comment 12 Peter Rajnoha 2009-09-04 11:31:22 UTC
Yes, last patches are waiting for final review (today?) so they can be included in new upstream release from which we will make a new rawhide release (next week?).

But...

There is still one problem we're tracking and which is quite painful -- grub2 does not try to follow symlinks (when using grub-probe utility). But udev guys insist on having nodes in /dev dir directly and symlinks anywhere else (which is /dev/mapper in our situation). So, what I'll try to do next is to persuade grub guys to try to follow these symlinks, if that is possible for them to do and won't cause any other harm.

On the other hand, mount utility follows symlinks to their 'dead end' :) (which causes mount, df and everything reading the mtab file to display /dev/dm-X instead of nice names like /dev/mapper/my_very_nice_name -- this situation is not quite acceptable by users, of course..).

So, it's quite 'colorful', but we will make our udev thing to go out once. We will! :)

Comment 13 Mark McLoughlin 2009-09-14 11:42:47 UTC
adding to F12VirtBlocker

Any more progress? This is a serious issue for Fedora 12 virt

Comment 14 Mark McLoughlin 2009-09-14 11:44:05 UTC
*** Bug 523009 has been marked as a duplicate of this bug. ***

Comment 15 Justin M. Forbes 2009-09-17 17:02:00 UTC
Any update on this? It is still a major issue.

Comment 16 Geert Jansen 2009-09-18 07:31:04 UTC
Guys, basically virt on LVM storage does not work for Rawhide/F12. This is a serious issue and should be resolved asap. I don't understand why this BZ has priority Low...

In response to comment #10, can we decouple the multiple updates and just do an update of the /dev/mapper changes?

Comment 17 Daniel Berrangé 2009-09-18 16:50:39 UTC
The /dev/mapper  directory is not the problem - that already has the correct permissions "drwxr-xr-x."

The issue is with the volume group directory, eg for a vol group 'HostVG', the directory /dev/HostVG/  has permissions "drwx--------" which prevents QMEU reading any of the symlinks for the volumes inside there.

We really do just need a fix for whatever piece of code creates the /dev/$VGNAME directories - this shouldn't need to block on completely re-writing LVM to use udev

Comment 18 Milan Broz 2009-09-18 16:55:59 UTC
setting umask = 022 in /etc/lvm/lvm.conf will not help (as temp. workaround)?

(you will need to regenerate initrd so this cfg is updated there too)

Comment 19 Mark McLoughlin 2009-09-21 14:39:47 UTC
Can someone confirm that mbroz's suggestion fixes the issue ?

(In reply to comment #18)
> setting umask = 022 in /etc/lvm/lvm.conf will not help (as temp. workaround)?

So, you are proposing changing the default umask lvm.conf from the lvm package for F12?

Comment 20 Peter Rajnoha 2009-09-21 16:20:04 UTC
OK, we've just made the changes in rawhide just now - udev synchronisation is now turned on and udev rules are installed by default (lvm2-2_02_52-3, device-mapper-1.02.37-3).

If there are any problems, please let me know as soon as possible...

Comment 21 Peter Rajnoha 2009-09-21 16:33:10 UTC
...if you would like to set permissions for particular nodes, you can use the template that is in /usr/share/doc/lvm2-.. dir (12-dm-permissions.rules). This one is supposed to be installed in /etc/udev/rules.d.

When udev is used, dirs have 0755 permissions set (this is hardcoded by udevd and can't be changed).

Comment 22 Justin M. Forbes 2009-09-23 22:00:50 UTC
Things appear to work with rawhide now.

Comment 23 Peter Rajnoha 2009-09-24 23:12:46 UTC
I'm really sorry to tell you this but we had to revert the last build. There were some problems with this change considering the interaction with other packages.

So udev synchronisation code and udev rules are disabled in device-mapper/lvm2 packages again...

Comment 24 Mark McLoughlin 2009-09-25 08:10:25 UTC
Yes, well it is very late in the release cycle

Now can we get this fixed with a simple change like setting the default umask to 022 in /etc/lvm/lvm.conf ?

Comment 25 Daniel Berrangé 2009-09-25 09:25:57 UTC
Indeed, we never wanted the entire of LVM OS integration to be re-written for this bug. We only want the directory permissions on /dev/$VGNAME to be addressed which should not be more than a couple of lines change.

Comment 26 Alasdair Kergon 2009-09-25 11:24:25 UTC
No, but that was being done anyway and would have fixed it.  We'll go for a specific 3-line change instead.

Changing the default umask on a multi-user system is not a good idea - that setting is only really there so that people have the option of a temporary workaround in situations such as this.

Comment 27 Daniel Berrangé 2009-09-25 11:29:10 UTC
Whether it is a multi-user system or not is irrelevant. We are only talking about permissions on the directory /dev/$VGNAME.  This directory is merely full of symlinks to /dev/mapper which already has world read+execute. We're not requesting a change on the individual volumes in /dev/mapper, so relaxing /dev/$VGNAME directory permissions has zero impact on security of the system.

Comment 28 Peter Rajnoha 2009-09-25 12:02:52 UTC
Changed in upstream. We'll do a new rawhide build today as well..

Comment 29 Peter Rajnoha 2009-09-29 08:56:22 UTC
In rawhide: device-mapper-1.02.38-2, lvm2-2.02.53-2.

Comment 30 Mark McLoughlin 2009-09-29 21:06:53 UTC
Doesn't seem to be working for me:

# rpm -q lvm2 device-mapper
lvm2-2.02.53-2.fc12.x86_64
device-mapper-1.02.38-2.fc12.x86_64
# ls -ld /dev/VolGroup00/
drwx------. 2 root root 80 2009-09-29 22:00 /dev/VolGroup00/

I have rebooted this system since updating the packages

Comment 31 Peter Rajnoha 2009-09-30 08:40:48 UTC
Just a quick check - what are the permissions for /dev/mapper directory?

(Now we use exactly the same perms for /dev/<vgname> as well as for /dev/mapper, umask setting in lvm.conf is not taken in this situation. We create the dirs with 0777 perms and set default 0022 umask directly for these ones...)

Comment 32 Daniel Berrangé 2009-09-30 08:53:05 UTC
Is it neccessary to rebuild the initrd after updating the lvm2 package with this change ?  I thought perhaps this directory might be being created/setup while still in the initrd phase ?

Comment 33 Mark McLoughlin 2009-09-30 09:04:06 UTC
Good call Dan, I did a full yum update (i.e. including a kernel update) and it seems to be working fine now

Sorry for the false alarm


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