Bug#779902: /tmp can be mounted as tmpfs against user's will

2015-03-20 Thread Didier Roche

Le 20/03/2015 09:03, Michael Biebl a écrit :

[adding the bug to CC]
Am 20.03.2015 um 08:46 schrieb Didier Roche:

Le 20/03/2015 08:39, Michael Biebl a écrit :

thanks for the patch. I had something like this in mind.
We could be extra nice and only add the After=tmp.mount if tmp.mount is
actually enabled, because we only need the After ordering in this case.
But that's mostly a cosmetic issue and I'm happy to ship the patch as is.

That's a nice idea. It needs testing though to ensure that the fstab
generator generated the right enablement for that unit (in case the
tmpfs config was done in fstab).

/etc/fstab entries are automatically hooked up in
/run/systemd/generator/local-fs.target.requires/ unless the fstab entry
uses noauto.

That said, I'm not sure if we should really test the file system path.
We should check if systemd offers an API for this.

Also, we need to ensure that any later

enablement of that unit, this is taken into account by new units. Not
sure I'll have time to test this properly today, will be more early next
week if needed though.

As said, I'd probably be happy to ship the patch as is and would include
it in the upload I plan this weekend.

We can defer this to a later upload though, if you want?

Sounds the best approach: ship it as it is for this week-end upload, at 
least the immediate concern is addressed this way.
I'm keeping in mind to check if we can request for the unit enablement 
for a later upload.


(Note: the patch is against experimental, I can rebase on master and 
include the bug reference if you wish, but I guess you are going to 
merge other fixes as well…)


Thanks for the bug triaging work btw :)
Cheers,
Didier


--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#779902: /tmp can be mounted as tmpfs against user's will

2015-03-20 Thread Didier Roche

Hey,

Attaching the patch (which tries to be less intrusive with mounts, only
affecting /tmp) that I pinged on IRC for better tracking.
Tested under multiple configurations. /tmp isn't mounted as tmpfs
neither at boot, nor after a service restart having PrivateTmp. Enabling
the tmp mount unit now ensures that it's started at boot, before
services having PrivateTmp.

Cheers,
Didier


---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
http://www.avast.com
From 624f2a956a93acfd2da9132e991994c4e3218f2f Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Thu, 19 Mar 2015 08:53:03 +0100
Subject: [PATCH] Avoid /tmp being mounted as tmpfs without the user's will

Ensure PrivateTmp doesn't require tmpfs through tmp.mount, but rather adds
an After relationship.
---
 debian/changelog   |  2 ++
 .../PrivateTmp-shouldn-t-require-tmpfs.patch   | 24 ++
 debian/patches/series  |  1 +
 3 files changed, 27 insertions(+)
 create mode 100644 debian/patches/PrivateTmp-shouldn-t-require-tmpfs.patch

