./src/job-mkfs.c now uses mkdtemp() in /var/run/DeviceKit-disks, but does not create this directory if it does not exist. This regressed in commit 84743d97547388116a31f9082b8b2c9470b34572, before that it just used /tmp/ (which is really what /tmp/ is for, after all, but I understand that it might cause trouble with SELinux). Often, /var/run/ is on a tmpfs nowadays, so creating it on "make install" (as the current source does) is not sufficient. I see several possibilities: - just mkdir(PACKAGE_LOCALSTATE_DIR "/run/DeviceKit-disks") before mkdtemp() - use mkdtemp() in /var/run directly - revert to /tmp/ - Create an init script to mkdir /var/run/DeviceKit-disks (but eww, this is a hell of a lot of boot time overhead for a simple mkdir) I'm fine with providing a patch, but I'd like to know your opinion first before starting to patch. Under the assumption that you want to avoid /tmp/, I'd just go with option 2, or 1 if that makes SELinux rules any easier.
> Often, /var/run/ is on a tmpfs nowadays, so creating it on "make install" > (as the current source does) is not sufficient. First, I think a lot of other programs do this and the FHS specifically allows for it ("Programs may have a subdirectory of /var/run; this is encouraged for programs that use more than one run-time file."). Second, 'make install' should use (and does, at least with rpm and deb) DESTDIR so at least the package payload should be correct. Third, I think if you are careful you can still have /var/run on a tmpfs if you really want. You just need to have some logic that copies the directories over from the real /var/run directory at boot time. You also need some logic when you do package install/uninstalls at run-time. Btw, won't you see the same problem with lots of other software? I can imagine a piece of software even creating /var/run/foo that is owned by the user foo and all of the executables for that software run as the foo user. You can't do mkdir in any of these executables because /var/run is not writable for foo. Then there's also extended attributes for these directories - e.g. SELinux security contexts.
$ Btw, won't you see the same problem with lots of other software? Mixed. Some software, like cups, mkdirs/chowns itself, for others (like dbus) we need to do the mkdir/chown in the init/upstart script. If we need an upstart script for a daemon anyway, then the mkdir/chown is not a biggie. But installing an upstart job for _just_ an mkdir is quite a large overhead; also, this is a bit tricky with d-bus activated services since you have to ensure that the mkdir happens before d-bus startup. The "make install" approach works here because the directory is owned by root:root (and thus having a subdirectory isn't even strictly necessary). It does not work any more for services which require a system user-owned subdirectory, since you can't put the system user/group (>= 100, < 500) IDs into the .rpm/.deb. Anyway, I'm fine with keeping this as a local patch as well, if you don't want/need it in Fedora/RH.
Created attachment 30064 [details] [review] Use /tmp/ for temporary directories So, just for the record, I used option (2) for Debian/Ubuntu. It's a classic usage of a temporary file/dir. It's not state which needs to be kept through the lifetime of DK-disks, so it doesn't really need to go into /var/run/ (https://bugzilla.redhat.com/show_bug.cgi?id=491744 does not really give a convincing reason why /var/run/ should be used). Keeping that here for transparency, or for other distros which might have the same problem.
David, do you think the attached patch is okay? It uses /tmp/ for what it's supposed to be used for, but I realize this might need some feedback from your SELinux folks? Otherwise this should probably be closed as "wontfix", and we just keep it in the distros.
(In reply to comment #4) > David, do you think the attached patch is okay? It uses /tmp/ for what it's > supposed to be used for, but I realize this might need some feedback from your > SELinux folks? > > Otherwise this should probably be closed as "wontfix", and we just keep it in > the distros. > I still think it's better to use /var/run/udisks... and I think something is wrong with the OS if the /var/run/udisks isn't available. FWIW, the FHS is very very clear on this ("Programs may have a subdirectory of /var/run; this is encouraged for programs that use more than one run-time file.") and I doubt udisks is the only piece of software you need to patch. I'm closing the bug as WONTFIX.
(In reply to comment #5) > I still think it's better to use /var/run/udisks... and I think something is > wrong with the OS if the /var/run/udisks isn't available. FWIW, the FHS is very > very clear on this ("Programs may have a subdirectory of /var/run; this is > encouraged for programs that use more than one run-time file.") and I doubt > udisks is the only piece of software you need to patch. I'm closing the bug as > WONTFIX. Now that Lennart has switched Fedora Rawhide over to use tmpfs for /var/run and /var/lock, this issue will hit Fedora users too, not only Debian/Ubuntu. What's your plan to address this in Fedora?
(In reply to comment #6) > (In reply to comment #5) > > > I still think it's better to use /var/run/udisks... and I think something is > > wrong with the OS if the /var/run/udisks isn't available. FWIW, the FHS is very > > very clear on this ("Programs may have a subdirectory of /var/run; this is > > encouraged for programs that use more than one run-time file.") and I doubt > > udisks is the only piece of software you need to patch. I'm closing the bug as > > WONTFIX. > > Now that Lennart has switched Fedora Rawhide over to use tmpfs for /var/run and > /var/lock, this issue will hit Fedora users too, not only Debian/Ubuntu. > > What's your plan to address this in Fedora? Why are you asking me and not Lennart about this? :-) Lennart?
Asked Kay about this and he says to use systemd's /etc/tmpfiles.d mechanism to create the directory.
*** Bug 36589 has been marked as a duplicate of this bug. ***
In the mean-time this also became a problem on Fedora (on where I supposedly maintain the udisks packages) and it's apparent that people (including Lennart) are not reading the FHS the same way I am (as per comment 1) - could be I was reading it wrong then or being too stubborn or whatever :-) Anyway, there's no point in not having the fix in the code - I went for this http://cgit.freedesktop.org/udisks/commit/?id=bd0e9b6f62957f7bc99d01edaedb45a2e8ed8ac8
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.