Bug 903716 - udev: device node permissions not applied with "change" event
udev: device node permissions not applied with "change" event
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: systemd (Show other bugs)
18
All Linux
urgent Severity urgent
: ---
: ---
Assigned To: Michal Schmidt
Fedora Extras Quality Assurance
:
: 903703 (view as bug list)
Depends On:
Blocks: ovirt-3.2-release
  Show dependency treegraph
 
Reported: 2013-01-24 11:14 EST by Federico Simoncelli
Modified: 2013-02-25 21:43 EST (History)
16 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-02-25 21:43:18 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
12-vdsm-lvm.rules (4.55 KB, application/octet-stream)
2013-01-24 11:14 EST, Federico Simoncelli
no flags Details

  None (edit)
Description Federico Simoncelli 2013-01-24 11:14:12 EST
Created attachment 686847 [details]
12-vdsm-lvm.rules

Description of problem:
systemd-udevd is not setting permissions (owner/group/mode) on lvm devices according to the udev rules (in attachment).
Please note that the same rules are working on el6, on f17 and probably also on f18 alpha.

Version-Release number of selected component (if applicable):
systemd-197-1.fc18.1.x86_64

How reproducible:
100%

Steps to Reproduce:
1. lvchange -ay /dev/078a61d3-fc5d-4e8d-a8b3-8a0d41b7e834/leases
2. ls -l `readlink -f /dev/078a61d3-fc5d-4e8d-a8b3-8a0d41b7e834/leases`
  
Actual results:
The LV has the regular permissions root:disk:

# ls -l `readlink -f /dev/078a61d3-fc5d-4e8d-a8b3-8a0d41b7e834/leases`
brw-rw----. 1 root disk 253, 11 Jan 24 17:05 /dev/dm-11

Expected results:
According to the udev rule:

