Bug 1071224 - systemd: removes /usr/lib/systemd/system/tmp.mount in %post
systemd: removes /usr/lib/systemd/system/tmp.mount in %post
Status: VERIFIED FIXED
Classification: openSUSE
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Basesystem
Current
Other Other
: P5 - None : Normal (vote)
: ---
Assigned To: systemd maintainers
E-mail List
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-12-05 09:00 UTC by Ludwig Nussel
Modified: 2021-06-17 09:12 UTC (History)
5 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments
disable tmpfs on /tmp by removing /usr/lib/systemd/system/tmp.mount (4.16 KB, patch)
2018-02-08 16:46 UTC, Franck Bui
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ludwig Nussel 2017-12-05 09:00:48 UTC
systemd's %post script removes /usr/lib/systemd/system/tmp.mount. That breaks rpm -V systemd, therefore delta rpms for systemd. Don't do that. If you don't want the file don't ship it. To mask it use systemctl mask.
Comment 1 Ludwig Nussel 2017-12-05 09:01:09 UTC
Affects 42 and 14 too
Comment 2 Ludwig Nussel 2017-12-05 09:01:59 UTC
s/14/15
Comment 3 Franck Bui 2017-12-07 14:04:57 UTC
The problem with "stop shipping it" is that it would break any users who choose to use this unit file.

