Re: [systemd-devel] systemd + ExecStart + python script
On 18/11/13 19:24, Zbigniew Jędrzejewski-Szmek wrote: On Mon, Nov 18, 2013 at 05:56:47PM +0100, Abdelghani Ouchabane wrote: From my understanding I can combine PathChanged PathExists but it did not work for me. They must be *both* satisfied. I you want the unit to start when *either* is satisfied, you should use trigger conditions, written like PathExists=| Zbyszek Thanks a lot. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 11/18/2013 05:45 PM, Michal Sekletar wrote:
On Mon, Nov 18, 2013 at 04:19:20PM -0500, Daniel J Walsh wrote: On
11/16/2013 08:10 AM, Lennart Poettering wrote:
On Thu, 14.11.13 15:43, Daniel J Walsh (dwa...@redhat.com) wrote:
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1
On 11/14/2013 12:50 PM, Harald Hoyer wrote:
On 11/05/2013 11:12 PM, Daniel J Walsh wrote:
On 11/05/2013 12:22 PM, Lennart Poettering wrote:
Ok lets add a check that checks for start on a service labeled
with the remote process label, then we can add rules like
allow systemd_logind_t self:service start
Or we can make it simpler and have the local end check against
the init_t process.
allow systemd_logind_t init_t:service start;
Which is probably a better solution, if we have no way of
differentiating the services.
Machineid usually runs as init_t now.
systemd-run runs as the label of the process that executes it,
Usually unconfined_t, and sysadm_t.
has any solution been found for this?
seems like one is needed for
https://bugzilla.redhat.com/show_bug.cgi?id=1008864
I guess the question I have is do you expect a patch from me? Or
are you guys working on it? I would go with the checking based on
process label.
I am hoping for a patch for this!
Thanks,
Lennart
This patch adds a new call for SELINUX_SNAPSHOT_ACCESS_CHECK, because I
believe this error will happen when a snapshot is created. Also now pass
in system when doing a system check, if it is doing a service check and
does not pass in a unit file we will default the target to the label that
systemd is running with.
Hi,
Maybe I am missing something but isn't this about transient units in
general, i.e. what about StartTransient()? What is going to change in
this case after applying this patch? tclass will be system since in
SELINUX_ACCESS_CHECK you now pass system as path and you will set
tclass in else branch to system which is afaik same as before.
In the current code, passing a unit_file of NULL (StartTransients has a NULL
UnitFile) will indicate that it should do a system check. My patch is
intended to change this so a NULL UnitFile will indicate that systemd should
check the access between the calling process label and the current process
label for a service access. Where as the SELINUX_ACCESS_CHECK will now pass
a system flag to the function to make it do a system check.
On the side note, you forgot to define SELINUX_SNAPSHOT_ACCESS_CHECK as
do {} while (false) in case if we don't HAVE_SELINUX.
It might be the case that I completely misunderstood what's this about,
in such case ignore this email.
Michal
Thanks added SELINUX_SNAPSHOT_ACCESS_CHECK as do {} while (false) in case if
we don't HAVE_SELINUX.
Updated patch.
snip
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlKLbaEACgkQrlYvE4MpobPMMACeNeyrC3OBhx99DZ+JzOtV1ykZ
PvMAoJfiYBoJRBFgh2+FyOV+tNTuojNU
=9I5G
-END PGP SIGNATURE-
From b372b48016f5c015b34db9f53b7a54a64a5a84e8 Mon Sep 17 00:00:00 2001
From: Dan Walsh dwa...@redhat.com
Date: Mon, 18 Nov 2013 15:52:37 -0500
Subject: [PATCH] Fix SELinux check for snapshot and transitent unit creation.
SELinux does not have a path to check for a snapshot or transient
unit files service creation. Currently systemd does a bogus check.
If we don't have a unit file for a snapshot or transient unit we i
should check if the remote process label, has the required access
for a service with the SELinux label that systemd is running with.
Add SELINUX_SNAPSHOT_ACCESS_CHECK for non SELinux environments
---
src/core/dbus-manager.c | 2 +-
src/core/selinux-access.c | 9 +
src/core/selinux-access.h | 13 +
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 747bcfc..a60a568 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1112,7 +1112,7 @@ static DBusHandlerResult bus_manager_message_handler(DBusConnection *connection,
dbus_bool_t cleanup;
Snapshot *s;
-SELINUX_ACCESS_CHECK(connection, message, start);
+SELINUX_SNAPSHOT_ACCESS_CHECK(connection, message, start);
if (!dbus_message_get_args(
message,
diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index c7e951c..af34b9e 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -374,8 +374,9 @@ int selinux_access_check(
goto finish;
}
-if (path) {
-tclass = service;
+
+tclass = service;
+if (path strneq(path,system, strlen(system))) {
/* get the file context of the unit file */
r = getfilecon(path, fcon);
if (r 0)
Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units
On Tue, Nov 19, 2013 at 08:54:41AM -0500, Daniel J Walsh wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 11/18/2013 05:45 PM, Michal Sekletar wrote:
On Mon, Nov 18, 2013 at 04:19:20PM -0500, Daniel J Walsh wrote: On
11/16/2013 08:10 AM, Lennart Poettering wrote:
On Thu, 14.11.13 15:43, Daniel J Walsh (dwa...@redhat.com) wrote:
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1
On 11/14/2013 12:50 PM, Harald Hoyer wrote:
On 11/05/2013 11:12 PM, Daniel J Walsh wrote:
On 11/05/2013 12:22 PM, Lennart Poettering wrote:
Ok lets add a check that checks for start on a service labeled
with the remote process label, then we can add rules like
allow systemd_logind_t self:service start
Or we can make it simpler and have the local end check against
the init_t process.
allow systemd_logind_t init_t:service start;
Which is probably a better solution, if we have no way of
differentiating the services.
Machineid usually runs as init_t now.
systemd-run runs as the label of the process that executes it,
Usually unconfined_t, and sysadm_t.
has any solution been found for this?
seems like one is needed for
https://bugzilla.redhat.com/show_bug.cgi?id=1008864
I guess the question I have is do you expect a patch from me? Or
are you guys working on it? I would go with the checking based on
process label.
I am hoping for a patch for this!
Thanks,
Lennart
This patch adds a new call for SELINUX_SNAPSHOT_ACCESS_CHECK, because I
believe this error will happen when a snapshot is created. Also now pass
in system when doing a system check, if it is doing a service check and
does not pass in a unit file we will default the target to the label that
systemd is running with.
Hi,
Maybe I am missing something but isn't this about transient units in
general, i.e. what about StartTransient()? What is going to change in
this case after applying this patch? tclass will be system since in
SELINUX_ACCESS_CHECK you now pass system as path and you will set
tclass in else branch to system which is afaik same as before.
In the current code, passing a unit_file of NULL (StartTransients has a NULL
UnitFile) will indicate that it should do a system check. My patch is
intended to change this so a NULL UnitFile will indicate that systemd should
check the access between the calling process label and the current process
label for a service access. Where as the SELINUX_ACCESS_CHECK will now pass
a system flag to the function to make it do a system check.
Hi Dan,
Agreed, I get the idea, but this means that SELINUX_SNAPSHOT_ACCESS_CHECK should
be performed in StartTransient branch in dbus-manager.c too and macro should be
probably renamed to something like SELINUX_RUNTIME_UNIT_ACCESS_CHECK.
Hope that makes sense.
Michal
On the side note, you forgot to define SELINUX_SNAPSHOT_ACCESS_CHECK as
do {} while (false) in case if we don't HAVE_SELINUX.
It might be the case that I completely misunderstood what's this about,
in such case ignore this email.
Michal
Thanks added SELINUX_SNAPSHOT_ACCESS_CHECK as do {} while (false) in case if
we don't HAVE_SELINUX.
Updated patch.
snip
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlKLbaEACgkQrlYvE4MpobPMMACeNeyrC3OBhx99DZ+JzOtV1ykZ
PvMAoJfiYBoJRBFgh2+FyOV+tNTuojNU
=9I5G
-END PGP SIGNATURE-
From b372b48016f5c015b34db9f53b7a54a64a5a84e8 Mon Sep 17 00:00:00 2001
From: Dan Walsh dwa...@redhat.com
Date: Mon, 18 Nov 2013 15:52:37 -0500
Subject: [PATCH] Fix SELinux check for snapshot and transitent unit creation.
SELinux does not have a path to check for a snapshot or transient
unit files service creation. Currently systemd does a bogus check.
If we don't have a unit file for a snapshot or transient unit we i
should check if the remote process label, has the required access
for a service with the SELinux label that systemd is running with.
Add SELINUX_SNAPSHOT_ACCESS_CHECK for non SELinux environments
---
src/core/dbus-manager.c | 2 +-
src/core/selinux-access.c | 9 +
src/core/selinux-access.h | 13 +
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 747bcfc..a60a568 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1112,7 +1112,7 @@ static DBusHandlerResult
bus_manager_message_handler(DBusConnection *connection,
dbus_bool_t cleanup;
Snapshot *s;
-SELINUX_ACCESS_CHECK(connection, message, start);
+SELINUX_SNAPSHOT_ACCESS_CHECK(connection, message, start);
if (!dbus_message_get_args(
message,
diff --git a/src/core/selinux-access.c
Re: [systemd-devel] French translation for systemd
You're right, my bad. Here is an updated patch to fix these issues. Le 18 novembre 2013 17:22, Jean-Michel Pollion jeanmichel.poll...@gmail.com a écrit : 2013/11/18 Sylvain Plantefeve sylvain.plantef...@gmail.com Ok, thanks, I'll have a look at it. BTW, while re-reading the catalog again, I found a couple of typos... --- diff --git a/catalog/systemd-fr.catalog b/catalog/systemd-fr.catalog index 92cc15f..dd7af91 100644 --- a/catalog/systemd-fr.catalog +++ b/catalog/systemd-fr.catalog @@ -128,7 +128,7 @@ Subject: Le démarrage du système est terminé Defined-By: systemd Support: http://lists.freedesktop.org/mailman/listinfo/systemd-devel -Tous les services nécéssaires au démarrage du système ont tous été lancés +Tous les services nécessaires au démarrage du système ont tous été lancés avec succès. Notez que cela ne signifie pas que le système est maintenant au repos, car des services peuvent encore être en train de terminer leur démarrage. That's still an incorrect sentence in French, using tous twice is wrong, you should use either one of these instead : - Tous les services nécessaires au démarrage du système ont été lancés... - Les services nécessaires au démarrage du système ont tous été lancés... Either one should be OK. diff --git a/catalog/systemd-fr.catalog b/catalog/systemd-fr.catalog index 92cc15f..3ec7d8e 100644 --- a/catalog/systemd-fr.catalog +++ b/catalog/systemd-fr.catalog @@ -128,8 +128,8 @@ Subject: Le démarrage du système est terminé Defined-By: systemd Support: http://lists.freedesktop.org/mailman/listinfo/systemd-devel -Tous les services nécéssaires au démarrage du système ont tous été lancés -avec succès. Notez que cela ne signifie pas que le système est maintenant au +Tous les services nécessaires au démarrage du système ont été lancés avec +succès. Notez que cela ne signifie pas que le système est maintenant au repos, car des services peuvent encore être en train de terminer leur démarrage. @@ -137,7 +137,7 @@ Le chargement du noyau a nécessité @KERNEL_USEC@ microsecondes. Le chargement du « RAM disk » initial a nécessité @INITRD_USEC@ microsecondes. -Le chargement de l'espace utilisateur a nécéssité @USERSPACE_USEC@ microsecondes. +Le chargement de l'espace utilisateur a nécessité @USERSPACE_USEC@ microsecondes. -- 6bbd95ee977941e497c48be27c254128 fr Subject: Le système entre dans l'état de repos (sleep state) @SLEEP@ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
On Mon, 2013-11-18 at 19:35 -0500, Colin Walters wrote:
And that's what I'm testing - with Martin's patch in the loop I was
still getting XDG_DATA_DIR for uid 1000, I'll try to debug soon.
Ok, some discussion on IRC revealed that I was only using the second
patch to s/loginuid/uid/, but we need the first patch too.
Before I continue, a quick TL;DR: For downstreams that want to carry a
noninvasive fix for this problem, Martin's 2 patches are OK.
But I wanted to do a bit more due diligence on this and attempt to
understand where Lennart is coming from, and why the code is how it is
today.
Some archeology with git points to this commit:
http://cgit.freedesktop.org/systemd/systemd/commit/?id=21c390ccd1b4f7bc962c16549df929ad518a1d37
Quite old now...and there's not a lot of explanation as to the
real-world scenario that caused it to be introduced.
There's a followup commit here:
http://cgit.freedesktop.org/systemd/systemd/commit/?id=954449b82df7fc2d37a8d854977a1a73a65edfbd
with some more information. Now one thing to note is: we're using the
uid of the original session as the source for all information, including
the runtime dir.
Martin's first patch is creating a situation where logind says one
thing, pam module overrides it. But both come from the same git module,
we can fix logind.
I'm attaching a patch (for discussion only) that needs to go on top of
Martin's pam: Don't use loginuid patch.
Both of our patch series currently are basically going to have the
effect that with pkexec, XDG_RUNTIME_DIR is unset. But this is
undesirable because it forces the rest of userspace to go back to the
old dark ages when XDG_RUNTIME_DIR didn't exist and there was no
reliable mechanism for two processes of equal uid but different sessions
find each other and communicate. (Think ssh login + gdm login).
My patch though starts to pave the way for having XDG_RUNTIME_DIR
consistently point to that of the user's uid - I am increasingly
thinking the option #3 I mention in the patch where we refcount the
runtime dir in addition to users and sessions would work.
From 7fe51a82a5cc1ad2185b4ee4faa087f17e27d24a Mon Sep 17 00:00:00 2001
From: Colin Walters walt...@verbum.org
Date: Tue, 19 Nov 2013 10:21:16 -0500
Subject: [PATCH] login: Make XDG_RUNTIME_DIR match target uid with pkexec/sudo/su
These get attached to the same session, but we should ensure that the
XDG_RUNTIME_DIR is matches the new uid. Otherwise bad things can
happen such as a uid 0 process writing files into uid 1000's runtime
dir, corrupting it.
This patch currently has the semantics that if you log in via e.g. gdm
as non-root and then use pkexec, the new shell will have *no*
XDG_RUNTIME_DIR.
If however root is logged in for real elsewhere via getty or ssh,
then the XDG_RUNTIME_DIR will be set. However, once root logs out,
that directory will be garbage collected, which is probably
undesirable. We could either:
1) Attempt to make pkexec/su/sudo have real sessions (invasive)
2) Drop this hunk of the patch, and *always* leave XDG_RUNTIME_DIR
unset (simple)
3) Add internal refcounting for XDG_RUNTIME_DIR such that the user
session holds a ref on the root-owned runtime dir (fairly noninvasive)
---
src/login/logind-dbus.c | 17 ++---
src/login/pam-module.c | 10 ++
2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 0f25e23..397750d 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -553,10 +553,21 @@ static int method_create_session(sd_bus *bus, sd_bus_message *message, void *use
if (session) {
_cleanup_free_ char *path = NULL;
_cleanup_close_ int fifo_fd = -1;
+User *target_user;
+const char *runtime_path;
/* Session already exists, client is probably
- * something like su which changes uid but is still
- * the same session */
+ * something like su which changes uid. We want to join
+ * it to the same session. One exception to this is the runtime
+ * path.
+ * See https://bugzilla.redhat.com/show_bug.cgi?id=753882
+ * for some discussion.
+ */
+target_user = hashmap_get(m-users, ULONG_TO_PTR((unsigned long) uid));
+if (target_user)
+runtime_path = target_user-runtime_path;
+else
+runtime_path = ; /* PAM module should treat this as unset */
fifo_fd = session_create_fifo(session);
if (fifo_fd 0)
@@ -570,7 +581,7 @@ static int method_create_session(sd_bus *bus, sd_bus_message *message, void *use
bus, message, soshsub,
session-id,
path,
-
[systemd-devel] Restart instantiated services after suspend
Hi folks,
I am trying to disable HDDs power management in a systemd way (aka no
shell scripts :)
/etc/udev/rules.d/99-hdparm.rules
SUBSYSTEM==block, KERNEL==sd*, ATTR{removable}==0,
TAG+=systemd, ENV{SYSTEMD_WANTS}+=hdparm@%k.service
/etc/systemd/system/hdparm@.service
[Unit]
Description=Set disk /dev/%I parameters
ConditionFileIsExecutable=/usr/sbin/hdparm
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/usr/sbin/hdparm -B254 /dev/%I
It works fine, but after the machine wakes up from suspend, I need
that all hdparm@.service be run again. Is there a way to accomplish
this?
Perhaps something in /usr/lib/systemd/system-sleep could help, but
that would be hacky as the man page says...
Fedora 20, systemd 208.
--
Marcos
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Restart instantiated services after suspend
On Tue, Nov 19, 2013 at 5:18 PM, Marcos Felipe Rasia de Mello
marcos...@gmail.com wrote:
Hi folks,
I am trying to disable HDDs power management in a systemd way (aka no
shell scripts :)
/etc/udev/rules.d/99-hdparm.rules
SUBSYSTEM==block, KERNEL==sd*, ATTR{removable}==0,
TAG+=systemd, ENV{SYSTEMD_WANTS}+=hdparm@%k.service
/etc/systemd/system/hdparm@.service
[Unit]
Description=Set disk /dev/%I parameters
ConditionFileIsExecutable=/usr/sbin/hdparm
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/usr/sbin/hdparm -B254 /dev/%I
It works fine, but after the machine wakes up from suspend, I need
that all hdparm@.service be run again. Is there a way to accomplish
this?
Shouldn't the kernel preserve all such settings during suspend/resume?
-t
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Restart instantiated services after suspend
On Wed, Nov 20, 2013 at 2:18 AM, Marcos Felipe Rasia de Mello marcos...@gmail.com wrote: ConditionFileIsExecutable=/usr/sbin/hdparm It seems sketchy to me to put the executable in ExecStart into ConditionFileIsExecutable. Is it supposed to fail silently when hdparm is missing? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [RFC][PATCH] conf-parser: allow instanced sections
This will treat [Section:bar], [Section:foo], and [Section:baz],
as [Section], but pass on the full section name to the options parser
so it can treat them separately.
---
This is needed so we can add [Address:xxx] and [Route:xxx] sections
ot .network files.
src/shared/conf-parser.c | 28 +---
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c
index efd2147..4f9cb98 100644
--- a/src/shared/conf-parser.c
+++ b/src/shared/conf-parser.c
@@ -156,6 +156,7 @@ static int next_assignment(const char *unit,
ConfigItemLookup lookup,
void *table,
const char *section,
+ const char *section_instance,
const char *lvalue,
const char *rvalue,
bool relaxed,
@@ -178,8 +179,8 @@ static int next_assignment(const char *unit,
if (r 0) {
if (func)
-return func(unit, filename, line, section, lvalue,
ltype,
-rvalue, data, userdata);
+return func(unit, filename, line, section_instance,
lvalue,
+ltype, rvalue, data, userdata);
return 0;
}
@@ -202,6 +203,7 @@ static int parse_line(const char* unit,
bool relaxed,
bool allow_include,
char **section,
+ char **section_instance,
char *l,
void *userdata) {
@@ -238,7 +240,7 @@ static int parse_line(const char* unit,
if (*l == '[') {
size_t k;
-char *n;
+char *n, *m;
k = strlen(l);
assert(k 0);
@@ -253,17 +255,27 @@ static int parse_line(const char* unit,
if (!n)
return -ENOMEM;
-if (sections !nulstr_contains(sections, n)) {
+e = strchr(n, ':');
+if (e)
+m = strndup(n, e - n);
+else
+m = strdup(n);
+
+if (sections !nulstr_contains(sections, m)) {
if (!relaxed)
log_syntax(unit, LOG_WARNING, filename, line,
EINVAL,
- Unknown section '%s'. Ignoring.,
n);
+ Unknown section '%s'. Ignoring.,
m);
free(n);
+free(m);
*section = NULL;
+*section_instance = NULL;
} else {
free(*section);
-*section = n;
+free(*section_instance);
+*section = m;
+*section_instance = n;
}
return 0;
@@ -293,6 +305,7 @@ static int parse_line(const char* unit,
lookup,
table,
*section,
+ *section_instance,
strstrip(l),
strstrip(e),
relaxed,
@@ -310,7 +323,7 @@ int config_parse(const char *unit,
bool allow_include,
void *userdata) {
-_cleanup_free_ char *section = NULL, *continuation = NULL;
+_cleanup_free_ char *section = NULL, *section_instance = NULL,
*continuation = NULL;
_cleanup_fclose_ FILE *ours = NULL;
unsigned line = 0;
int r;
@@ -381,6 +394,7 @@ int config_parse(const char *unit,
relaxed,
allow_include,
section,
+ section_instance,
p,
userdata);
free(c);
--
1.8.4.2
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Restart instantiated services after suspend
El 19/11/13 13:18, Marcos Felipe Rasia de Mello escribió: It works fine, but after the machine wakes up from suspend, I need that all hdparm@.service be run again. Is there a way to accomplish You are looking at the wrong place.. if you disable the HDD power managment, then suspend but after wake up the setting is lost, you are dealing with a kernel bug. -- Any real systematist (or scientist in general) has to be ready to heave all that he or she has believed in, consider it crap, and move on, in the face of new evidence. That is how we differ from clerics. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Restart instantiated services after suspend
2013/11/19 David Timothy Strauss da...@davidstrauss.net: On Wed, Nov 20, 2013 at 2:18 AM, Marcos Felipe Rasia de Mello marcos...@gmail.com wrote: ConditionFileIsExecutable=/usr/sbin/hdparm It seems sketchy to me to put the executable in ExecStart into ConditionFileIsExecutable. Is it supposed to fail silently when hdparm is missing? If hdparm is not preset, unit will enter a failed state. ExecStart command prefixed with - will prevent this, but still I prefer the service to not run at all in this case. In my first attempts, I forgot to install hdparm. ;) -- Marcos ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Restart instantiated services after suspend
2013/11/19 Tom Gundersen t...@jklm.no:
On Tue, Nov 19, 2013 at 5:18 PM, Marcos Felipe Rasia de Mello
marcos...@gmail.com wrote:
Hi folks,
I am trying to disable HDDs power management in a systemd way (aka no
shell scripts :)
/etc/udev/rules.d/99-hdparm.rules
SUBSYSTEM==block, KERNEL==sd*, ATTR{removable}==0,
TAG+=systemd, ENV{SYSTEMD_WANTS}+=hdparm@%k.service
/etc/systemd/system/hdparm@.service
[Unit]
Description=Set disk /dev/%I parameters
ConditionFileIsExecutable=/usr/sbin/hdparm
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/usr/sbin/hdparm -B254 /dev/%I
It works fine, but after the machine wakes up from suspend, I need
that all hdparm@.service be run again. Is there a way to accomplish
this?
Shouldn't the kernel preserve all such settings during suspend/resume?
I do not know. AFAIK, kernel only touches SATA link power settings,
not drive internal ones.
--
Marcos
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Restart instantiated services after suspend
On Wed, Nov 20, 2013 at 3:00 AM, Marcos Felipe Rasia de Mello marcos...@gmail.com wrote: In my first attempts, I forgot to install hdparm. ;) Shouldn't you only drop the rules and service with installation of hdparm, then? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
Hello, Colin Walters [2013-11-19 10:42 -0500]: Both of our patch series currently are basically going to have the effect that with pkexec, XDG_RUNTIME_DIR is unset. But this is undesirable because it forces the rest of userspace to go back to the old dark ages when XDG_RUNTIME_DIR didn't exist and there was no reliable mechanism for two processes of equal uid but different sessions find each other and communicate. (Think ssh login + gdm login). Full ack. My patch though starts to pave the way for having XDG_RUNTIME_DIR consistently point to that of the user's uid - I am increasingly thinking the option #3 I mention in the patch where we refcount the runtime dir in addition to users and sessions would work. For the record, I much prefer something like this to my original patch which simply unsets it. I just shied away from that as Lennart repeatedly said on the RHBZ bug that he doesn't want su behave that way. I disagree, but his word counts more than mine in this situation, so I at least want to stop sessions using the wrong runtime dir. If logind would actually give you the session data for the uid you call it for, instead of only looking at the seat/logind session data, that would indeed be more useful/correct in my opinion. Doing ~user$ su - otheruser or ssh otheruser@localhost should effectively behave the same, but right now logind gives you the session info for ~user in the first, and for ~otheruser in the second case. Thanks! Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Restart instantiated services after suspend
2013/11/19 Cristian Rodríguez crrodrig...@opensuse.org: El 19/11/13 13:18, Marcos Felipe Rasia de Mello escribió: It works fine, but after the machine wakes up from suspend, I need that all hdparm@.service be run again. Is there a way to accomplish You are looking at the wrong place.. if you disable the HDD power managment, then suspend but after wake up the setting is lost, you are dealing with a kernel bug. Maybe. On all machines I have HDD APM setting does not survive a drive power cycle. Appears to be the default kernel (buggy?) behaviour for a long time. -- Marcos ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC][PATCH] conf-parser: allow instanced sections
'Twas brillig, and Tom Gundersen at 19/11/13 16:57 did gyre and gimble:
This will treat [Section:bar], [Section:foo], and [Section:baz],
as [Section], but pass on the full section name to the options parser
so it can treat them separately.
What is the semantics here? (I should probably know as you've likely
posted it already!)
e.g. I've seen the following semantics before for ini-style parsing:
[database]
database.adapter = pdo_mysql
database.params.host = db.dev
database.params.username = ninja
database.params.password = dressedinblack
[mydb : database]
database.params.dbname = mydatabase
This basically means the mydb section inherits from the database
section and only changes what it needs.
It seems you're using using this slightly differently in that you'll have a:
[General]
Address=xyz
and then a
[Address:xyz]
section? i.e. the instances section actually inherits from an earlier value?
If so then I doubt the semantics here can be generalised to this other
example in which case my comment is rather moot!
So take this comment with a pinch of salt :D
@@ -253,17 +255,27 @@ static int parse_line(const char* unit,
if (!n)
return -ENOMEM;
-if (sections !nulstr_contains(sections, n)) {
+e = strchr(n, ':');
+if (e)
+m = strndup(n, e - n);
+else
+m = strdup(n);
+
+if (sections !nulstr_contains(sections, m)) {
Do you want to do some whitespace trimming here perhaps? e.g. if n is
Foo :bar or Foo : bar or Foo: bar etc.
being silently tolerant of this is probably most sensible I guess.
Col
--
Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/
Day Job:
Tribalogic Limited http://www.tribalogic.net/
Open Source:
Mageia Contributor http://www.mageia.org/
PulseAudio Hacker http://www.pulseaudio.org/
Trac Hacker http://trac.edgewall.org/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
On Tue, 2013-11-19 at 18:15 +0100, Martin Pitt wrote:
For the record, I much prefer something like this to my original patch
which simply unsets it. I just shied away from that as Lennart
repeatedly said on the RHBZ bug that he doesn't want su behave that
way.
This is a complex discussion because there are many different cases. As
I mentioned before, I care primarily about the login as non-root, run
pkexec to get a root shell case. And only a shell - no $DISPLAY, no
pulseaudio.
What Lennart was more referring to in the RHBZ was the people asking for
more than that out of su, like $DISPLAY proxying. But from the
PolicyKit side, we're trying hard to move people away from that.
Then there's the case of going root - non-root. The below patch
doesn't affect that.
And these can all be combined, e.g. you can go non-root - root -
different non-root. That's actually not that crazy; login as your user,
become root, then sudo -u mysql to admin the database.
I disagree, but his word counts more than mine in this situation,
so I at least want to stop sessions using the wrong runtime dir.
Right.
If logind would actually give you the session data for the uid you
call it for, instead of only looking at the seat/logind session data,
that would indeed be more useful/correct in my opinion. Doing
~user$ su - otheruser or ssh otheruser@localhost should effectively behave
the same, but right now logind gives you the session info for ~user in
the first, and for ~otheruser in the second case.
The tricky thing is it's not just about the data - it's the *lifecycle*
of that data. If we hand out an XDG_RUNTIME_DIR, we need to ensure it
isn't garbage collected by logind.
This new patch is a laser-targeted fix for what I consider the #1 most
important case of non-root gdm - pkexec/sudo. We solve the lifecycle
issue for /run/user/0 in a simple way - it just always exists now via
tmpfiles.d. Something else in systemd is creating that currently
anyways.
Solving some of the other cases would be tricky; we'd have to make them
be at least partial sessions to ensure that the XDG_RUNTIME_DIR for
their uid stays around.
We could do this on the systemd side with explicit child sessions or
so, but that's a high degree of complexity that I'm not sure is
warranted right now. Perhaps the real fix would just be for a special
kernel automount type setup that where a process of uid N
reads /run/userdir it is a symlink to /run/user/N which is automatically
mounted as a tmpfs.
Anyways, new tested patch attached. Lennart, any objections?
From 98da613a2dfcf4bb6bee709f29aba142cd34f118 Mon Sep 17 00:00:00 2001
From: Colin Walters walt...@verbum.org
Date: Tue, 19 Nov 2013 10:21:16 -0500
Subject: [PATCH] login: Use correct XDG_RUNTIME_DIR for uid 0 with pkexec and similar
This fixes a special case of the more general issue of uid-changing
programs such as pkexec/su/sudo/runuser. For the case where a user
logs into a desktop and then runs pkexec bash or sudo su - or
equivalent to become root, we should ensure that XDG_RUNTIME_DIR
contains the correct information.
Previous to this patch, we'd retain the incorrect XDG_RUNTIME_DIR from
the current session. There bad things can happen such as a uid 0
process writing files into uid 1000's runtime dir, corrupting it.
However, for changing to a *non-root* uid, we now leave
XDG_RUNTIME_DIR unset, because we cannot guarantee it exists for the
new uid. This occurs with sudo mysql or variants as recommended by
some MySQL documentation. But those tools long predate the runtime
dir, and they likely don't even use it. If they do, the'll now just
fall back to the pre-runtime dir code.
See https://bugzilla.redhat.com/show_bug.cgi?id=753882
See http://lists.freedesktop.org/archives/systemd-devel/2013-November/014370.html
---
src/login/logind-dbus.c | 26 +++---
src/login/pam-module.c | 10 ++
tmpfiles.d/systemd.conf |1 +
3 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 0f25e23..7cb6d36 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -553,10 +553,30 @@ static int method_create_session(sd_bus *bus, sd_bus_message *message, void *use
if (session) {
_cleanup_free_ char *path = NULL;
_cleanup_close_ int fifo_fd = -1;
+const char *runtime_path;
/* Session already exists, client is probably
- * something like su which changes uid but is still
- * the same session */
+ * something like su or pkexec which changes uid.
+ * Here we avoid creating a full new session (since we
+ * don't have child sessions), so we're just reusing
+ * the existing session data almost unchanged.
+ *
+ * One exception to this is the runtime path. See
+ *
[systemd-devel] systemd 208:trouble with inactive user sessions at non-seat0 seats
Hi there! I'm testing both Fedora 20 Beta and openSUSE 13.1 in my multiseat system (with GNOME 3.10, GDM 3.10.0.1 and systemd 208). I'm currently observing a strange behaviour which didn't occur in previous distro release (with GNOME 3.8.4 and systemd 204). When I boot my system, gdm greeter session at both seat0 and non-seat0 seats are active, as expected. However, when a user logs in at a non-seat0 seat, the user session gets inactive. Nevertheless, I can activate it manually with command loginctl activate user-session-id, but if that user logs out, the new greeter session now becomes inactive. I suspect this strange behaviour is related to that generic multi-session support introduced in systemd 208. I can see that, when a user logs in, the greeter and user sessions coexist for a while, one at closing state and the other at online state, depending on whether the user logs in or out. What could it be? Lack of generic multi-session support in GDM? CANTATE DOMINO CANTICUM NOVUM QUIA MIRABILIA FECIT Laércio ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Restart instantiated services after suspend
On Tue, Nov 19, 2013 at 03:47:39PM -0200, Marcos Felipe Rasia de Mello wrote: 2013/11/19 Cristian Rodríguez crrodrig...@opensuse.org: El 19/11/13 13:18, Marcos Felipe Rasia de Mello escribió: It works fine, but after the machine wakes up from suspend, I need that all hdparm@.service be run again. Is there a way to accomplish You are looking at the wrong place.. if you disable the HDD power managment, then suspend but after wake up the setting is lost, you are dealing with a kernel bug. Maybe. On all machines I have HDD APM setting does not survive a drive power cycle. Appears to be the default kernel (buggy?) behaviour for a long time. The kernel does not know internal drive settings, as it was never told about them (you did a pass-through command to set them, you didn't tell the kernel to set them...) sorry, greg k-h ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Systemd / PID 1 leak
Systemd system instance (not the user one) is leaking for me. WinterMute # while :; do systemctl daemon-reload; ps v 1; done PID TTY STAT TIME MAJFL TRS DRS RSS %MEM COMMAND 1 ?Rs 0:01 48 1000 30535 4244 0.0 /usr/lib/systemd/systemd PID TTY STAT TIME MAJFL TRS DRS RSS %MEM COMMAND 1 ?Rs 0:01 48 1000 30795 4488 0.0 /usr/lib/systemd/systemd PID TTY STAT TIME MAJFL TRS DRS RSS %MEM COMMAND 1 ?Rs 0:01 48 1000 31023 4708 0.0 /usr/lib/systemd/systemd PID TTY STAT TIME MAJFL TRS DRS RSS %MEM COMMAND 1 ?Rs 0:01 48 1000 31023 4776 0.0 /usr/lib/systemd/systemd PID TTY STAT TIME MAJFL TRS DRS RSS %MEM COMMAND 1 ?Rs 0:01 48 1000 31247 4936 0.0 /usr/lib/systemd/systemd PID TTY STAT TIME MAJFL TRS DRS RSS %MEM COMMAND 1 ?Rs 0:05 48 1000 44827 18096 0.2 /usr/lib/systemd/systemd PID TTY STAT TIME MAJFL TRS DRS RSS %MEM COMMAND 1 ?Rs 0:05 48 1000 44895 18128 0.2 /usr/lib/systemd/systemd PID TTY STAT TIME MAJFL TRS DRS RSS %MEM COMMAND 1 ?Rs 0:05 48 1000 44895 18168 0.2 /usr/lib/systemd/systemd PID TTY STAT TIME MAJFL TRS DRS RSS %MEM COMMAND 1 ?Rs 0:05 48 1000 44999 18248 0.2 /usr/lib/systemd/systemd ^C WinterMute # systemctl daemon-reexec WinterMute # ps v 1 PID TTY STAT TIME MAJFL TRS DRS RSS %MEM COMMAND 1 ?Ss 0:05 48 1000 22027 3740 0.0 /usr/lib/systemd/systemd --system --deserialize 21 Any suggestions how to find the problem? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Systemd / PID 1 leak
On Wed, Nov 20, 2013 at 5:33 AM, Oleksii Shevchuk alx...@gmail.com wrote: Any suggestions how to find the problem? Valgrind ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Systemd / PID 1 leak
Valgrind Any methodology for pid 1 valgrinding? How to start/gather information for example? Do you have any setup for that? Maybe I can clone it ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] bus/rtnl: silence warnings with clang
--- src/libsystemd-bus/bus-internal.h | 2 +- src/libsystemd-rtnl/rtnl-internal.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsystemd-bus/bus-internal.h b/src/libsystemd-bus/bus-internal.h index 4f9d941..faed183 100644 --- a/src/libsystemd-bus/bus-internal.h +++ b/src/libsystemd-bus/bus-internal.h @@ -307,4 +307,4 @@ char *bus_address_escape(const char *v); * bus from the callback doesn't destroy the object we are working * on */ #define BUS_DONT_DESTROY(bus) \ -_cleanup_bus_unref_ sd_bus *_dont_destroy_##bus = sd_bus_ref(bus) +_cleanup_bus_unref_ _unused_ sd_bus *_dont_destroy_##bus = sd_bus_ref(bus) diff --git a/src/libsystemd-rtnl/rtnl-internal.h b/src/libsystemd-rtnl/rtnl-internal.h index c8a97da..a229ae7 100644 --- a/src/libsystemd-rtnl/rtnl-internal.h +++ b/src/libsystemd-rtnl/rtnl-internal.h @@ -80,4 +80,4 @@ int socket_read_message(sd_rtnl *nl, sd_rtnl_message **ret); /* Make sure callbacks don't destroy the rtnl connection */ #define RTNL_DONT_DESTROY(rtnl) \ -_cleanup_sd_rtnl_unref_ sd_rtnl *_dont_destroy_##rtnl = sd_rtnl_ref(rtnl) +_cleanup_sd_rtnl_unref_ _unused_ sd_rtnl *_dont_destroy_##rtnl = sd_rtnl_ref(rtnl) -- 1.8.4.2 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC][PATCH] conf-parser: allow instanced sections
On Tue, Nov 19, 2013 at 7:07 PM, Colin Guthrie gm...@colin.guthr.ie wrote:
'Twas brillig, and Tom Gundersen at 19/11/13 16:57 did gyre and gimble:
This will treat [Section:bar], [Section:foo], and [Section:baz],
as [Section], but pass on the full section name to the options parser
so it can treat them separately.
What is the semantics here? (I should probably know as you've likely
posted it already!)
e.g. I've seen the following semantics before for ini-style parsing:
[database]
database.adapter = pdo_mysql
database.params.host = db.dev
database.params.username = ninja
database.params.password = dressedinblack
[mydb : database]
database.params.dbname = mydatabase
This basically means the mydb section inherits from the database
section and only changes what it needs.
It seems you're using using this slightly differently in that you'll have a:
[General]
Address=xyz
and then a
[Address:xyz]
section? i.e. the instances section actually inherits from an earlier value?
What I have in mind (though it is not dictated by this patch) is
something different (first proposed by Lennart in an earlier thread):
[Network]
Address=192.168.0.1/24
Address=192.168.0.2/24
Gateway=192.168.1.1
[Address:oneaddress]
Address=192.168.0.3/24
Label=three
Peer=192.168.1.1
[Address:anotheraddress]
Address=192.168.0.4/24
Label=four
In this case we'll configure four addresses. The two first ones could
also have been expressed as:
[Address:foo]
Address=192.168.0.1/24
[Address:bar]
Address=192.168.0.2/24,
but we allow putting them directly in the [Network] section rather
than in a named [Address] section as a shorthand.
Notice that if we simply did
[Address]
Address=192.168.0.3/24
Label=three
Peer=192.168.1.1
[Address]
Address=192.168.0.4/24
Label=four,
that wouldn't work as it is (at least currently) equivalent to
[Address]
Address=192.168.0.3/24
Label=three
Peer=192.168.1.1
Address=192.168.0.4/24
Label=four,
which is why we need to give the secitons unique names.
If so then I doubt the semantics here can be generalised to this other
example in which case my comment is rather moot!
So take this comment with a pinch of salt :D
@@ -253,17 +255,27 @@ static int parse_line(const char* unit,
if (!n)
return -ENOMEM;
-if (sections !nulstr_contains(sections, n)) {
+e = strchr(n, ':');
+if (e)
+m = strndup(n, e - n);
+else
+m = strdup(n);
+
+if (sections !nulstr_contains(sections, m)) {
Do you want to do some whitespace trimming here perhaps? e.g. if n is
Foo :bar or Foo : bar or Foo: bar etc.
being silently tolerant of this is probably most sensible I guess.
Yes, that's probably a good idea. Thanks.
-t
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Systemd / PID 1 leak
'Twas brillig, and Oleksii Shevchuk at 19/11/13 21:10 did gyre and gimble: Valgrind Any methodology for pid 1 valgrinding? How to start/gather information for example? Do you have any setup for that? Maybe I can clone it Probably best to use systemd-nspawn and a container tree to boot a virtual PID 1. I don't know the correct commands to run but I think this is typically the best approach. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
'Twas brillig, and Colin Walters at 19/11/13 18:13 did gyre and gimble: +d /run/user/0 0755 root root 10d This should probably be 0700 like the runtime dirs usually are I think. Also won't this folder be naturally reaped in user_finalize() in login/logind-user.c: /* Kill XDG_RUNTIME_DIR */ k = user_remove_runtime_path(u); e.g. if you really do login as root, then logout again, it'll kill this folder, and thus the tmpfiles stuff only helps up until root logs in directly then logs out again after which point the folder is gone and we're snookered. So you likely need to suppress the tidyup for root too unless I've misunderstood something. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] bus/rtnl: silence warnings with clang
Hi Thomas, I'm not able to apply this patch, could you please resend using git-send-email? Cheers, Tom On Tue, Nov 19, 2013 at 10:16 PM, Thomas H.P. Andersen pho...@gmail.com wrote: --- src/libsystemd-bus/bus-internal.h | 2 +- src/libsystemd-rtnl/rtnl-internal.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsystemd-bus/bus-internal.h b/src/libsystemd-bus/bus-internal.h index 4f9d941..faed183 100644 --- a/src/libsystemd-bus/bus-internal.h +++ b/src/libsystemd-bus/bus-internal.h @@ -307,4 +307,4 @@ char *bus_address_escape(const char *v); * bus from the callback doesn't destroy the object we are working * on */ #define BUS_DONT_DESTROY(bus) \ -_cleanup_bus_unref_ sd_bus *_dont_destroy_##bus = sd_bus_ref(bus) +_cleanup_bus_unref_ _unused_ sd_bus *_dont_destroy_##bus = sd_bus_ref(bus) diff --git a/src/libsystemd-rtnl/rtnl-internal.h b/src/libsystemd-rtnl/rtnl-internal.h index c8a97da..a229ae7 100644 --- a/src/libsystemd-rtnl/rtnl-internal.h +++ b/src/libsystemd-rtnl/rtnl-internal.h @@ -80,4 +80,4 @@ int socket_read_message(sd_rtnl *nl, sd_rtnl_message **ret); /* Make sure callbacks don't destroy the rtnl connection */ #define RTNL_DONT_DESTROY(rtnl) \ -_cleanup_sd_rtnl_unref_ sd_rtnl *_dont_destroy_##rtnl = sd_rtnl_ref(rtnl) +_cleanup_sd_rtnl_unref_ _unused_ sd_rtnl *_dont_destroy_##rtnl = sd_rtnl_ref(rtnl) -- 1.8.4.2 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units
Út 19. listopad 2013, 15:16:47 CET, Michal Sekletar napsal:
On Tue, Nov 19, 2013 at 08:54:41AM -0500, Daniel J Walsh wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 11/18/2013 05:45 PM, Michal Sekletar wrote:
On Mon, Nov 18, 2013 at 04:19:20PM -0500, Daniel J Walsh wrote: On
11/16/2013 08:10 AM, Lennart Poettering wrote:
On Thu, 14.11.13 15:43, Daniel J Walsh (dwa...@redhat.com) wrote:
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1
On 11/14/2013 12:50 PM, Harald Hoyer wrote:
On 11/05/2013 11:12 PM, Daniel J Walsh wrote:
On 11/05/2013 12:22 PM, Lennart Poettering wrote:
Ok lets add a check that checks for start on a service labeled
with the remote process label, then we can add rules like
allow systemd_logind_t self:service start
Or we can make it simpler and have the local end check against
the init_t process.
allow systemd_logind_t init_t:service start;
Which is probably a better solution, if we have no way of
differentiating the services.
Machineid usually runs as init_t now.
systemd-run runs as the label of the process that executes it,
Usually unconfined_t, and sysadm_t.
has any solution been found for this?
seems like one is needed for
https://bugzilla.redhat.com/show_bug.cgi?id=1008864
I guess the question I have is do you expect a patch from me? Or
are you guys working on it? I would go with the checking based on
process label.
I am hoping for a patch for this!
Thanks,
Lennart
This patch adds a new call for SELINUX_SNAPSHOT_ACCESS_CHECK, because I
believe this error will happen when a snapshot is created. Also now pass
in system when doing a system check, if it is doing a service
check and
does not pass in a unit file we will default the target to the label
that
systemd is running with.
Hi,
Maybe I am missing something but isn't this about transient units in
general, i.e. what about StartTransient()? What is going to change in
this case after applying this patch? tclass will be system since in
SELINUX_ACCESS_CHECK you now pass system as path and you will set
tclass in else branch to system which is afaik same as before.
In the current code, passing a unit_file of NULL (StartTransients has
a NULL
UnitFile) will indicate that it should do a system check. My patch is
intended to change this so a NULL UnitFile will indicate that systemd
should
check the access between the calling process label and the current
process
label for a service access. Where as the SELINUX_ACCESS_CHECK will
now pass
a system flag to the function to make it do a system check.
Hi Dan,
Agreed, I get the idea, but this means that
SELINUX_SNAPSHOT_ACCESS_CHECK should
be performed in StartTransient branch in dbus-manager.c too and macro
should be
probably renamed to something like SELINUX_RUNTIME_UNIT_ACCESS_CHECK.
Hope that makes sense.
Michal
Hi,
I tried to improve Dan's patch, so I added an empty call if selinux is
not supported, renamed the function so it doesn't imply it's only for
snapshots and used it in StartTransient and RemoveSnapshot as well. I
wanted to test it, but I am not sure if the policy is already updated.
Vaclav
On the side note, you forgot to define SELINUX_SNAPSHOT_ACCESS_CHECK as
do {} while (false) in case if we don't HAVE_SELINUX.
It might be the case that I completely misunderstood what's this about,
in such case ignore this email.
Michal
Thanks added SELINUX_SNAPSHOT_ACCESS_CHECK as do {} while (false) in
case if
we don't HAVE_SELINUX.
Updated patch.
snip
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlKLbaEACgkQrlYvE4MpobPMMACeNeyrC3OBhx99DZ+JzOtV1ykZ
PvMAoJfiYBoJRBFgh2+FyOV+tNTuojNU
=9I5G
-END PGP SIGNATURE-
From b372b48016f5c015b34db9f53b7a54a64a5a84e8 Mon Sep 17 00:00:00 2001
From: Dan Walsh dwa...@redhat.com
Date: Mon, 18 Nov 2013 15:52:37 -0500
Subject: [PATCH] Fix SELinux check for snapshot and transitent unit
creation.
SELinux does not have a path to check for a snapshot or transient
unit files service creation. Currently systemd does a bogus check.
If we don't have a unit file for a snapshot or transient unit we i
should check if the remote process label, has the required access
for a service with the SELinux label that systemd is running with.
Add SELINUX_SNAPSHOT_ACCESS_CHECK for non SELinux environments
---
src/core/dbus-manager.c | 2 +-
src/core/selinux-access.c | 9 +
src/core/selinux-access.h | 13 +
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 747bcfc..a60a568 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1112,7 +1112,7 @@ static DBusHandlerResult
bus_manager_message_handler(DBusConnection *connection,
dbus_bool_t cleanup;
Snapshot *s;
- SELINUX_ACCESS_CHECK(connection, message, start);
+
Re: [systemd-devel] [PATCH] selinux: fix selinux check for transient units
And I obviously attached wrong file...this is the right one, sorry
St 20. listopad 2013, 05:47:36 CET, Václav Pavlín napsal:
Út 19. listopad 2013, 15:16:47 CET, Michal Sekletar napsal:
On Tue, Nov 19, 2013 at 08:54:41AM -0500, Daniel J Walsh wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 11/18/2013 05:45 PM, Michal Sekletar wrote:
On Mon, Nov 18, 2013 at 04:19:20PM -0500, Daniel J Walsh wrote: On
11/16/2013 08:10 AM, Lennart Poettering wrote:
On Thu, 14.11.13 15:43, Daniel J Walsh (dwa...@redhat.com) wrote:
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1
On 11/14/2013 12:50 PM, Harald Hoyer wrote:
On 11/05/2013 11:12 PM, Daniel J Walsh wrote:
On 11/05/2013 12:22 PM, Lennart Poettering wrote:
Ok lets add a check that checks for start on a service labeled
with the remote process label, then we can add rules like
allow systemd_logind_t self:service start
Or we can make it simpler and have the local end check against
the init_t process.
allow systemd_logind_t init_t:service start;
Which is probably a better solution, if we have no way of
differentiating the services.
Machineid usually runs as init_t now.
systemd-run runs as the label of the process that executes it,
Usually unconfined_t, and sysadm_t.
has any solution been found for this?
seems like one is needed for
https://bugzilla.redhat.com/show_bug.cgi?id=1008864
I guess the question I have is do you expect a patch from me? Or
are you guys working on it? I would go with the checking based on
process label.
I am hoping for a patch for this!
Thanks,
Lennart
This patch adds a new call for SELINUX_SNAPSHOT_ACCESS_CHECK,
because I
believe this error will happen when a snapshot is created. Also now
pass
in system when doing a system check, if it is doing a service
check and
does not pass in a unit file we will default the target to the
label that
systemd is running with.
Hi,
Maybe I am missing something but isn't this about transient units in
general, i.e. what about StartTransient()? What is going to change in
this case after applying this patch? tclass will be system since in
SELINUX_ACCESS_CHECK you now pass system as path and you will set
tclass in else branch to system which is afaik same as before.
In the current code, passing a unit_file of NULL (StartTransients
has a NULL
UnitFile) will indicate that it should do a system check. My patch is
intended to change this so a NULL UnitFile will indicate that
systemd should
check the access between the calling process label and the current
process
label for a service access. Where as the SELINUX_ACCESS_CHECK will
now pass
a system flag to the function to make it do a system check.
Hi Dan,
Agreed, I get the idea, but this means that
SELINUX_SNAPSHOT_ACCESS_CHECK should
be performed in StartTransient branch in dbus-manager.c too and macro
should be
probably renamed to something like SELINUX_RUNTIME_UNIT_ACCESS_CHECK.
Hope that makes sense.
Michal
Hi,
I tried to improve Dan's patch, so I added an empty call if selinux is
not supported, renamed the function so it doesn't imply it's only for
snapshots and used it in StartTransient and RemoveSnapshot as well. I
wanted to test it, but I am not sure if the policy is already updated.
Vaclav
On the side note, you forgot to define
SELINUX_SNAPSHOT_ACCESS_CHECK as
do {} while (false) in case if we don't HAVE_SELINUX.
It might be the case that I completely misunderstood what's this
about,
in such case ignore this email.
Michal
Thanks added SELINUX_SNAPSHOT_ACCESS_CHECK as do {} while (false) in
case if
we don't HAVE_SELINUX.
Updated patch.
snip
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlKLbaEACgkQrlYvE4MpobPMMACeNeyrC3OBhx99DZ+JzOtV1ykZ
PvMAoJfiYBoJRBFgh2+FyOV+tNTuojNU
=9I5G
-END PGP SIGNATURE-
From b372b48016f5c015b34db9f53b7a54a64a5a84e8 Mon Sep 17 00:00:00 2001
From: Dan Walsh dwa...@redhat.com
Date: Mon, 18 Nov 2013 15:52:37 -0500
Subject: [PATCH] Fix SELinux check for snapshot and transitent unit
creation.
SELinux does not have a path to check for a snapshot or transient
unit files service creation. Currently systemd does a bogus check.
If we don't have a unit file for a snapshot or transient unit we i
should check if the remote process label, has the required access
for a service with the SELinux label that systemd is running with.
Add SELINUX_SNAPSHOT_ACCESS_CHECK for non SELinux environments
---
src/core/dbus-manager.c | 2 +-
src/core/selinux-access.c | 9 +
src/core/selinux-access.h | 13 +
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 747bcfc..a60a568 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1112,7 +1112,7 @@ static DBusHandlerResult
Re: [systemd-devel] [PATCH] bus/rtnl: silence warnings with clang
Hi Tom, Sorry. Git send-email is just giving me error messages right now. I have attached the patch instead. On Wed, Nov 20, 2013 at 1:12 AM, Tom Gundersen t...@jklm.no wrote: Hi Thomas, I'm not able to apply this patch, could you please resend using git-send-email? Cheers, Tom On Tue, Nov 19, 2013 at 10:16 PM, Thomas H.P. Andersen pho...@gmail.com wrote: --- src/libsystemd-bus/bus-internal.h | 2 +- src/libsystemd-rtnl/rtnl-internal.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsystemd-bus/bus-internal.h b/src/libsystemd-bus/bus-internal.h index 4f9d941..faed183 100644 --- a/src/libsystemd-bus/bus-internal.h +++ b/src/libsystemd-bus/bus-internal.h @@ -307,4 +307,4 @@ char *bus_address_escape(const char *v); * bus from the callback doesn't destroy the object we are working * on */ #define BUS_DONT_DESTROY(bus) \ -_cleanup_bus_unref_ sd_bus *_dont_destroy_##bus = sd_bus_ref(bus) +_cleanup_bus_unref_ _unused_ sd_bus *_dont_destroy_##bus = sd_bus_ref(bus) diff --git a/src/libsystemd-rtnl/rtnl-internal.h b/src/libsystemd-rtnl/rtnl-internal.h index c8a97da..a229ae7 100644 --- a/src/libsystemd-rtnl/rtnl-internal.h +++ b/src/libsystemd-rtnl/rtnl-internal.h @@ -80,4 +80,4 @@ int socket_read_message(sd_rtnl *nl, sd_rtnl_message **ret); /* Make sure callbacks don't destroy the rtnl connection */ #define RTNL_DONT_DESTROY(rtnl) \ -_cleanup_sd_rtnl_unref_ sd_rtnl *_dont_destroy_##rtnl = sd_rtnl_ref(rtnl) +_cleanup_sd_rtnl_unref_ _unused_ sd_rtnl *_dont_destroy_##rtnl = sd_rtnl_ref(rtnl) -- 1.8.4.2 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel From b806e849ba7e3df79d9d3212fbe2b11394d574ef Mon Sep 17 00:00:00 2001 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com Date: Tue, 19 Nov 2013 21:12:38 +0100 Subject: [PATCH] bus/rtnl: silence clang warnings --- src/libsystemd-bus/bus-internal.h | 2 +- src/libsystemd-rtnl/rtnl-internal.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsystemd-bus/bus-internal.h b/src/libsystemd-bus/bus-internal.h index 4f9d941..faed183 100644 --- a/src/libsystemd-bus/bus-internal.h +++ b/src/libsystemd-bus/bus-internal.h @@ -307,4 +307,4 @@ char *bus_address_escape(const char *v); * bus from the callback doesn't destroy the object we are working * on */ #define BUS_DONT_DESTROY(bus) \ -_cleanup_bus_unref_ sd_bus *_dont_destroy_##bus = sd_bus_ref(bus) +_cleanup_bus_unref_ _unused_ sd_bus *_dont_destroy_##bus = sd_bus_ref(bus) diff --git a/src/libsystemd-rtnl/rtnl-internal.h b/src/libsystemd-rtnl/rtnl-internal.h index c8a97da..a229ae7 100644 --- a/src/libsystemd-rtnl/rtnl-internal.h +++ b/src/libsystemd-rtnl/rtnl-internal.h @@ -80,4 +80,4 @@ int socket_read_message(sd_rtnl *nl, sd_rtnl_message **ret); /* Make sure callbacks don't destroy the rtnl connection */ #define RTNL_DONT_DESTROY(rtnl) \ -_cleanup_sd_rtnl_unref_ sd_rtnl *_dont_destroy_##rtnl = sd_rtnl_ref(rtnl) +_cleanup_sd_rtnl_unref_ _unused_ sd_rtnl *_dont_destroy_##rtnl = sd_rtnl_ref(rtnl) -- 1.8.4.2 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] event: clear pending-state when re-arming timers
If a timer fires and is marked pending, but an application re-arms it
before it is dispatched, we now clear the pending state.
This fixes a bug where an application arms a timer, which fires and is
marked pending. But before it is dispatched, the application loses
interest in it and disables it. Now if the timer is re-armed and
re-enabled later, it will be immediately dispatched as it is still marked
pending.
This behavior is unexpected, so avoid it by clearing pending state when
re-arming timers. Note that applications have no way to clear pending
state themselves, so there's no current workaround.
---
src/libsystemd-bus/sd-event.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/libsystemd-bus/sd-event.c b/src/libsystemd-bus/sd-event.c
index 0996316..d01e82d 100644
--- a/src/libsystemd-bus/sd-event.c
+++ b/src/libsystemd-bus/sd-event.c
@@ -1241,6 +1241,7 @@ _public_ int sd_event_source_set_time(sd_event_source *s,
uint64_t usec) {
return 0;
s-time.next = usec;
+source_set_pending(s, false);
if (s-type == SOURCE_REALTIME) {
prioq_reshuffle(s-event-realtime_earliest, s,
s-time.earliest_index);
--
1.8.4.2
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