ENV{DM_VG_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", ENV{DM_LV_NAME}=="leases", MODE:="0660", OWNER:="vdsm", GROUP:="sanlock", GOTO="lvm_end"

The LV should have the following permissions:

# ls -l `readlink -f /dev/078a61d3-fc5d-4e8d-a8b3-8a0d41b7e834/leases`
brw-rw----. 1 vdsm sanlock 253, 11 Jan 24 17:06 /dev/dm-11


Additional info:
It looks like the udev "add" event doesn't carry the relevant information (DM_LV_NAME, DM_VG_NAME, etc), and the "change" event (which receives the relevant info) has been recently changed to not set the permission anymore:

# udevadm monitor -u -e
UDEV  [99132.729059] add      /devices/virtual/bdi/253:11 (bdi)
ACTION=add
DEVPATH=/devices/virtual/bdi/253:11
SEQNUM=95779
SUBSYSTEM=bdi
USEC_INITIALIZED=132728122

UDEV  [99132.737751] add      /devices/virtual/block/dm-11 (block)
ACTION=add
DEVNAME=/dev/dm-11
DEVPATH=/devices/virtual/block/dm-11
DEVTYPE=disk
DM_UDEV_DISABLE_DISK_RULES_FLAG=1
DM_UDEV_DISABLE_OTHER_RULES_FLAG=1
DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG=1
MAJOR=253
MINOR=11
MPATH_SBIN_PATH=/sbin
SEQNUM=95780
SUBSYSTEM=block
SYSTEMD_READY=0
TAGS=:systemd:
USEC_INITIALIZED=132728447

UDEV  [99132.783519] change   /devices/virtual/block/dm-11 (block)
ACTION=change
DEVLINKS=/dev/078a61d3-fc5d-4e8d-a8b3-8a0d41b7e834/leases /dev/disk/by-id/dm-name-078a61d3--fc5d--4e8d--a8b3--8a0d41b7e834-leases /dev/disk/by-id/dm-uuid-LVM-RvNIyteApv9Gf8SXSaIWUlHqIB4VKV5zFpqzQIY6zGGFfFO0S1VnPSRxcaMcO0LN /dev/mapper/078a61d3--fc5d--4e8d--a8b3--8a0d41b7e834-leases
DEVNAME=/dev/dm-11
DEVPATH=/devices/virtual/block/dm-11
DEVTYPE=disk
DM_COOKIE=6356336
DM_LV_NAME=leases
DM_NAME=078a61d3--fc5d--4e8d--a8b3--8a0d41b7e834-leases
DM_SUSPENDED=0
DM_UDEV_DISABLE_LIBRARY_FALLBACK_FLAG=1
DM_UDEV_PRIMARY_SOURCE_FLAG=1
DM_UDEV_RULES_VSN=2
DM_UUID=LVM-RvNIyteApv9Gf8SXSaIWUlHqIB4VKV5zFpqzQIY6zGGFfFO0S1VnPSRxcaMcO0LN
DM_VG_NAME=078a61d3-fc5d-4e8d-a8b3-8a0d41b7e834
MAJOR=253
MINOR=11
MPATH_SBIN_PATH=/sbin
SEQNUM=95781
SUBSYSTEM=block
TAGS=:systemd:
USEC_INITIALIZED=132728447
Comment 1 Kay Sievers 2013-01-24 11:35:31 EST
Recent udev versions only set the device node permissions and selinux context
with "add" and never later.

We need to find some fix here ...
Comment 2 Kay Sievers 2013-01-24 11:36:06 EST
*** Bug 903703 has been marked as a duplicate of this bug. ***
Comment 3 Bill Nottingham 2013-01-24 11:53:51 EST
Don't we need 'change' for the cases where there are extension rules in the main system that aren't in the initramfs?
Comment 4 Kay Sievers 2013-01-24 11:59:30 EST
The main system does run "add" again. There is no difference if an initramfs
is used or not, a full coldplug always happens in the main system.

But we need something for the dm/md "fake devices" which don't do anything
at "add" and only at "change".
Comment 5 Ayal Baron 2013-01-24 12:34:45 EST
Note that this is preventing vdsm from working properly
Comment 6 Itamar Heim 2013-01-24 22:11:43 EST
can we please raise the priority of this from unspecified - this is blocking ovirt 3.2 from working on fedora 18.

thanks,
   Itamar
Comment 7 Jóhann B. Guðmundsson 2013-01-25 05:11:18 EST
Federico Simoncelli please do not mess with Priority or Severity state of bugs in the Fedora project. 

You might be using this in Red Hat but in Fedora these fields are strictly used internally by maintainers themselves usually based on their freetime,workload and priority.
Comment 8 Federico Simoncelli 2013-01-25 05:35:36 EST
(In reply to comment #7)
> Federico Simoncelli please do not mess with Priority or Severity state of
> bugs in the Fedora project. 
> 
> You might be using this in Red Hat but in Fedora these fields are strictly
> used internally by maintainers themselves usually based on their
> freetime,workload and priority.

Since I opened the bug and nobody changed the priority yet you can take it as my personal opinion (as if it was done opening the bug, I suppose the fields are there to be used somehow by the reporter too).

Unspecified/unspecified wasn't reflecting the importance of the issue in the first place and I'm sure maintainers aren't afraid to change the priority/severity according to their own opinion/workload.
Comment 9 Jóhann B. Guðmundsson 2013-01-25 06:13:54 EST
(In reply to comment #8)
> (In reply to comment #7)
> > Federico Simoncelli please do not mess with Priority or Severity state of
> > bugs in the Fedora project. 
> > 
> > You might be using this in Red Hat but in Fedora these fields are strictly
> > used internally by maintainers themselves usually based on their
> > freetime,workload and priority.
> 
> Since I opened the bug and nobody changed the priority yet you can take it
> as my personal opinion (as if it was done opening the bug, I suppose the
> fields are there to be used somehow by the reporter too).

The only reason you actually see those fields are because you are a Red Hat employee. Community users/reporters dont see this we had it removed due to them always changing this state failing to understand this was used strictly being used internally with maintainers.  

I'm going to see if Red Hat employees cant have those privileges revoked  when they are working with bugs reported in the Fedoraproject as in use the same scheme as Fedora community member if not this will be added to the list why the Fedora and Red Hat should not be sharing bugzilla instances and I will personally maintain that instance if I have to.
Comment 10 Federico Simoncelli 2013-01-25 06:56:45 EST
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > Federico Simoncelli please do not mess with Priority or Severity state of
> > > bugs in the Fedora project. 
> > > 
> > > You might be using this in Red Hat but in Fedora these fields are strictly
> > > used internally by maintainers themselves usually based on their
> > > freetime,workload and priority.
> > 
> > Since I opened the bug and nobody changed the priority yet you can take it
> > as my personal opinion (as if it was done opening the bug, I suppose the
> > fields are there to be used somehow by the reporter too).
> 
> The only reason you actually see those fields are because you are a Red Hat
> employee. Community users/reporters dont see this we had it removed due to
> them always changing this state failing to understand this was used strictly
> being used internally with maintainers.

Actually the severity can be set by any community user.

> I'm going to see if Red Hat employees cant have those privileges revoked 
> when they are working with bugs reported in the Fedoraproject as in use the
> same scheme as Fedora community member if not this will be added to the list
> why the Fedora and Red Hat should not be sharing bugzilla instances and I
> will personally maintain that instance if I have to.

For as much as we all can enjoy this conversation sadly it's off-topic with the current issue and it's not focused on fixing the real problem.
I like your attitude to polish the process volunteering for such an important matter.
For now, as I said, I trust enough the maintainers to change the priority according to their schedule even though I already spoke with Kay (before opening this bug) and we agreed on the importance of a quick fix.

I have been personally involved in often fixing other component issues when the resources weren't enough, let's stay focused and at least point out a guideline for the fix. Is there any upstream thread/bug that we should track?
Comment 11 Alasdair Kergon 2013-01-25 07:00:56 EST
(In reply to comment #1)
> Recent udev versions only set the device node permissions and selinux context
> with "add" and never later.
> 
> We need to find some fix here ...

In which version did this change?
What was the reason?
Comment 12 Alasdair Kergon 2013-01-25 07:05:18 EST
Could the change be reverted and an alternative solution be worked out?
Comment 13 Federico Simoncelli 2013-01-25 07:10:48 EST
(In reply to comment #12)
> Could the change be reverted and an alternative solution be worked out?

We didn't have many technical details on this bz yet but my current understanding is that the "add" event should try to carry all the LVM info (DM_LV_NAME, etc) that "change" has. This might not be viable as the info might not be available at the "add" time. Is there anyone that can comment on this?
Comment 14 Alasdair Kergon 2013-01-25 08:02:46 EST
(In reply to comment #13)
> We didn't have many technical details on this bz yet but my current
> understanding is that the "add" event should try to carry all the LVM info
> (DM_LV_NAME, etc) that "change" has. This might not be viable as the info
> might not be available at the "add" time. 

That's indeed not viable.

The bug appears to be that the way udev handles change events on dm devices changed without full consideration of the consequences, and so in the short-term that change should be reverted.
Comment 15 Kay Sievers 2013-01-27 15:34:07 EST
(In reply to comment #14)
> (In reply to comment #13)
> > We didn't have many technical details on this bz yet but my current
> > understanding is that the "add" event should try to carry all the LVM info
> > (DM_LV_NAME, etc) that "change" has. This might not be viable as the info
> > might not be available at the "add" time. 
> 
> That's indeed not viable.

At "add", the dm device is just some kind of a dead placeholder, only after
the tables are set up, and "change" is sent, the device becomes usable.

> The bug appears to be that the way udev handles change events on dm devices
> changed without full consideration of the consequences, and so in the
> short-term that change should be reverted.

Currently, permissions are not applied any more at "change". We will need to
rework that, including the default rules matches, for f19, but it's not an
entirely trivial change.

In F18 this commit should probably just be reverted for now:
  http://cgit.freedesktop.org/systemd/systemd/commit/?id=48a849ee17fb25e0001bfcc0f28a4aa633d016a1
Comment 16 Michal Schmidt 2013-01-28 08:05:53 EST
(In reply to comment #15)
> In F18 this commit should probably just be reverted for now:
>  
> http://cgit.freedesktop.org/systemd/systemd/commit/
> ?id=48a849ee17fb25e0001bfcc0f28a4aa633d016a1

OK, I'll revert it in F18.
Comment 17 Fedora Update System 2013-01-28 14:12:30 EST
systemd-197-1.fc18.2 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/systemd-197-1.fc18.2
Comment 18 Fedora Update System 2013-01-29 19:33:40 EST
Package systemd-197-1.fc18.2:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing systemd-197-1.fc18.2'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2013-1590/systemd-197-1.fc18.2
then log in and leave karma (feedback).
Comment 19 Fedora Update System 2013-02-22 20:01:29 EST
Package initscripts-9.42.2-1.fc18, systemd-197-1.fc18.2:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing initscripts-9.42.2-1.fc18 systemd-197-1.fc18.2'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2013-1590/initscripts-9.42.2-1.fc18,systemd-197-1.fc18.2
then log in and leave karma (feedback).
Comment 20 Fedora Update System 2013-02-25 21:43:23 EST
initscripts-9.42.2-1.fc18, systemd-197-1.fc18.2 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.

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