diff --git a/debian/changelog b/debian/changelog
index 9589d09..ea22101 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -4,6 +4,8 @@ systemd (219-5) UNRELEASED; urgency=medium
   * Add systemd-fsckd autopkgtest. (LP: #1427312)
   * Fix mount point detection on overlayfs and similar file systems without
 name_to_handle_at() and st_dev support. (LP: #1411140)
+  * Ensure PrivateTmp doesn't require tmpfs through tmp.mount, but rather adds
+an After relationship.
 
   [ Martin Pitt ]
   * journald: Suppress expected cases of Failed to set file attributes
diff --git a/debian/patches/PrivateTmp-shouldn-t-require-tmpfs.patch b/debian/patches/PrivateTmp-shouldn-t-require-tmpfs.patch
new file mode 100644
index 000..cef5628
--- /dev/null
+++ b/debian/patches/PrivateTmp-shouldn-t-require-tmpfs.patch
@@ -0,0 +1,24 @@
+From: Didier Roche didro...@ubuntu.com
+Date: Wed, 18 Mar 2015 17:11:00 +0100
+Subject: PrivateTmp shouldn't require tmpfs
+
+As PrivateTmp is requiring tmp.mount, this one will mount (but only after boot)
+/tmp as tmpfs adding a Requires=tmp.mount to the unit. This change downgrades
+the requirements to an after relationship.
+---
+ src/core/unit.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+Index: systemd-debian/src/core/unit.c
+===
+--- systemd-debian.orig/src/core/unit.c
 systemd-debian/src/core/unit.c
+@@ -807,7 +807,7 @@ int unit_add_exec_dependencies(Unit *u,
+ return 0;
+ 
+ if (c-private_tmp) {
+-r = unit_require_mounts_for(u, /tmp);
++r = unit_add_dependency_by_name(u, UNIT_AFTER, tmp.mount, NULL, true);
+ if (r  0)
+ return r;
+ 
diff --git a/debian/patches/series b/debian/patches/series
index 0a0e482..59b3524 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -70,3 +70,4 @@ cgroup-don-t-trim-cgroup-trees-created-by-someone-el.patch
 core-mount-ensure-that-we-parse-proc-self-mountinfo.patch
 Revert-journald-allow-restarting-journald-without-lo.patch
 path_is_mount_point-handle-false-positive-on-some-fs.patch
+PrivateTmp-shouldn-t-require-tmpfs.patch
-- 
2.1.4

___
Pkg-systemd-maintainers mailing list
pkg-systemd-maintain...@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-systemd-maintainers

Bug#779902: /tmp can be mounted as tmpfs against user's will

2015-03-06 Thread Didier Roche

Le 06/03/2015 16:04, Michael Biebl a écrit :

Am 06.03.2015 um 10:10 schrieb Martin Pitt:

Control: found -1 215-12
Control: tag -1 confirmed

Ça va Didier,

Didier Roche [2015-03-06  9:36 +0100]:

In debian, tmp.mount is disabled through a distro-patch by default. It means
we don't want user's system to get /tmp on tmpfs without explicit enablement
(either by enabling tmp.mount unit or via fstab).

We noticed that starting an unit using PrivateTmp=yes will pull tmp.mount
(which mounts /tmp on tmpfs) in its requirements chain (even if this unit is
condition fail).

Confirmed. systemctl start colord or systemd-timesyncd will start
tmp.mount and thus overmount the existing /tmp in the running system.
I reproduced that in a clean sid VM (with LXDE, but I suppose that
doesn't matter much).

The odd thing though is, that PrivateTmp=yes does not trigger the start
of tmp.mount during boot at least on all the test systems I have.

Do we know, why that is? A Required=tmp.mount should always start the
referenced unit, but it seemingly doesn't.


For reference, I dig a little bit into it and asked on the upstream ML: 
http://lists.freedesktop.org/archives/systemd-devel/2015-March/029072.html


Let's see if the bug we are seeing is really due to tmp.mount not being 
explicitly referenced anywhere.



--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#779902: /tmp can be mounted as tmpfs against user's will

2015-03-06 Thread Didier Roche

Package: systemd
Severity: serious

In debian, tmp.mount is disabled through a distro-patch by default. It 
means we don't want user's system to get /tmp on tmpfs without explicit 
enablement (either by enabling tmp.mount unit or via fstab).


We noticed that starting an unit using PrivateTmp=yes will pull 
tmp.mount (which mounts /tmp on tmpfs) in its requirements chain (even 
if this unit is condition fail).

For instance:
systemctl show -p Requires -p RequiresMountsFor systemd-timesyncd.service
Requires=-.mount tmp.mount
RequiresMountsFor=/var/lib/systemd/clock /tmp /var/tmp

Note that multiple units are using the private tmp feature like cups.

Also, if the unit is restarted (like an upgrade involving cups), /tmp 
will as well be remounted as tmpfs, which can wipe /tmp content to new 
process and get some annoying side-effects.



We need to find a way to ensure that tmp.mount is never accidentally 
trigger, while still enabling the user using fstab to enable /tmp as 
tmpfs. Enabling the unit to get the same effect would be a nice addition.


Bug#771739: could not start nodm with systemd 217-1 or 217-2

2014-12-02 Thread Didier Roche

Le 02/12/2014 12:28, Michael Biebl a écrit :

Am 02.12.2014 um 06:48 schrieb Martin Pitt:

積丹尼 Dan Jacobson [2014-12-02 13:36 +0800]:

# cat /etc/X11/default-display-manager
cat: /etc/X11/default-display-manager: No such file or directory

Indeed, nodm doesn't use this file at all, nor does it conflict with
any other window manager. So installing nodm together with gdm,
lightdm, kdm, etc. will just result in a disaster :-(

Didier, I'm afraid we have to exclude nodm from your DM masking -- I
don't see a way how we can fix this without actually fixing nodm.

I'm not sure if the generator is a good idea. I played around with that
idea a long time ago but discarded it again.


It seems the way Lennart advertised the use of a default configuration 
(like in /e/X/d-d-m) keeping in sync with Aliases like 
display-manager.service.

It also changes the behaviour of the insserv generator, which no longer
generates dependencies if native units exist.


systemd does that itself as well anyway (I didn't put that on the 
overriding stack on purpose). I can condition that masking for 
display-manager only if you feel more comfortable that way.
Note that this patch modification is for jessie-only and should be 
removed as being useless as soon as all dms ship systemd units.


At the current stage, I don't think these patches should be merged as is
for master.

Didier, Martin: could you elaborate what those patches try to fix?


This is to fix multiple bug reports where people don't have (or have 
multiple) dm services starting. Also, it enables people changing 
manually /e/X/d-d-m to not reboot in a state where no dm is started.
The use case is to ensure that display-manager.service will always 
points to a systemd service that will be runnable (if any). Meaning that 
we won't have failing units due to ExecStartPre failing and having that 
shell execution, or disable them if there is an init dm matching. It's 
going to enable us as well to migrate to Alias=display-manager.service 
one dm service after another instead in jessie+1 than doing the 
transition in a batch.


Precise behavior in coordination to the 2 generators is:
- if /etc/X11/default-display-manager matches a systemd unit and matches 
display-manager.service:
  - noop on the systemd side, mask all dms sysvinit not matching a 
systemd unit
- if /etc/X11/default-display-manager matches systemd unit and but 
doesn't match what's pointed by display-manager.service:
  - we ensure display-manager.service transiantly points to correct 
systemd unit, we mask all dms sysvinit not matching a systemd unit

- if /etc/X11/default-display-manager matches a non systemd unit:
  - we transiently mask display-manager.service (so no systemd unit 
startup) and mask all sysvinit dm scripts, but the one matching 
/etc/X11/default-display-manager

- if no /etc/X11/default-display-manager:
   - we even don't look at the systemd side (we hope there will be one 
display-manager.service matching at least one), we mask all sysvinit dm 
scripts


The last case is what is at fault here. I would suggest that we change 
the patch:
- if /etc/X11/default-display-manager matches anything at all or doesn't 
exist, we avoid masking any sysvinit dm script if there is no 
display-manager.service matching a real service file.


Didier


--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#771739: could not start nodm with systemd 217-1 or 217-2

2014-12-02 Thread Didier Roche

Le 02/12/2014 13:44, Michael Biebl a écrit :

Am 02.12.2014 um 13:06 schrieb Didier Roche:

This is to fix multiple bug reports where people don't have (or have
multiple) dm services starting.

Can you provide links to such bug reports so we have a bit of a context?


Sure:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=748668
and
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=770404
in particular.

Those generators will enable in jessie+1 to implement something like in 
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=764607 without needing 
all DM to transition to this scheme.



--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#771739: could not start nodm with systemd 217-1 or 217-2

2014-12-02 Thread Didier Roche

Le 02/12/2014 14:33, Michael Biebl a écrit :

Am 02.12.2014 um 14:17 schrieb Didier Roche:

Le 02/12/2014 13:44, Michael Biebl a écrit :

Am 02.12.2014 um 13:06 schrieb Didier Roche:

This is to fix multiple bug reports where people don't have (or have
multiple) dm services starting.

Can you provide links to such bug reports so we have a bit of a context?


Sure:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=748668
and
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=770404
in particular.

Those are bugs in the lxdm and slim packages and need to be fixed there.
Trying to workaround that in systemd looks like the wrong approach.


Hence my first patch about it, however, bigon asked the release team 
which nacked doing this for jessie. That's why I'm telling this patch is 
intended to be a bandaid until we get there.



Those generators will enable in jessie+1 to implement something like in
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=764607 without needing
all DM to transition to this scheme.

What exactly do you mean with this scheme? Are you referring to having
native .service files ship an Alias=display-manager.service in their
[Install] section?


Exactly, as intended upstream and done in other distributions.


--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#771653: systemd 217 breaks X11 (not more kdm and loggin and using startx gives me no mouse)

2014-12-01 Thread Didier Roche

Control: severity -1 important
Control: tag -1 moreinfo

Hey,

I tried this morning on a vm (with multiple dms installed: lightdm, xdm, 
gdm and kdm) and when choosing kdm as the alternative to choose in the 
postinst and checking the status, all seems fine:

- all other units are disabled,
- display-manager.service is masked (as the default dm is an init script)
- generated kdm.service is chosen

Do you mind showing the output (as root) of:
systemctl status kdm.service
systemctl status display-manger.service

Thanks!


--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#771287: systemd: Multiple DMs can be started at the same time or none under systemd

2014-11-28 Thread Didier Roche

Package: systemd
Version: 215-7
Severity: serious


We are starting to see multiple bug reports about several display 
managers started at boot. In addition, if 
/etc/X11/default-display-manager if changed manually, we can end up in a 
situation where no display manager is started at all on system boot.


This is due to the fact that some display manager are using init scripts 
only, and the dynamic converted version of this unit is then started. 
Some systemd units as well are not checking 
/etc/X11/default-display-manager (see 
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=748668 for more details).


One solution would be to convert all dms to ship systemd services with 
the same template and /etc/X11/default-display-manager check. One 
side-effect is that if you install multiple dms, you will always end up 
with a Status: degraded in systemctl.


A less invasive solution is to create and patch systemd generators to 
dynamically (at each boot) mask in memory all sysv-init scripts-only 
services that don't match /etc/X11/default-display-manager, and redirect 
the default configured dm to the selected systemd or sysv-init unit.



--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#770404: systemd: breaks lightdm, does not start anymore

2014-11-25 Thread Didier Roche
Actually, I think we can settle it that way (which would help fixing 
fixing bug #770633:


Everytime we have a service which provided an alternative, we declare an 
Alias=this alternative.service name. Then, we patch systemctl to 
update the alternative file (if it exists) on this alias name for those 
units having an alias.

That's fixing the systemctl enable service - put in sync the alternative.

Then, we patch update-alternative to look for a correspond .service 
name, and if any (and systemctl installed), we call systemctl enable 
--force service (and need to find a way to not have the circular 
dependency on both, like checking the alternatives file directly)

That's fixing the update-alternative command - syncing on systemctl state.

With this, we can even remove the tweaked postinst for the DMs as the 
normal alternative prompts will do the right thing.
That of course wouldn't fix people changing the alternatives file by 
hand, but I'm unsure we can gracefully handle this.


What do you think?
Didier


--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#770404: systemd: breaks lightdm, does not start anymore

2014-11-24 Thread Didier Roche

Le 21/11/2014 22:46, Sjoerd Simons a écrit :

reassign 770404 lxdm
thanks

On Fri, Nov 21, 2014 at 08:01:50PM +, Simon McVittie wrote:

This looks wrong. I think it might be caused by this in lxdm.service:

 [Install]
 Alias=display-manager.service

Neither gdm3 nor lightdm have that, which suggests that it isn't
meant to be necessary.

I think what's happening is that when you install lxdm, that Alias directive
causes the debhelper snippets in its postinst[1] to break the mechanism
that is meant to arbitrate who owns display-manager.service: the part of
its postinst headed # set default-display-manager systemd service link
is correct, but then the #DEBHELPER# snippet runs systemctl enable lxdm
which sees the Alias, obeys it, and overwrites the display-manager.service
symlink with an incorrect target.

Correct, that Alias= breaks our current mechanism for arbitrating the DM to use
(that is, the sylink and the config file go out of sync).

See also Martin pitt's comment for lightdm way back when:
   https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=733220#25


Actually, we revisited that with Martin and we can use Alias in relation 
to the correct postinst scripts to achieve this in a clean way.

See the snippet I proposed on bug #764607.

Cheers,
Didier


--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#770404: systemd: breaks lightdm, does not start anymore

2014-11-24 Thread Didier Roche

Le 24/11/2014 10:01, Sjoerd Simons a écrit :

On Mon, 2014-11-24 at 09:31 +0100, Didier Roche wrote:

Le 21/11/2014 22:46, Sjoerd Simons a écrit :

reassign 770404 lxdm
thanks

On Fri, Nov 21, 2014 at 08:01:50PM +, Simon McVittie wrote:

This looks wrong. I think it might be caused by this in lxdm.service:

  [Install]
  Alias=display-manager.service

Neither gdm3 nor lightdm have that, which suggests that it isn't
meant to be necessary.

I think what's happening is that when you install lxdm, that Alias directive
causes the debhelper snippets in its postinst[1] to break the mechanism
that is meant to arbitrate who owns display-manager.service: the part of
its postinst headed # set default-display-manager systemd service link
is correct, but then the #DEBHELPER# snippet runs systemctl enable lxdm
which sees the Alias, obeys it, and overwrites the display-manager.service
symlink with an incorrect target.

Correct, that Alias= breaks our current mechanism for arbitrating the DM to use
(that is, the sylink and the config file go out of sync).

See also Martin pitt's comment for lightdm way back when:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=733220#25

Actually, we revisited that with Martin and we can use Alias in relation
to the correct postinst scripts to achieve this in a clean way.
See the snippet I proposed on bug #764607.

Adding Bigon and Joss to the CC for their GDM perspective ;)

Interesting. Your postinst (patch from the bug you mentioned) seems a
bit brute-force though (it tries to (re)enable the default display
manager regardless of whether it, itself, is the default). But at least
that means things stay in sync :p

However I guess that still breaks things if the user enables a
non-default DM by hand ? (iotw the DM gets enabled but will bail
resulting in no DM being started if the user doesn't also update the etc
file)




Indeed, but even manually crafted solutions will have that issue as well 
(manual symlinks) as the alias is just the systemd way to do that 
automatically on systemctl enable unit.
We are currently discussing with Laurent to maybe ensure that all dms 
(the 7 in debians) have some service file as a RC bug to avoid such 
things to happen. However, there is an issue with gdm3 due to it being a 
symlink (will raise a bug once we found a proper way to avoid this)


Cheers,
Didier


--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org