Bug 24265 - Needs to create /var/run/udisks/ on demand
Needs to create /var/run/udisks/ on demand
Status: RESOLVED FIXED
Product: udisks
Classification: Unclassified
Component: general
unspecified
Other All
: medium normal
Assigned To: David Zeuthen (not reading bugmail)
:
: 36589 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-10-01 14:23 UTC by Martin Pitt
Modified: 2011-08-26 09:21 UTC (History)
3 users (show)

See Also:


Attachments
Use /tmp/ for temporary directories (899 bytes, patch)
2009-10-05 01:31 UTC, Martin Pitt
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Pitt 2009-10-01 14:23:03 UTC
./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.
Comment 1 David Zeuthen (not reading bugmail) 2009-10-02 07:57:18 UTC
> 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.
Comment 2 Martin Pitt 2009-10-05 00:43:30 UTC
$ 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.
Comment 3 Martin Pitt 2009-10-05 01:31:14 UTC
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.
Comment 4 Martin Pitt 2010-03-12 04:16:34 UTC
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.
Comment 5 David Zeuthen (not reading bugmail) 2010-03-12 06:38:02 UTC
(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.
Comment 6 Michael Biebl 2010-11-28 04:15:19 UTC
(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?
Comment 7 David Zeuthen (not reading bugmail) 2010-11-28 06:57:13 UTC
(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?
Comment 8 David Zeuthen (not reading bugmail) 2010-11-30 07:46:17 UTC
Asked Kay about this and he says to use systemd's /etc/tmpfiles.d mechanism to create the directory.
Comment 9 David Zeuthen (not reading bugmail) 2011-08-26 08:32:11 UTC
*** Bug 36589 has been marked as a duplicate of this bug. ***
Comment 10 David Zeuthen (not reading bugmail) 2011-08-26 08:38:37 UTC
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