And masking the unit wouldn't help either since we don't want to mask tmp.mount if it's been generated from /etc/fstab or the user created his own version of the file.
Comment 4 Ludwig Nussel 2017-12-07 14:14:54 UTC
in any case you cannot just rm it, it's part of the package
Comment 5 Franck Bui 2017-12-07 14:18:16 UTC
You're right, however I'm not sure how we could fix this properly...
Comment 6 Ludwig Nussel 2017-12-07 14:24:48 UTC
Just don't do it. 15 will be a new major release, we document the change in behavior and be done with it.
Comment 7 Ludwig Nussel 2017-12-07 14:29:42 UTC
Note that by default we have a btrfs subvolume for /tmp that is mounted via fstab, so according to https://www.freedesktop.org/wiki/Software/systemd/APIFileSystems/ this takes preference ie no change in behavior for a default install on btrfs actually.
Comment 8 Franck Bui 2017-12-07 14:30:21 UTC
Just to make sure: you meant to stop shipping it and break any users that would rely on it ?
Comment 9 Franck Bui 2017-12-07 14:33:09 UTC
(In reply to Ludwig Nussel from comment #7)
> Note that by default we have a btrfs subvolume for /tmp that is mounted via
> fstab, so according to
> https://www.freedesktop.org/wiki/Software/systemd/APIFileSystems/ this takes
> preference ie no change in behavior for a default install on btrfs actually.

Correct but if a customer selected ext4 I'm not sure that /tmp is referenced in fstab.
Comment 10 Ludwig Nussel 2017-12-07 14:47:01 UTC
(In reply to Franck Bui from comment #8)
> Just to make sure: you meant to stop shipping it and break any users that
> would rely on it ?

For this case you could still copy a backup of it to /etc in %pre of the new package.
Comment 11 Ludwig Nussel 2017-12-07 14:51:45 UTC
another option would be to move the file to a subpackage.

Did we ever ship the file installed anyways?
Comment 12 Franck Bui 2017-12-07 15:03:59 UTC
How about this:

  1. check that tmp.mount is active: if so we can assume that either tmp.mount from fstab is used or customer has its own version in /etc/systemd/...

     In these cases we can safely ship the default tmp.mount in /usr/lib/systemd/system since the active version will take precedence.

 2. if tmp.mount is not active: we still ship tmp.mount but we mask it

WDYT ?
Comment 13 Franck Bui 2017-12-08 07:54:40 UTC
Answering to myself: that can't work during installation.

How about letting the installer sorting this out in this case ?

IOW it could mask tmp.mount unless it writes an entry in fstab for /tmp.
Comment 14 Ludwig Nussel 2017-12-08 15:04:09 UTC
The installer might work but then how about just removing the requirement on tmp.mount from basic.target? That should also prevent it from starting automatically, right?
Comment 15 Franck Bui 2017-12-08 15:52:50 UTC
But that would also break all users who are using it.
Comment 16 Ludwig Nussel 2017-12-14 15:03:52 UTC
another variant: tmp.mount could be removed from basic.target but an install section could be added:

[Install]
WantedBy=basic.target

That means one would have to explicitly run systemctl enable to get tmp.mount
Comment 17 Ludwig Nussel 2017-12-14 15:04:56 UTC
hmm, wonder how that plays with tmp.mount from fstab
Comment 18 Franck Bui 2017-12-20 10:19:35 UTC
Sorry for the delay...

Letting tmp.mount still accessible is pretty dangerous (even if basic.target won't pull it in) because there are several other ways to activate it implicitly: for example when a unit is using private /tmp.

So the only safe way I can see would be to mask the unit by default during the installation if no entry for /tmp is created in fstab by the installer.

Regarding the upgrade path, we could mask tmp.mount if no tmp.mount is currently active and restore /usr/lib/systemd/system/tmp.mount if not present. Otherwise if a tmp.mount unit is active, simply restoring the one shipped by udev should be enough.
Comment 19 Thorsten Kukuk 2018-01-19 11:58:30 UTC
(In reply to Franck Bui from comment #18)

> So the only safe way I can see would be to mask the unit by default during
> the installation if no entry for /tmp is created in fstab by the installer.

We have many installation ways and methods, and also upgrade has to work.
So, a solution based on special code in the installer violates with the requirement and goal, to have an universal usable installer. And it does not fix the problem for images (like build with kiwi), updates, rpm, SUSE Manager, and a lot of other cases I'm pretty sure I forgot to mention.

The only good, reliable solution is one, which works if you install with rpm -ihv into a chroot environment: the it will always work.

In my opinion, we should not ship tmp.mount in the default path, but as 'example' or documentation in /usr/share/doc/packages/systemd or so.
Yes, there is the risk to break very few installations, but I really doubt that people outside there use it, since we remove it if not activated. So how should an user ever activate it, if it is already removed at that point in time?
And if a user want's it, he can link the example to /etc/systemd/... or copy it.
Comment 20 Franck Bui 2018-01-19 14:37:30 UTC
(In reply to Thorsten Kukuk from comment #19)
> (In reply to Franck Bui from comment #18)
> 
> > So the only safe way I can see would be to mask the unit by default during
> > the installation if no entry for /tmp is created in fstab by the installer.
> 
> We have many installation ways and methods, and also upgrade has to work.

Updates of systemd package would take care of this.

> 
> In my opinion, we should not ship tmp.mount in the default path, but as
> 'example' or documentation in /usr/share/doc/packages/systemd or so.

That's what we're doing except that we care about users that already set up tmpfs on /tmp.

> Yes, there is the risk to break very few installations, but I really doubt
> that people outside there use it, since we remove it if not activated. So
> how should an user ever activate it, if it is already removed at that point
> in time?

I don't agree with this and I'm wondering how you did figure out that few users are using tmpfs. Actually I'm using tmpfs on /tmp.

And even if that would be true, breaking (voluntarily) user systems is a bad choice.
Comment 21 Thorsten Kukuk 2018-01-19 14:46:15 UTC
(In reply to Franck Bui from comment #20)
> (In reply to Thorsten Kukuk from comment #19)
> > (In reply to Franck Bui from comment #18)
> > 
> > > So the only safe way I can see would be to mask the unit by default during
> > > the installation if no entry for /tmp is created in fstab by the installer.
> > 
> > We have many installation ways and methods, and also upgrade has to work.
> 
> Updates of systemd package would take care of this.

But not fresh installations not done by YaST2, which are the majority of our non-openSUSE installations.

> > Yes, there is the risk to break very few installations, but I really doubt
> > that people outside there use it, since we remove it if not activated. So
> > how should an user ever activate it, if it is already removed at that point
> > in time?
> 
> I don't agree with this and I'm wondering how you did figure out that few
> users are using tmpfs. Actually I'm using tmpfs on /tmp.

But only either because you enabled it with a broken systemd package or by copying the file to /usr/lib/systemd/system/..., which was wrong, because you should have copied to /etc/systemd/...
If you install systemd, the tmp.mount is immediately deleted during the install process, so as normal openSUSE or SLE user, you cannot enable it because it is already deleted.
If you copy it correctly to /etc/systemd/... yourself, the removal of the file from systemd package will not break your system.
 
> And even if that would be true, breaking (voluntarily) user systems is a bad
> choice.

The systems are not broken, you only don't use tmpfs anymore for /tmp.

But it's still better that a few customers suffer than all customers by not beeing able to update systemd via delta-RPMs and by having warnings in their monitoring tools since rpm -Va reports missing files.
Comment 22 Franck Bui 2018-01-25 11:35:39 UTC
Ok so how about doing this:

--- %< ---

$ cat /etc/systemd/system/systemd-disable-tmpfs-for-tmp.service 
# By default, /tmp doesn't use tmpfs on SUSE distros.
#
# This script is either run automatically during the firstboot (i.e.
# only once) of the system.
#
# Or the service can also be (manually) started during systemd update
# (%post) only and only if tmp.mount wasn't installed in /usr/lib
# during %pre. In this case tmp.mount should also masked.

[Unit]
Description=Mask tmp.mount by default on SUSE systems
DefaultDependencies=no
Conflicts=shutdown.target
After=systemd-remount-fs.service
Before=tmp.mount
ConditionPathIsReadWrite=/etc
ConditionPathExists=!/usr/lib/systemd/scripts/.disable-tmp-mount-by-default.sh~done

[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/bin/sh -c '						\
	case "$(systemctl show -pFragmentPath tmp.mount)" in	\
	FragmentPath=/usr/lib/systemd/system/tmp.mount)		\
		systemctl mask --now tmp.mount ;;		\
	esac'

ExecStartPost=/usr/bin/touch /usr/lib/systemd/scripts/.disable-tmp-mount-by-default.sh~done

--- %< ---

this service should be statically pulled in by sysinit.target.

And in systemd.spec:

%pre
if test -e /usr/lib/systemd/system/tmp.mount; then
    # If tmp.mount is there it means that admin restored it and
    # wants to use tmpfs. In that case do nothing and prevent
    # the service to be executed during %post.
    touch /usr/lib/systemd/scripts/.disable-tmp-mount-by-default.sh~done
fi

%post
# Let's the service figure out what needs to be done, note
# that it's going to be executed only once.
systemctl start systemd-disable-tmpfs-for-tmp.service

That way we should support the various ways a SUSE system can be created (heck!) and it should be fairly simple to switch back to tmpfs for /tmp the day SUSE will decide to do it.

Ludwig, would that fill the gap ?
Comment 23 Ludwig Nussel 2018-01-25 16:54:58 UTC
I am still not convinced about any complex solution. I'd still favor the subpackage or shipping as example as Thorsten wrote.
Comment 24 Franck Bui 2018-01-26 09:58:49 UTC
(In reply to Ludwig Nussel from comment #23)
> I am still not convinced about any complex solution.

Fair enough.

But the previous solution is supposed to handle gracefully all the corner cases that we can encounter out there though.

Also it would ease the switch to "tmpfs for /tmp" the day SUSE will decide to make it the default since we would simply have to stop shipping the new service that figures out if tmp.mount needs to be masked during the first boot.

But if we're fine to forget those corner cases then shipping tmp.mount as a template (like we did already) only and recommending to symlink it from /etc if one wants to use tmpfs seems the simplest indeed.
Comment 25 Franck Bui 2018-01-29 08:02:54 UTC
After thinking more about it, I'll go for the solution posted in comment #22.

I think it's better because:

 1. it's supposed to support all cases

 2. masking tmp.mount is the official and documented way to disable tmpfs on /tmp

 3. it will be easier and cleaner to switch the defaults to tmpfs in the future.

I'll make the changes reach Factory first and see how it goes.
Comment 26 Franck Bui 2018-02-02 08:48:02 UTC
Ludwig, I guess this also needs to be fixed in SLE12-SP2+, right ?
Comment 27 Richard Brown 2018-02-08 09:38:22 UTC
(In reply to Franck Bui from comment #26)
> Ludwig, I guess this also needs to be fixed in SLE12-SP2+, right ?

The solution in Comment #22 has been submitted to factory

https://build.opensuse.org/package/rdiff/openSUSE:Factory/systemd?linkrev=base&rev=269

It is now broken in Kubic

https://openqa.opensuse.org/tests/603973#step/journal_check/14

You can not write to /usr/lib/systemd/scripts/ on a read only filesystem

We support read-only root file systems on Kubic (based on tumbleweed) and CaaSP 3 (12 SP3) and 4 (15)

I think this change needs to be reverted and reworked

It should not be submitted in it's current form to SLE12-SP2+ because it will break on SUSE CaaSP.
Comment 28 Franck Bui 2018-02-08 09:52:06 UTC
(In reply to Richard Brown from comment #27)
> 
> You can not write to /usr/lib/systemd/scripts/ on a read only filesystem

The problem happens when the system boots right, it doesn't happen when systemd package is upgraded ?

And how are we supposed to implement "run this script once" on these platforms ?

Thanks.
Comment 29 Richard Brown 2018-02-08 09:58:41 UTC
(In reply to Franck Bui from comment #28)
> (In reply to Richard Brown from comment #27)
> > 
> > You can not write to /usr/lib/systemd/scripts/ on a read only filesystem
> 
> The problem happens when the system boots right, it doesn't happen when
> systemd package is upgraded ?
> 
> And how are we supposed to implement "run this script once" on these
> platforms ?
> 
> Thanks.

The only time you can write to the rootfs on Kubic and CaaSP is during a package installation/upgrade - the rootfs is immutable, but the package changes are written to a new snapshot which will take effect on next boot

So the only place you COULD implement this logic is in the %post

I think you should not, I think this is an overly complex solution for little or no benefit. I think this is a perfect example in the flesh of why Ludwig and Thorsten were both right earlier in this thread.

Please reconsider their advised solutions of a subpackage or packaged examples
Comment 30 Franck Bui 2018-02-08 10:21:21 UTC
Well it's "overly" complex because the number of different platforms that are created in various ways (not only by YaST) we have to deal with is.

And actually it's not "overly" complex, it's a simple service that is run once during the first boot to figure out if it should mask tmp.mount and addresses points listed in comment #25 which is not the case for the other solution.

BTW what is used for /tmp on Kubic and CaaSP ?
Comment 31 Thorsten Kukuk 2018-02-08 11:02:44 UTC
(In reply to Franck Bui from comment #30)

> BTW what is used for /tmp on Kubic and CaaSP ?

Whatever the customers configured.
Comment 32 Thorsten Kukuk 2018-02-08 11:08:58 UTC
Products having or officially supporting read-only root filesystems:
- SLES 10 and newer
- CaaSP
- Kubic
Not sure about Tumbleweed, if the code is there, too, or not. But I doubt it's official supported there.

So whatever the solution will be, it has to work on a read-only root filesystem.
Comment 33 Franck Bui 2018-02-08 13:27:37 UTC
(In reply to Thorsten Kukuk from comment #32)
> 
> So whatever the solution will be, it has to work on a read-only root
> filesystem.

I'm fine to use the other solution but the main issue will remain and will need to be fixed the same way the day when SUSE will decide to make tmpfs the default for /tmp: everyone who don't use tmpfs will need to keep this setting once the default will be changed and the only solution I can see currently is to mask tmp.mount only *once*. And to implement that we need write access to /usr...

OTOH if we're fine with the fact that some users will switch to tmpfs during an update then the problem is solved.
Comment 34 Franck Bui 2018-02-08 16:46:53 UTC
Created attachment 759425 [details]
disable tmpfs on /tmp by removing /usr/lib/systemd/system/tmp.mount
Comment 35 Franck Bui 2018-02-08 16:47:08 UTC
In order to fix distros using a RO rootfs I implemented the other solution.

Richard, Thorsten, could you double check ?
Comment 36 Franck Bui 2018-02-09 08:25:02 UTC
OK I submitted the changes to Factory/SLE15.
Comment 37 Richard Brown 2018-02-09 09:04:19 UTC
(In reply to Franck Bui from comment #34)
> Created attachment 759425 [details]
> disable tmpfs on /tmp by removing /usr/lib/systemd/system/tmp.mount

I'm reasonably confident the change proposed should work on SLE 15, TW, Leap and _FRESH INSTALLATIONS_ of CaaSP and Kubic

But I expect the following line will fail on any upgrade of CaaSP or Kubic

ln -sf %{_datadir}/systemd/tmp.mount /etc/systemd/system/

IIRC /etc is not mounted during a transactional upgrade, as it's an overlayfs

It might fail silently (which is still not good) but if the script fails vocally, this will mean systemd will be un-upgradable on CaaSP and Kubic if systemctl show -pFragmentPath tmp.mount is true

Thorsten, am I missing something?
Comment 38 Ludwig Nussel 2018-02-09 09:31:30 UTC
(In reply to Franck Bui from comment #26)
> Ludwig, I guess this also needs to be fixed in SLE12-SP2+, right ?

I think it makes sense to include in any upcoming update to released code streams, yes.
Comment 39 Thorsten Kukuk 2018-02-09 09:55:11 UTC
(In reply to Richard Brown from comment #37)

> But I expect the following line will fail on any upgrade of CaaSP or Kubic
> 
> ln -sf %{_datadir}/systemd/tmp.mount /etc/systemd/system/
> 
> IIRC /etc is not mounted during a transactional upgrade, as it's an overlayfs

It will work fine.

The real /etc is available during upgrade, only the user modifications (which are stored in the overlayfs) are not. Which doesn't matter, in worst case we create a symlink, which is shadowed by the user modifications.
Comment 41 Franck Bui 2018-02-09 14:50:49 UTC
I submitted the fix to SLE15/Factory/SLE12-SP2+ so I think this can be closed.
Comment 46 Swamp Workflow Management 2018-02-26 20:08:16 UTC
SUSE-SU-2018:0546-1: An update that solves one vulnerability and has 5 fixes is now available.

Category: security (moderate)
Bug References: 1057974,1068588,1071224,1071311,1075801,1077925
CVE References: CVE-2017-18078
Sources used:
SUSE Linux Enterprise Software Development Kit 12-SP3 (src):    systemd-228-150.32.1
SUSE Linux Enterprise Software Development Kit 12-SP2 (src):    systemd-228-150.32.1
SUSE Linux Enterprise Server for Raspberry Pi 12-SP2 (src):    systemd-228-150.32.1
SUSE Linux Enterprise Server 12-SP3 (src):    systemd-228-150.32.1
SUSE Linux Enterprise Server 12-SP2 (src):    systemd-228-150.32.1
SUSE Linux Enterprise Desktop 12-SP3 (src):    systemd-228-150.32.1
SUSE Linux Enterprise Desktop 12-SP2 (src):    systemd-228-150.32.1
SUSE CaaS Platform ALL (src):    systemd-228-150.32.1
OpenStack Cloud Magnum Orchestration 7 (src):    systemd-228-150.32.1
Comment 47 Swamp Workflow Management 2018-02-28 02:08:55 UTC
openSUSE-SU-2018:0560-1: An update that solves one vulnerability and has 5 fixes is now available.

Category: security (moderate)
Bug References: 1057974,1068588,1071224,1071311,1075801,1077925
CVE References: CVE-2017-18078
Sources used:
openSUSE Leap 42.3 (src):    systemd-228-44.1, systemd-mini-228-44.1
Comment 48 Martin Loviska 2021-06-17 09:12:50 UTC
Closing as it does not occur in openQA tests