date:20140103

Le vendredi 03 janvier 2014 à 00:58 +, Jóhann B. Guðmundsson a
écrit :
On 12/28/2013 01:30 PM, Lennart Poettering wrote:
On Fri, 27.12.13 23:26,m...@zarb.org (m...@zarb.org) wrote:

From: Michael Schererm...@zarb.org

This permit to let system administrators decide of the domain of a
service.
This can be used with templated units to have each service in a différent
domain ( for example, a per customer database, using MLS or anything ),
or can be used to force a non selinux enabled system (jvm, erlang, etc)
to start in a different domain for each service.
Hmm, so far (as I understood it) the SELinux guys always wanted to make
sure that label configuration stays in the the selinux database and
nowhere else.

Right and if you add this you need to add something for the other
security solutions as well ( apparmor etc ).

I do not see why we need to equally support all MAC frameworks for each
change we do. And while I am familiar enough with apparmor to create a
equivalent setting ( and plan to do ), I have no idea on how to
translate the concept for smack, ima and tomoyo.

In fact, the mere fact that tomoyo is not handled at all already show
that we do treat all security framework as equal.

This introduces yet another place for administrators to look at while
debugging problems so the question to ask here is.

Is adding this ability to unit files the right way to solve what's
trying to be solved here?

As Dan said, yes.

Usually, the type of transition from 1 domain to another is done at the
kernel level based on the label of the file executed. See
https://wiki.gentoo.org/wiki/SELinux/Tutorials/How_does_a_process_get_into_a_certain_context

and http://danwalsh.livejournal.com/23944.html

However, as said, there is some case where the approach of making the
transition based on the executed filename is not sufficient. For
example, the erlang vm, the jvm, etc, because each software will run in
the same domain, in different processes, because that's always the same
jvm being used. See the bug I mentioned before.

Now, if you have a more precise feedback and/or a better proposal, I am
ready to hear of it, but the only alternative I see is to make the JVM,
erlang, etc, to be SELinux aware. That's a much bigger task, and I am
not sure that's worth the code duplication ( not to mention that it make
sysadmin look in several different places ). And the design I was
thinking of ( ie, replicated the current system of doing transition
based on file label ) would requires reimplementing the kernel code in
userspace, in libselinux, and I would rather avoid this for various
reasons ( security, code duplication avoidance ).

Another solution would be to create shell wrapper for every java, erlang
and mono software, and then use process transition on the shell wrapper,
but that doesn't scale that well and do not offer the flexibility of the
current patch to the sysadmin. ( and would likely be Fedora specific as
well ).

--
Michael Scherer

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] Add SELinuxContext configuration item

2014-01-03 Thread Jóhann B. Guðmundsson



On 01/03/2014 10:56 AM, Michael Scherer wrote:

Le vendredi 03 janvier 2014 à 00:58 +, Jóhann B. Guðmundsson a
écrit :

On 12/28/2013 01:30 PM, Lennart Poettering wrote:

On Fri, 27.12.13 23:26,m...@zarb.org  (m...@zarb.org) wrote:


From: Michael Schererm...@zarb.org

This permit to let system administrators decide of the domain of a service.
This can be used with templated units to have each service in a différent
domain ( for example, a per customer database, using MLS or anything ),
or can be used to force a non selinux enabled system (jvm, erlang, etc)
to start in a different domain for each service.

Hmm, so far (as I understood it) the SELinux guys always wanted to make
sure that label configuration stays in the the selinux database and
nowhere else.

Right and if you add this you need to add something for the other
security solutions as well ( apparmor etc ).

I do not see why we need to equally support all MAC frameworks for each
change we do.


Because people will start to expect being able to configure that there.


  And while I am familiar enough with apparmor to create a
equivalent setting ( and plan to do ), I have no idea on how to
translate the concept for smack, ima and tomoyo.

In fact, the mere fact that tomoyo is not handled at all already show
that we do treat all security framework as equal.


How do you suppose we deal with man pages if selinux is not installed 
but still refer to this.


Wont users also need to check if selinux is enabled or not in the unit 
file?


Should systemd warn users if selinux is not installed,enabled and fail or?




This introduces yet another place for administrators to look at while
debugging problems so the question to ask here is.

Is adding this ability to unit files the right way to solve what's
trying to be solved here?

As Dan said, yes.


I would prefer if app writers do not start hard coding SELinux contexts 
into the unit file


I hardly call that solid yes and this is what will start taking place if 
this is introduced into the unit files.




However, as said, there is some case where the approach of making the
transition based on the executed filename is not sufficient. For
example, the erlang vm, the jvm, etc, because each software will run in
the same domain, in different processes, because that's always the same
jvm being used. See the bug I mentioned before.

Now, if you have a more precise feedback and/or a better proposal,


Add this to the daemon startup itself ( the confile or the code ) and or 
use runcon in an exec start pre section to set this up.


In anycase Lennart decides this to me this seems like a workaround being 
put in systemd for a limitation in selinux or the concept there of with 
the added complexity for administrators.


JBG
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [Patch 0/2] logind: make sure that closed sessions will be cleaned

2014-01-03 Thread Djalal Harouni

On logout pam_systemd should ensures the following:
If the last concurrent session of a user ends, the $XDG_RUNTIME_DIR
directory and all its contents are removed, too. from manpage.

Using git HEAD, and a simple systemd-nspawn test will show that the
above is not ensured and the sessions will stay!


A simple systemd-nspawn test:

1) login as user X
2) logout
3) login as user Y
4) loginctl  (will list session of user X)


In this example we are session c4:

-bash-4.2# loginctl list-sessions
   SESSIONUID USER SEAT 
 1   1000 tixxdz   seat0 
c1   1000 tixxdz   seat0
c2  0 root seat0
c3   1000 tixxdz   seat0
c4  0 root seat0

-bash-4.2# loginctl show-session --property=State 1 c1 c2 c3 c4
State=closing

State=closing

State=closing

State=closing

State=active


As shown only session c4 is active, all the others are dead sessions.

To close the dead sessions and clean things up, a dbus
TerminateSession()=session_stop() must be issued...

Please note that I'm running without pam_loginuid.so, due to another
bug related to audit: https://bugzilla.redhat.com/show_bug.cgi?id=966807


Anyway, after some debugging:

It seems that after ReleaseSession() which is called by pam_systemd,
the user,session and seat state files will also still be available.
The garbage-collector will miss them!

In src/login/logind.c:manager_gc() the while loops will never be entered.


The user slice units will start, then the match_job_removed() and co
signals on these units will call session_add_to_gc_queue() and
user_add_to_gc_queue() to push to gc_queue when done, in the mean time
the manager_gc() will consume the gc_queue and remove them

IOW *just* before and after the ReleaseSession() the manager
{session|user}_gc_queue queues might be empty, hence session, users
and seats will never be cleaned! the user's slice will still be alive...


To fix this, I'm attaching two patches and I can say that they are
related to each other from the perspective of the described bug, and at
the same time they are independent of each other from a general
perspective!


1) Make method_release_session() call session_add_to_gc_queue()
This will ensure that the released session is in the gc_queue.

This change gives the garbage collector a chance to collect sessions,
and should not affect the logind behaviour and other display managers,
the session_check_gc() is able to detect if the session is still valid.

The thing is that from a quick git log the method_release_session()
never called session_add_to_gc_queue(), so this bug was introduced or
made *visible* by another change... (not sure)


2) As in commit 63966da86d8e, in function session_check_gc() the session
manager will always be around so don't check it in order to
garbage-collect the session.

Thanks!
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/2] logind: make the manager able to collect closed sessions

2014-01-03 Thread Djalal Harouni

Currently on logout, session and user state files might stay and will
not be cleaned up, this is true on systems where dbus TerminateSession()
is not called on logouts.

The manager garbage-collector will miss them due to the session_gc_queue
being empty. A call to dbus TerminateSession() which will call
session_stop() is needed in order to push the session into the
session_gc_queue.

To ensure that sessions will have the chance to be collected, make the
method_release_session() call session_add_to_gc_queue() before
finishing, this gives the manager_gc() a chance to check if the current
session should be collected and if so call session_stop() on it.
---
 src/login/logind-dbus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 08510b5..c18a74a 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -742,6 +742,7 @@ static int method_release_session(sd_bus *bus, 
sd_bus_message *message, void *us
 session_remove_fifo(session);
 session_save(session);
 user_save(session-user);
+session_add_to_gc_queue(session);
 
 return sd_bus_reply_method_return(message, NULL);
 }
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] logind: Don't stop a running session manager from collecting a session

2014-01-03 Thread Djalal Harouni

As in commit 63966da86, the session manager will always be around, so
make sure that in function session_check_gc() we don't check it. This
gives the manager a chance to garbage-collect sessions.
---
 src/login/logind-session.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index dc4b3e1..a78d4dd 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -935,9 +935,6 @@ bool session_check_gc(Session *s, bool drop_not_started) {
 if (s-scope_job  manager_job_is_active(s-manager, s-scope_job))
 return true;
 
-if (s-scope  manager_unit_is_active(s-manager, s-scope))
-return true;
-
 return false;
 }
 
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] Add SELinuxContext configuration item

Le vendredi 03 janvier 2014 à 12:23 +, Jóhann B. Guðmundsson a
écrit :
 On 01/03/2014 10:56 AM, Michael Scherer wrote:
  Le vendredi 03 janvier 2014 à 00:58 +, Jóhann B. Guðmundsson a
  écrit :
  On 12/28/2013 01:30 PM, Lennart Poettering wrote:
  On Fri, 27.12.13 23:26,m...@zarb.org  (m...@zarb.org) wrote:
 
  From: Michael Schererm...@zarb.org
 
  This permit to let system administrators decide of the domain of a 
  service.
  This can be used with templated units to have each service in a 
  différent
  domain ( for example, a per customer database, using MLS or anything ),
  or can be used to force a non selinux enabled system (jvm, erlang, etc)
  to start in a different domain for each service.
  Hmm, so far (as I understood it) the SELinux guys always wanted to make
  sure that label configuration stays in the the selinux database and
  nowhere else.
  Right and if you add this you need to add something for the other
  security solutions as well ( apparmor etc ).
  I do not see why we need to equally support all MAC frameworks for each
  change we do.
 
 Because people will start to expect being able to configure that there.

So they can as well start to fill bug report and start to contribute
code for this.

We didn't added detection of all security framework for
ConditionSecurity at the first patch, it was added later as people
expressed interest ( hence the lack of tomoyo ), so I expect the same to
be done for security frameworks. Also, having done my homework, IMA do
not have the concept of domain, apparmor has profiles, and I have no
idea for smack, so I prefer to defer integration to people who know,
based on their use cases.

And while I am familiar enough with apparmor to create a
  equivalent setting ( and plan to do ), I have no idea on how to
  translate the concept for smack, ima and tomoyo.
 
  In fact, the mere fact that tomoyo is not handled at all already show
  that we do treat all security framework as equal.
 
 How do you suppose we deal with man pages if selinux is not installed 
 but still refer to this.

In the same way we do for all #ifdef feature. For example, for PAMName,
the documentation is always present. 

 Wont users also need to check if selinux is enabled or not in the unit 
 file?

I would rather do it in the C code, no need to have everybody asking for
it.

 Should systemd warn users if selinux is not installed,enabled and fail or?

It all depend. Either we are consistent with the other settings ( ie,
setting a syscall filter will fail if not supported on the kernel ), and
so fail, or we decide that selinux is special and we should silently
ignore failure if it cannot be applied.

I choose the first one for the first patch, but adding a conditional
would be trivial if we decide to silently ignore if the setting cannot
be applied. 

The main issue being of course that people who disable selinux do not
always properly disable it ( ie using permissive rather than selinux=0
).


 
 
  This introduces yet another place for administrators to look at while
  debugging problems so the question to ask here is.
 
  Is adding this ability to unit files the right way to solve what's
  trying to be solved here?
  As Dan said, yes.
 
 I would prefer if app writers do not start hard coding SELinux contexts 
 into the unit file
 
 I hardly call that solid yes and this is what will start taking place if 
 this is introduced into the unit files.

Then what about :
I like this patch, and I have seen people saying we have this
capability in RHEL7  :^(

We should separate the concern about people in distribution/upstream
using it if offered and the rest of the world. Distribution policy is a
matter of the distribution. If Fedora wish to forbid this unless there
is a good use case, that's up to Fedora to do it, and to have it
integrated into rpmlint or anything.

I must also say that I didn't really see a rush from application
developers to add selinux support or anything, and that people can
already distribute policy along their application, with all support
problem this could create for distributions. So we already have the
problem, adding the setting in systemd wouldn't change much. 

 
  However, as said, there is some case where the approach of making the
  transition based on the executed filename is not sufficient. For
  example, the erlang vm, the jvm, etc, because each software will run in
  the same domain, in different processes, because that's always the same
  jvm being used. See the bug I mentioned before.
 
  Now, if you have a more precise feedback and/or a better proposal,
 
 Add this to the daemon startup itself ( the confile or the code ) and or 
 use runcon in an exec start pre section to set this up.

Runcon cannot be used in ExecStartPre, that's like su. So you have to
run it in ExecStart, and this make things harder for sysadmins, and
doesn't look like in line with systemd philosophy since that's replacing
configuration by code. 

On the integration

[systemd-devel] Multiseat session creation fail, VT number not 0

2014-01-03 Thread Matthew Monaco

I was having trouble getting a session on seat1 with v208, so I moved to git
which has a nicer error message than EINVAL:

pam_systemd(lightdm:session): Asking logind to create session: uid=1000 pid=637
service=lightdm type=x11 class=user seat=seat1 vtnr=2 tty= display=:1 remote=no
remote_user= remote_host=

pam_systemd(lightdm:session): Failed to create session: Seat has no VTs but VT
number not 0

I'm using lightdm 1.8.5. My X servers are

/usr/sbin/X :0 -config xorg-seat0.conf -seat seat0 -auth /run/lightdm/root/:0
-nolisten tcp vt1 -novtswitch

/usr/sbin/X -sharevts :1 -config xorg-seat1.conf -seat seat1 -auth
/run/lightdm/root/:1 -nolisten tcp vt2 -novtswitch

(I have no problems with seat0).

This same setup used to work with systemd ~v205, but I have used multiseat in a
while do to a move.

So, I don't understand where the failure is. Is lightdm starting X on the wrong
vt? Why is vt2/tty2 not allowed for a second seat?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Multiseat session creation fail, VT number not 0

2014-01-03 Thread David Herrmann

Hi

On Fri, Jan 3, 2014 at 3:24 PM, Matthew Monaco m...@0x01b.net wrote:
 I was having trouble getting a session on seat1 with v208, so I moved to git
 which has a nicer error message than EINVAL:

 pam_systemd(lightdm:session): Asking logind to create session: uid=1000 
 pid=637
 service=lightdm type=x11 class=user seat=seat1 vtnr=2 tty= display=:1 
 remote=no
 remote_user= remote_host=

Yeah, that vtnr=2 line is wrong. You really shouldn't set any VTNR if
seat!=seat0. I think the correct fix would be to set vtnr=0 in
get_seat_from_display() in pam-module.c if we're not on seat0.

As Lennart changed that during the sd-bus transition (if I read the
history correctly..), maybe he can comment on that.

Another related commit is this:
  
http://cgit.freedesktop.org/systemd/systemd/commit/src/login?id=c506027af881a9e4210845a7a8a6ec5910aa0f3b
Which I am still not sure is the correct thing to do.

Thanks
David


 pam_systemd(lightdm:session): Failed to create session: Seat has no VTs but VT
 number not 0

 I'm using lightdm 1.8.5. My X servers are

 /usr/sbin/X :0 -config xorg-seat0.conf -seat seat0 -auth /run/lightdm/root/:0
 -nolisten tcp vt1 -novtswitch

 /usr/sbin/X -sharevts :1 -config xorg-seat1.conf -seat seat1 -auth
 /run/lightdm/root/:1 -nolisten tcp vt2 -novtswitch

 (I have no problems with seat0).

 This same setup used to work with systemd ~v205, but I have used multiseat in 
 a
 while do to a move.

 So, I don't understand where the failure is. Is lightdm starting X on the 
 wrong
 vt? Why is vt2/tty2 not allowed for a second seat?
 ___
 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] [RFC] Initial libsystemd-asyncns commit

2014-01-03 Thread Daniel Buch

Yes im still working on it (Should have been finished days ago. 24 hours a
day is again not enough -.-). So far i went for the threaded version, and
cleaned up stuff according to this mail thread.

And i updated it to match systemd with c99 null initialisation of structs
etc. That might be wrong since you want it to be public now?


2014/1/3 David Timothy Strauss da...@davidstrauss.net

 Just to consider what other folks are doing, I know Fedora builds
 libcurl with a thread-isolated, NSS-based resolver.

 On a less-related note, at Pantheon improve DNS performance on servers
 by setting resolv.conf to localhost and running Unbound there. Unbound
 then uses the datacenter's recursive DNS servers for things that miss
 the local cache. This minimizes the time spent in blocked threads --
 and waiting for lookups even with async libraries. As a bonus, you
 also get DNSSec validation when possible.
 ___
 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

[systemd-devel] [PATCH 2/2] Add AppArmor profile switching

2014-01-03 Thread misc

From: Michael Scherer m...@zarb.org

This permit to switch to a specific apparmor profile when starting a daemon. 
This
will result in a non operation if apparmor is disabled.
---
 man/systemd.exec.xml  | 12 
 src/core/dbus-execute.c   |  1 +
 src/core/execute.c| 19 +++
 src/core/execute.h|  2 ++
 src/core/load-fragment-gperf.gperf.m4 |  3 ++-
 src/shared/exit-status.c  |  3 +++
 src/shared/exit-status.h  |  3 ++-
 7 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index 17748d4..250de13 100644
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -931,6 +931,18 @@
 /varlistentry
 
 varlistentry
+
termvarnameAppArmorProfile=/varname/term
+
+listitemparaTake a profile name as 
argument.
+The process executed by the unit will switch to
+this profile when started. Profiles must 
already
+be loaded in the kernel, or the unit will fail.
+This result in a non operation if AppArmor is 
not
+enabled.
+/para/listitem
+/varlistentry
+
+varlistentry
 termvarnameIgnoreSIGPIPE=/varname/term
 
 listitemparaTakes a boolean
diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index b79a456..df55fd0 100644
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -422,6 +422,7 @@ const sd_bus_vtable bus_exec_vtable[] = {
 SD_BUS_PROPERTY(PrivateNetwork, b, bus_property_get_bool, 
offsetof(ExecContext, private_network), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY(SameProcessGroup, b, bus_property_get_bool, 
offsetof(ExecContext, same_pgrp), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY(UtmpIdentifier, s, NULL, offsetof(ExecContext, 
utmp_id), SD_BUS_VTABLE_PROPERTY_CONST),
+SD_BUS_PROPERTY(AppArmorProfile, s, NULL, offsetof(ExecContext, 
apparmor_profile), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY(IgnoreSIGPIPE, b, bus_property_get_bool, 
offsetof(ExecContext, ignore_sigpipe), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY(NoNewPrivileges, b, bus_property_get_bool, 
offsetof(ExecContext, no_new_privileges), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY(SystemCallFilter, au, property_get_syscall_filter, 
0, SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/execute.c b/src/core/execute.c
index 6ae9a5e..b0f4cd7 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -68,6 +68,7 @@
 #include fileio.h
 #include unit.h
 #include async.h
+#include apparmor-util.h
 
 #define IDLE_TIMEOUT_USEC (5*USEC_PER_SEC)
 #define IDLE_TIMEOUT2_USEC (1*USEC_PER_SEC)
@@ -1570,6 +1571,16 @@ int exec_spawn(ExecCommand *command,
 goto fail_child;
 }
 }
+
+if (context-apparmor_profile) {
+if (use_apparmor()) {
+err = 
switch_apparmor_profile(context-apparmor_profile);
+if (err  0) {
+r = EXIT_APPARMOR;
+goto fail_child;
+}
+}
+}
 }
 
 err = build_environment(context, n_fds, watchdog_usec, home, 
username, shell, our_env);
@@ -1728,6 +1739,9 @@ void exec_context_done(ExecContext *c) {
 free(c-utmp_id);
 c-utmp_id = NULL;
 
+free(c-apparmor_profile);
+c-apparmor_profile = NULL;
+
 free(c-syscall_filter);
 c-syscall_filter = NULL;
 }
@@ -2096,6 +2110,11 @@ void exec_context_dump(ExecContext *c, FILE* f, const 
char *prefix) {
 fprintf(f,
 %sUtmpIdentifier: %s\n,
 prefix, c-utmp_id);
+
+if (c-apparmor_profile)
+fprintf(f,
+%sAppArmorProfile: %s\n,
+prefix, c-apparmor_profile);
 }
 
 void exec_status_start(ExecStatus *s, pid_t pid) {
diff --git a/src/core/execute.h b/src/core/execute.h
index 989373f..754f163 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -133,6 +133,8 @@ struct ExecContext {
 
 char *utmp_id;
 
+char *apparmor_profile;
+
 char **read_write_dirs, **read_only_dirs, **inaccessible_dirs;
 unsigned long mount_flags;
 
diff --git a/src/core/load-fragment-gperf.gperf.m4

[systemd-devel] [PATCH 1/2] Add switch_apparmor_profile helper, to switch the profile of the next command to run. This can be used to load a custom apparmor profile for a unit.

2014-01-03 Thread misc

From: Michael Scherer m...@zarb.org

---
 src/shared/apparmor-util.c | 15 +++
 src/shared/apparmor-util.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/src/shared/apparmor-util.c b/src/shared/apparmor-util.c
index 2b85da1..a75bec4 100644
--- a/src/shared/apparmor-util.c
+++ b/src/shared/apparmor-util.c
@@ -39,3 +39,18 @@ bool use_apparmor(void) {
 
 return use_apparmor_cached;
 }
+
+int switch_apparmor_profile(const char * profile) {
+_cleanup_free_ char *filename = NULL;
+_cleanup_fclose_ FILE *proc = NULL;
+
+if (asprintf (filename, /proc/%d/attr/exec, getpid()) 0)
+return -ENOMEM;
+
+proc = fopen (filename, w);
+if (! proc)
+return -errno;
+
+fprintf (proc, exec %s\n, profile);
+return 0;
+}
diff --git a/src/shared/apparmor-util.h b/src/shared/apparmor-util.h
index 4b056a1..f27608d 100644
--- a/src/shared/apparmor-util.h
+++ b/src/shared/apparmor-util.h
@@ -24,3 +24,4 @@
 #include stdbool.h
 
 bool use_apparmor(void);
+int switch_apparmor_profile(const char * profile);
-- 
1.8.4.2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Apparmor profile switching support

2014-01-03 Thread misc

As discussed on the SELinux thread, this patch attempt to offer the same
level of configuration for Apparmor distributions by permitting to the
sysadmin to set the profile used by a unit. I didn't tested it but would 
like to get early feedback on it from openSUSE and Ubuntu users, as they
are the 2 main set of users of AppArmor.

Main inspiration come from the upstart support, on 
https://code.launchpad.net/~mdeslaur/upstart/apparmor-support
However, we are currently lacking the capacity of using directly a on disk 
profile, and
I am not sure on the best way to support that. 

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] Add SELinuxContext configuration item

2014-01-03 Thread Daniel J Walsh

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 01/03/2014 09:16 AM, Michael Scherer wrote:
 Le vendredi 03 janvier 2014 à 12:23 +, Jóhann B. Guðmundsson a écrit
 :
 On 01/03/2014 10:56 AM, Michael Scherer wrote:
 Le vendredi 03 janvier 2014 à 00:58 +, Jóhann B. Guðmundsson a 
 écrit :
 On 12/28/2013 01:30 PM, Lennart Poettering wrote:
 On Fri, 27.12.13 23:26,m...@zarb.org  (m...@zarb.org) wrote:
 
 From: Michael Schererm...@zarb.org
 
 This permit to let system administrators decide of the domain
 of a service. This can be used with templated units to have
 each service in a différent domain ( for example, a per
 customer database, using MLS or anything ), or can be used to
 force a non selinux enabled system (jvm, erlang, etc) to start
 in a different domain for each service.
 Hmm, so far (as I understood it) the SELinux guys always wanted to
 make sure that label configuration stays in the the selinux
 database and nowhere else.
 Right and if you add this you need to add something for the other 
 security solutions as well ( apparmor etc ).
 I do not see why we need to equally support all MAC frameworks for
 each change we do.
 
 Because people will start to expect being able to configure that there.
 
 So they can as well start to fill bug report and start to contribute code
 for this.
 
 We didn't added detection of all security framework for ConditionSecurity
 at the first patch, it was added later as people expressed interest ( hence
 the lack of tomoyo ), so I expect the same to be done for security
 frameworks. Also, having done my homework, IMA do not have the concept of
 domain, apparmor has profiles, and I have no idea for smack, so I prefer to
 defer integration to people who know, based on their use cases.
 
 And while I am familiar enough with apparmor to create a equivalent
 setting ( and plan to do ), I have no idea on how to translate the
 concept for smack, ima and tomoyo.
 
 In fact, the mere fact that tomoyo is not handled at all already show 
 that we do treat all security framework as equal.
 
 How do you suppose we deal with man pages if selinux is not installed but
 still refer to this.
 
 In the same way we do for all #ifdef feature. For example, for PAMName, the
 documentation is always present.
 
 Wont users also need to check if selinux is enabled or not in the unit 
 file?
 
 I would rather do it in the C code, no need to have everybody asking for 
 it.
 
 Should systemd warn users if selinux is not installed,enabled and fail
 or?
 
 It all depend. Either we are consistent with the other settings ( ie, 
 setting a syscall filter will fail if not supported on the kernel ), and so
 fail, or we decide that selinux is special and we should silently ignore
 failure if it cannot be applied.
 
 I choose the first one for the first patch, but adding a conditional would
 be trivial if we decide to silently ignore if the setting cannot be
 applied.
 
 The main issue being of course that people who disable selinux do not 
 always properly disable it ( ie using permissive rather than selinux=0 ).
 
 
 
 
 This introduces yet another place for administrators to look at
 while debugging problems so the question to ask here is.
 
 Is adding this ability to unit files the right way to solve what's 
 trying to be solved here?
 As Dan said, yes.
 
 I would prefer if app writers do not start hard coding SELinux contexts
  into the unit file
 
 I hardly call that solid yes and this is what will start taking place if
  this is introduced into the unit files.
 
 Then what about : I like this patch, and I have seen people saying we have
 this capability in RHEL7  :^(
 
 We should separate the concern about people in distribution/upstream using
 it if offered and the rest of the world. Distribution policy is a matter
 of the distribution. If Fedora wish to forbid this unless there is a good
 use case, that's up to Fedora to do it, and to have it integrated into
 rpmlint or anything.
 
 I must also say that I didn't really see a rush from application developers
 to add selinux support or anything, and that people can already distribute
 policy along their application, with all support problem this could create
 for distributions. So we already have the problem, adding the setting in
 systemd wouldn't change much.
 
 
 However, as said, there is some case where the approach of making the 
 transition based on the executed filename is not sufficient. For 
 example, the erlang vm, the jvm, etc, because each software will run
 in the same domain, in different processes, because that's always the
 same jvm being used. See the bug I mentioned before.
 
 Now, if you have a more precise feedback and/or a better proposal,
 
 Add this to the daemon startup itself ( the confile or the code ) and or
  use runcon in an exec start pre section to set this up.
 
 Runcon cannot be used in ExecStartPre, that's like su. So you have to run
 it in ExecStart, and this make things harder for sysadmins, and

Re: [systemd-devel] Apparmor profile switching support

Le vendredi 03 janvier 2014 à 17:22 +0100, m...@zarb.org a écrit :
 As discussed on the SELinux thread, this patch attempt to offer the same
 level of configuration for Apparmor distributions by permitting to the
 sysadmin to set the profile used by a unit. I didn't tested it but would 
 like to get early feedback on it from openSUSE and Ubuntu users, as they
 are the 2 main set of users of AppArmor.
 
 Main inspiration come from the upstart support, on 
 https://code.launchpad.net/~mdeslaur/upstart/apparmor-support
 However, we are currently lacking the capacity of using directly a on disk 
 profile, and
 I am not sure on the best way to support that. 

I have also been told on irc that Michael Stapelberg wrote the same kind
of patch ( if not the same, given there isn't much possible variation ),
cf https://lists.debian.org/debian-security/2014/01/msg8.html

-- 
Michael Scherer

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] Add SELinuxContext configuration item

2014-01-03 Thread Zbigniew Jędrzejewski-Szmek

On Fri, Jan 03, 2014 at 11:48:49AM -0500, Daniel J Walsh wrote:
  Should systemd warn users if selinux is not installed,enabled and fail
  or?
  
  It all depend. Either we are consistent with the other settings ( ie, 
  setting a syscall filter will fail if not supported on the kernel ), and so
  fail, or we decide that selinux is special and we should silently ignore
  failure if it cannot be applied.
  
  I choose the first one for the first patch, but adding a conditional would
  be trivial if we decide to silently ignore if the setting cannot be
  applied.
I think the usual style of - as the first character of RHS meaning that
the setting can be ignored should be used.

In general, if selinux=0 is used, or selinux support is not compiled
in, those options should not result in failure. So the algorithm should
be: if disabled, ignore, if enabled, and impossible to apply, fail, unless
- was prefixed.

  In anycase Lennart decides this to me this seems like a workaround being
   put in systemd for a limitation in selinux or the concept there of with
   the added complexity for administrators.
  
  yes, that's put in systemd because placing this in every other possible 
  place would be duplication. As i said, we could add this to jvm, to erlang,
  etc, but it would scale ( ie, we would have more place to look for
  configuration )
  
 Well thinking about this again, I think still to the single label.  Lets not
 break the field up into multiple labels.
Yes, this sounds nicer: easier to document, to configure, etc.

 And not make it SELinux specific.  Maybe the field could be SecurityLabel:
 
 That would allow smack to also use the field and any other LSM that used a
 labeling system.
This would make it impossible to use the same unit file with more than
one security framework. This might be desirable, even if they cannot be enabled
at the same time.

Zbyszek

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] Update .mailmap file

2014-01-03 Thread Stefan Beller

This commit updates email addresses of people, who are already in the
.mailmap file, so I'd assume they have sorted out their viewpoint on
privacy within the .mailmap file.

The entries for this commit have been produced using:
# Finding out duplicates by comparing email addresses:
git shortlog -sne |awk '{ print $NF }' |sort |uniq -d

# Finding out duplicates by comparing names:
git shortlog -sne |awk '{ NF--; $1=; print }' |sort |uniq -d
---
 .mailmap | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 2aa7cb1..bda835c 100644
--- a/.mailmap
+++ b/.mailmap
@@ -3,6 +3,7 @@ Kay Sievers k...@vrfy.org kay.siev...@vrfy.org
 Kay Sievers k...@vrfy.org kay.siev...@suse.de
 Kay Sievers k...@vrfy.org k...@pim.off.vrfy.org
 Kay Sievers k...@vrfy.org kay@pim
+Kay Sievers k...@vrfy.org k...@yik.fritz.box
 Greg KH g...@kroah.com
 Greg KH g...@kroah.com g...@kroah.com
 Greg KH g...@kroah.com greg@press.(none)
@@ -36,7 +37,7 @@ Michal Soltys sol...@ziu.info n...@ziu.info
 Piter PUNK piterp...@slackware.com pite...@terra.com.br
 Richard Hughes rich...@hughsie.com hughsi...@gmail.com
 Robby Workman r...@rlworkman.net rwork...@slackware.com
-Shawn Landden shawnland...@gmail.com
+Shawn Landden shawnland...@gmail.com sh...@churchofgit.com
 Simon Peeters peeters.si...@gmail.com
 Tobias Klauser tklau...@distanz.ch tklau...@nuerscht.ch
 Miklos Vajna vmik...@frugalware.org vmik...@gmail.com
@@ -50,6 +51,7 @@ Ted Ts'o ty...@mit.edu
 Tobias Klauser tklau...@access.unizh.ch
 Tobias Klauser tklau...@access.unizh.ch tklau...@access.unizh.chbk
 Tobias Klauser tklau...@access.unizh.ch klau...@access.unizh.ch
+Tobias Klauser tklau...@access.unizh.ch tklau...@distanz.ch
 Patrick Mansfield patm...@us.ibm.com
 Christophe Varoqui christophe.varo...@free.fr
 Daniel Stekloff dstek...@us.ibm.com
-- 
1.8.5.2.229.g4448466

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/2] swap: remove if/else with the same data path

2014-01-03 Thread Stefan Beller

This was introduced in e1770af812 (2012-02-03, swap: replace failure
boolean by result enum).

This just removes unneeded lines of code, no functional change.
---
 src/core/swap.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/core/swap.c b/src/core/swap.c
index e0627db..6b204df 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -999,10 +999,7 @@ static void swap_sigchld_event(Unit *u, pid_t pid, int 
code, int status) {
 case SWAP_DEACTIVATING_SIGKILL:
 case SWAP_DEACTIVATING_SIGTERM:
 
-if (f == SWAP_SUCCESS)
-swap_enter_dead(s, f);
-else
-swap_enter_dead(s, f);
+swap_enter_dead(s, f);
 break;
 
 default:
-- 
1.8.5.2.229.g4448466

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [RFC] networkd/udev: match on 'ethernet' devices

Hi,

I just pushed a change[0] which allows the match syntax
Type=ethernet to match on network devices without a DEVTYPE.

We had a discussion on IRC whether we should call it Type=wired or
Type=ethernet. I think the former may be more intuitive, but the
latter seems to be more in line with what is done elsewhere (connman
calls it ethernet, and udev prefixes the devices names with 'en').

Any thoughts?

Cheers,

Tom

[0]: 
http://cgit.freedesktop.org/systemd/systemd/commit/?id=4cd1214db6cf4b262e8ce6381bc710091b375c96
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] Add SELinuxContext configuration item

Le vendredi 03 janvier 2014 à 18:21 +0100, Zbigniew Jędrzejewski-Szmek a
écrit :
 On Fri, Jan 03, 2014 at 11:48:49AM -0500, Daniel J Walsh wrote:
   Should systemd warn users if selinux is not installed,enabled and fail
   or?
   
   It all depend. Either we are consistent with the other settings ( ie, 
   setting a syscall filter will fail if not supported on the kernel ), and 
   so
   fail, or we decide that selinux is special and we should silently ignore
   failure if it cannot be applied.
   
   I choose the first one for the first patch, but adding a conditional would
   be trivial if we decide to silently ignore if the setting cannot be
   applied.
 I think the usual style of - as the first character of RHS meaning that
 the setting can be ignored should be used.
 
 In general, if selinux=0 is used, or selinux support is not compiled
 in, those options should not result in failure. So the algorithm should
 be: if disabled, ignore, if enabled, and impossible to apply, fail, unless
 - was prefixed.

Good idea, i have coded that, I will test and send it later.

-- 
Michael Scherer

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] Add SELinuxContext configuration item

2014-01-03 Thread Kay Sievers

On Fri, Jan 3, 2014 at 6:21 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:

 And not make it SELinux specific.  Maybe the field could be SecurityLabel:

 That would allow smack to also use the field and any other LSM that used a
 labeling system.
 This would make it impossible to use the same unit file with more than
 one security framework. This might be desirable, even if they cannot be 
 enabled
 at the same time.

Udev uses:
  SECLABEL{selinux}=foo
  SECLABEL{smack}=bar

I think we should be able to distinguish the LSM-module-specific label
type somehow in the key or value.

Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Multiseat session creation fail, VT number not 0

2014-01-03 Thread Matthew Monaco

On 01/03/2014 07:51 AM, David Herrmann wrote:
 Hi
 
 On Fri, Jan 3, 2014 at 3:24 PM, Matthew Monaco m...@0x01b.net wrote:
 I was having trouble getting a session on seat1 with v208, so I moved to git
 which has a nicer error message than EINVAL:

 pam_systemd(lightdm:session): Asking logind to create session: uid=1000 
 pid=637
 service=lightdm type=x11 class=user seat=seat1 vtnr=2 tty= display=:1 
 remote=no
 remote_user= remote_host=
 
 Yeah, that vtnr=2 line is wrong. You really shouldn't set any VTNR if
 seat!=seat0. I think the correct fix would be to set vtnr=0 in
 get_seat_from_display() in pam-module.c if we're not on seat0.
 

Well, I just added

if (seat  !streq(seat, seat0)) {
pam_syslog(handle, LOG_WARNING,
Ignoring vtnr %d for %s which is not seat0, vtnr, seat);
vtrn = 0;
}

because in my case vtnr was coming from pam_getenv(XDG_VTNR), and
get_seat_from_display() isn't called.

But thank you, my system is a bit more usable now =)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v2] systemctl: improve readability on failed commands

2014-01-03 Thread Thomas H.P. Andersen

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Not long ago a failed command would print:
Failed to start something.service: ...
regardless of whether the command was to start/stop/restart/etc.

With e3e0314 this was improved to print the method used. E.g. for stopping:
Failed to StopUnit something.service: ...

This patch matches the method to a more human readable word. E.g:
Failed to stop something.service: ...
---
 src/systemctl/systemctl.c | 58 ++-
 1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 67bc426..01a4489 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -135,6 +135,22 @@ static char *arg_host = NULL;
 static unsigned arg_lines = 10;
 static OutputMode arg_output = OUTPUT_SHORT;
 static bool arg_plain = false;
+static const struct {
+const char *verb;
+const char *method;
+} unit_actions[] = {
+{ start, StartUnit },
+{ stop,  StopUnit },
+{ condstop,  StopUnit },
+{ reload,ReloadUnit },
+{ restart,   RestartUnit },
+{ try-restart,   TryRestartUnit },
+{ condrestart,   TryRestartUnit },
+{ reload-or-restart, ReloadOrRestartUnit },
+{ reload-or-try-restart, ReloadOrTryRestartUnit },
+{ condreload,ReloadOrTryRestartUnit },
+{ force-reload,  ReloadOrTryRestartUnit }
+};
 
 static int daemon_reload(sd_bus *bus, char **args);
 static int halt_now(enum action a);
@@ -2039,6 +2055,26 @@ static int check_triggering_units(
 return 0;
 }
 
+static const char *verb_to_method(const char *verb) {
+   uint i;
+
+   for (i = 0; i  ELEMENTSOF(unit_actions); i++)
+if (streq_ptr(unit_actions[i].verb, verb))
+return unit_actions[i].method;
+
+   return StartUnit;
+}
+
+static const char *method_to_verb(const char *method) {
+   uint i;
+
+   for (i = 0; i  ELEMENTSOF(unit_actions); i++)
+if (streq_ptr(unit_actions[i].method, method))
+return unit_actions[i].verb;
+
+   return n/a;
+}
+
 static int start_unit_one(
 sd_bus *bus,
 const char *method,
@@ -2067,12 +2103,16 @@ static int start_unit_one(
 reply,
 ss, name, mode);
 if (r  0) {
+const char *verb;
+
 if (r == -ENOENT  arg_action != ACTION_SYSTEMCTL)
 /* There's always a fallback possible for
  * legacy actions. */
 return -EADDRNOTAVAIL;
 
-log_error(Failed to %s %s: %s, method, name, 
bus_error_message(error, r));
+verb = method_to_verb(method);
+
+log_error(Failed to %s %s: %s, verb, name, 
bus_error_message(error, r));
 return r;
 }
 
@@ -2191,21 +2231,7 @@ static int start_unit(sd_bus *bus, char **args) {
 
 if (arg_action == ACTION_SYSTEMCTL) {
 enum action action;
-method =
-streq(args[0], stop) ||
-streq(args[0], condstop)  ? StopUnit :
-streq(args[0], reload)? ReloadUnit 
:
-streq(args[0], restart)   ? 
RestartUnit :
-
-streq(args[0], try-restart)   ||
-streq(args[0], condrestart)   ? 
TryRestartUnit :
-
-streq(args[0], reload-or-restart) ? 
ReloadOrRestartUnit :
-
-streq(args[0], reload-or-try-restart) ||
-streq(args[0], condreload)||
-streq(args[0], force-reload)  ? 
ReloadOrTryRestartUnit :
-  StartUnit;
+method = verb_to_method(args[0]);
 action = verb_to_action(args[0]);
 
 mode = streq(args[0], isolate) ? isolate :
-- 
1.8.4.2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH v2] systemctl: improve readability on failed commands

2014-01-03 Thread Zbigniew Jędrzejewski-Szmek

On Fri, Jan 03, 2014 at 11:59:08PM +0100, Thomas H.P. Andersen wrote:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com
 
 Not long ago a failed command would print:
 Failed to start something.service: ...
 regardless of whether the command was to start/stop/restart/etc.
 
 With e3e0314 this was improved to print the method used. E.g. for stopping:
 Failed to StopUnit something.service: ...
 
 This patch matches the method to a more human readable word. E.g:
 Failed to stop something.service: ...
Looks good.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [RFC] networkd/udev: match on 'ethernet' devices

2014-01-03 Thread Greg KH

On Fri, Jan 03, 2014 at 08:54:17PM +0100, Tom Gundersen wrote:
 Hi,
 
 I just pushed a change[0] which allows the match syntax
 Type=ethernet to match on network devices without a DEVTYPE.
 
 We had a discussion on IRC whether we should call it Type=wired or
 Type=ethernet. I think the former may be more intuitive, but the
 latter seems to be more in line with what is done elsewhere (connman
 calls it ethernet, and udev prefixes the devices names with 'en').
 
 Any thoughts?

Any reason why the kernel can't be setting this value in the first place
so you don't have to rely on it being null?

Seems like a kernel issue.

Not to imply that this patch is not right at all though, it looks good
to me.

thanks,

greg k-h
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [RFC] networkd/udev: match on 'ethernet' devices

On Sat, Jan 4, 2014 at 1:22 AM, Greg KH gre...@linuxfoundation.org wrote:
 On Fri, Jan 03, 2014 at 08:54:17PM +0100, Tom Gundersen wrote:
 Hi,

 I just pushed a change[0] which allows the match syntax
 Type=ethernet to match on network devices without a DEVTYPE.

 We had a discussion on IRC whether we should call it Type=wired or
 Type=ethernet. I think the former may be more intuitive, but the
 latter seems to be more in line with what is done elsewhere (connman
 calls it ethernet, and udev prefixes the devices names with 'en').

 Any thoughts?

 Any reason why the kernel can't be setting this value in the first place
 so you don't have to rely on it being null?

 Seems like a kernel issue.

I asked Marcel the same thing earlier. He said:

[Friday 03 January 2014] [19:58:22] holtmann  Because you have to
touch every single driver to get this done properly.
[Friday 03 January 2014] [19:58:34] holtmann  So DEVTYPE= means it
is wired Ethernet.
[Friday 03 January 2014] [19:58:42] holtmann  If that is not true,
then that is a bug.

From my point of view, I'd prefer the kernel just doing the right
thing (and I'd be happy to write the patches), but my impression was
that that's not going to happen...(?)

Also, if we start setting DEVTYPE=ethernet in the kernel, I guess apps
(such as connman) that relies on DEVTYPE=(null) to detect ethernet
devices will break.

 Not to imply that this patch is not right at all though, it looks good
 to me.

Cool. It is intentionally kept so that it will keep working in case
the kernel starts setting DEVTYPE=ethernet (as long as we agree on the
same name, that is).

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] various cleanups

Hello,

I have tried to do some cleanups in src/shared 

The CACHED_METHOD one might be going a bit to far,
so feel free to drop it if you don't like it.

[PATCH 1/5] shared: procfs_file_alloca: handle pid==0
[PATCH 2/5] util: CACHED_METHOD macro
[PATCH 3/5] shared: util.c: unify split and split_quoted
[PATCH 4/5] util.c: use read_one_line_file where possible
[PATCH 5/5] strv: multiple cleanups

 src/core/main.c |  20 ++--
 src/modules-load/modules-load.c |   6 +-
 src/shared/apparmor-util.c  |  16 +--
 src/shared/audit.c  |  10 +-
 src/shared/cgroup-util.c|   5 +-
 src/shared/ima-util.c   |  11 +-
 src/shared/macro.h  |  16 +++
 src/shared/path-lookup.c|  83 ---
 src/shared/smack-util.c |  15 ++-
 src/shared/strv.c   | 158 +--
 src/shared/strv.h   |   6 +-
 src/shared/util.c   | 229 +++-
 src/shared/util.h   |  23 ++--
 src/sysctl/sysctl.c |   6 +-
 src/systemctl/systemctl.c   |  17 ++-
 src/test/test-strv.c|  77 +-
 16 files changed, 208 insertions(+), 490 deletions(-)

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 5/5] strv: multiple cleanups

- turn strv_merge into strv_extend_strv.
   appending strv b to the end of strv a instead of creating a new strv
- strv_append: remove in favor of strv_extend and strv_push.
- strv_remove: write slightly more elegant
- strv_remove_prefix: remove unused function
- strv_overlap: use strv_contains
- strv_printf: STRV_FOREACH handles NULL correctly
---
 src/core/main.c |  20 ++---
 src/modules-load/modules-load.c |   6 +-
 src/shared/path-lookup.c|  83 ++---
 src/shared/strv.c   | 158 
 src/shared/strv.h   |   6 +-
 src/sysctl/sysctl.c |   6 +-
 src/systemctl/systemctl.c   |  17 ++---
 src/test/test-strv.c|  77 +++-
 8 files changed, 96 insertions(+), 277 deletions(-)

diff --git a/src/core/main.c b/src/core/main.c
index 064445d..ec68549 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -601,15 +601,12 @@ static int config_parse_join_controllers(const char *unit,
 if (strv_overlap(*a, l)) {
 char **c;
 
-c = strv_merge(*a, l);
-if (!c) {
+if (strv_extend_strv(l, *a)  0) {
 strv_free(l);
 strv_free_free(t);
 return log_oom();
 }
 
-strv_free(l);
-l = c;
 } else {
 char **c;
 
@@ -1853,10 +1850,11 @@ finish:
 shutdown_verb,
 NULL
 };
-char **env_block;
+_cleanup_strv_free_ char **env_block = NULL;
+env_block = strv_copy(environ);
 
 if (arm_reboot_watchdog  arg_shutdown_watchdog  0) {
-char e[32];
+char *e;
 
 /* If we reboot let's set the shutdown
  * watchdog and tell the shutdown binary to
@@ -1865,14 +1863,11 @@ finish:
 watchdog_close(false);
 
 /* Tell the binary how often to ping */
-snprintf(e, sizeof(e), WATCHDOG_USEC=%llu, (unsigned 
long long) arg_shutdown_watchdog);
-char_array_0(e);
+asprintf(e, WATCHDOG_USEC=%llu, (unsigned long 
long) arg_shutdown_watchdog);
 
-env_block = strv_append(environ, e);
-} else {
-env_block = strv_copy(environ);
+strv_push(env_block, e);
+} else
 watchdog_close(true);
-}
 
 /* Avoid the creation of new processes forked by the
  * kernel; at this point, we will not listen to the
@@ -1881,7 +1876,6 @@ finish:
 cg_uninstall_release_agent(SYSTEMD_CGROUP_CONTROLLER);
 
 execve(SYSTEMD_SHUTDOWN_BINARY_PATH, (char **) command_line, 
env_block);
-free(env_block);
 log_error(Failed to execute shutdown binary, freezing: %m);
 }
 
diff --git a/src/modules-load/modules-load.c b/src/modules-load/modules-load.c
index 5d141a8..01987f2 100644
--- a/src/modules-load/modules-load.c
+++ b/src/modules-load/modules-load.c
@@ -64,13 +64,9 @@ static int add_modules(const char *p) {
 if (!k)
 return log_oom();
 
-t = strv_merge(arg_proc_cmdline_modules, k);
-if (!t)
+if (strv_extend_strv(arg_proc_cmdline_modules, k)  0)
 return log_oom();
 
-strv_free(arg_proc_cmdline_modules);
-arg_proc_cmdline_modules = t;
-
 return 0;
 }
 
diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c
index 1a47ea9..e2ca942 100644
--- a/src/shared/path-lookup.c
+++ b/src/shared/path-lookup.c
@@ -90,9 +90,9 @@ static char** user_dirs(
 };
 
 const char *home, *e;
-char *config_home = NULL, *data_home = NULL;
-char **config_dirs = NULL, **data_dirs = NULL;
-char **r = NULL, **t;
+_cleanup_free_ char *config_home = NULL, *data_home = NULL;
+_cleanup_strv_free_ char **config_dirs = NULL, **data_dirs = NULL;
+char **r = NULL;
 
 /* Implement the mechanisms defined in
  *
@@ -150,89 +150,48 @@ static char** user_dirs(
 goto fail;
 
 /* Now merge everything we found. */
-if (generator_early) {
-t = strv_append(r, generator_early);
-if (!t)
+if (generator_early)
+if (strv_extend(r, generator_early)  0)

[systemd-devel] [PATCH 1/5] shared: procfs_file_alloca: handle pid==0

when pid is set to 0 use /proc/self
---
 src/shared/audit.c   | 10 ++
 src/shared/cgroup-util.c |  5 +
 src/shared/util.c| 35 +++
 src/shared/util.h|  8 ++--
 4 files changed, 16 insertions(+), 42 deletions(-)

diff --git a/src/shared/audit.c b/src/shared/audit.c
index 9ab4640..8038ac3 100644
--- a/src/shared/audit.c
+++ b/src/shared/audit.c
@@ -46,10 +46,7 @@ int audit_session_from_pid(pid_t pid, uint32_t *id) {
 if (detect_container(NULL)  0)
 return -ENOTSUP;
 
-if (pid == 0)
-p = /proc/self/sessionid;
-else
-p = procfs_file_alloca(pid, sessionid);
+p = procfs_file_alloca(pid, sessionid);
 
 r = read_one_line_file(p, s);
 if (r  0)
@@ -78,10 +75,7 @@ int audit_loginuid_from_pid(pid_t pid, uid_t *uid) {
 if (detect_container(NULL)  0)
 return -ENOTSUP;
 
-if (pid == 0)
-p = /proc/self/loginuid;
-else
-p = procfs_file_alloca(pid, loginuid);
+p = procfs_file_alloca(pid, loginuid);
 
 r = read_one_line_file(p, s);
 if (r  0)
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index f2af8dc..855c9cd 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -746,10 +746,7 @@ int cg_pid_get_path(const char *controller, pid_t pid, 
char **path) {
 } else
 controller = SYSTEMD_CGROUP_CONTROLLER;
 
-if (pid == 0)
-fs = /proc/self/cgroup;
-else
-fs = procfs_file_alloca(pid, cgroup);
+fs = procfs_file_alloca(pid, cgroup);
 
 f = fopen(fs, re);
 if (!f)
diff --git a/src/shared/util.c b/src/shared/util.c
index f491708..50dac70 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -495,10 +495,7 @@ int get_starttime_of_pid(pid_t pid, unsigned long long 
*st) {
 assert(pid = 0);
 assert(st);
 
-if (pid == 0)
-p = /proc/self/stat;
-else
-p = procfs_file_alloca(pid, stat);
+p = procfs_file_alloca(pid, stat);
 
 f = fopen(p, re);
 if (!f)
@@ -573,10 +570,7 @@ int get_process_comm(pid_t pid, char **name) {
 assert(name);
 assert(pid = 0);
 
-if (pid == 0)
-p = /proc/self/comm;
-else
-p = procfs_file_alloca(pid, comm);
+p = procfs_file_alloca(pid, comm);
 
 r = read_one_line_file(p, name);
 if (r == -ENOENT)
@@ -594,10 +588,7 @@ int get_process_cmdline(pid_t pid, size_t max_length, bool 
comm_fallback, char *
 assert(line);
 assert(pid = 0);
 
-if (pid == 0)
-p = /proc/self/cmdline;
-else
-p = procfs_file_alloca(pid, cmdline);
+p = procfs_file_alloca(pid, cmdline);
 
 f = fopen(p, re);
 if (!f)
@@ -716,10 +707,7 @@ int get_process_capeff(pid_t pid, char **capeff) {
 assert(capeff);
 assert(pid = 0);
 
-if (pid == 0)
-p = /proc/self/status;
-else
-p = procfs_file_alloca(pid, status);
+p = procfs_file_alloca(pid, status);
 
 return get_status_field(p, \nCapEff:, capeff);
 }
@@ -732,10 +720,7 @@ int get_process_exe(pid_t pid, char **name) {
 assert(pid = 0);
 assert(name);
 
-if (pid == 0)
-p = /proc/self/exe;
-else
-p = procfs_file_alloca(pid, exe);
+p = procfs_file_alloca(pid, exe);
 
 r = readlink_malloc(p, name);
 if (r  0)
@@ -2549,10 +2534,7 @@ int get_ctty_devnr(pid_t pid, dev_t *d) {
 
 assert(pid = 0);
 
-if (pid == 0)
-fn = /proc/self/stat;
-else
-fn = procfs_file_alloca(pid, stat);
+fn = procfs_file_alloca(pid, stat);
 
 f = fopen(fn, re);
 if (!f)
@@ -5095,10 +5077,7 @@ int getenv_for_pid(pid_t pid, const char *field, char 
**_value) {
 assert(field);
 assert(_value);
 
-if (pid == 0)
-path = /proc/self/environ;
-else
-path = procfs_file_alloca(pid, environ);
+path = procfs_file_alloca(pid, environ);
 
 f = fopen(path, re);
 if (!f)
diff --git a/src/shared/util.h b/src/shared/util.h
index f6d2ced..311315d 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -762,8 +762,12 @@ int unlink_noerrno(const char *path);
 ({  \
 pid_t _pid_ = (pid);\
 char *_r_;  \
-_r_ = alloca(sizeof(/proc/) -1 + DECIMAL_STR_MAX(pid_t) + 1 
+ sizeof(field)); \
-sprintf(_r_, /proc/%lu/ field, (unsigned long) _pid_); \
+if (_pid_ == 0) {

[systemd-devel] [PATCH 4/5] util.c: use read_one_line_file where possible

---
 src/shared/util.c | 48 +++-
 1 file changed, 15 insertions(+), 33 deletions(-)

diff --git a/src/shared/util.c b/src/shared/util.c
index db3051d..354d7eb 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -404,8 +404,7 @@ char *split(const char *c, size_t *l, const char 
*separator, bool quoted, char *
 
 int get_parent_of_pid(pid_t pid, pid_t *_ppid) {
 int r;
-_cleanup_fclose_ FILE *f = NULL;
-char line[LINE_MAX];
+_cleanup_free_ char *line = NULL;
 long unsigned ppid;
 const char *p;
 
@@ -418,14 +417,9 @@ int get_parent_of_pid(pid_t pid, pid_t *_ppid) {
 }
 
 p = procfs_file_alloca(pid, stat);
-f = fopen(p, re);
-if (!f)
-return -errno;
-
-if (!fgets(line, sizeof(line), f)) {
-r = feof(f) ? -EIO : -errno;
+r = read_one_line_file(p, line);
+if (r  0)
 return r;
-}
 
 /* Let's skip the pid and comm fields. The latter is enclosed
  * in () but does not escape any () in its value, so let's
@@ -452,25 +446,17 @@ int get_parent_of_pid(pid_t pid, pid_t *_ppid) {
 }
 
 int get_starttime_of_pid(pid_t pid, unsigned long long *st) {
-_cleanup_fclose_ FILE *f = NULL;
-char line[LINE_MAX];
+int r;
+_cleanup_free_ char *line = NULL;
 const char *p;
 
 assert(pid = 0);
 assert(st);
 
 p = procfs_file_alloca(pid, stat);
-
-f = fopen(p, re);
-if (!f)
-return errno == ENOENT ? -ESRCH : -errno;
-
-if (!fgets(line, sizeof(line), f)) {
-if (ferror(f))
-return -errno;
-
-return -EIO;
-}
+r = read_one_line_file(p, line);
+if (r  0)
+return r;
 
 /* Let's skip the pid and comm fields. The latter is enclosed
  * in () but does not escape any () in its value, so let's
@@ -2491,21 +2477,17 @@ int getttyname_harder(int fd, char **r) {
 }
 
 int get_ctty_devnr(pid_t pid, dev_t *d) {
-_cleanup_fclose_ FILE *f = NULL;
-char line[LINE_MAX], *p;
+int r;
+_cleanup_free_ char *line = NULL;
+const char *p;
 unsigned long ttynr;
-const char *fn;
 
 assert(pid = 0);
 
-fn = procfs_file_alloca(pid, stat);
-
-f = fopen(fn, re);
-if (!f)
-return -errno;
-
-if (!fgets(line, sizeof(line), f))
-return feof(f) ? -EIO : -errno;
+p = procfs_file_alloca(pid, stat);
+r = read_one_line_file(p, line);
+if (r  0)
+return r;
 
 p = strrchr(line, ')');
 if (!p)
-- 
1.8.5.2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 3/5] shared: util.c: unify split and split_quoted

---
 src/shared/util.c | 88 ---
 src/shared/util.h | 15 ++
 2 files changed, 36 insertions(+), 67 deletions(-)

diff --git a/src/shared/util.c b/src/shared/util.c
index 2350204..db3051d 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -359,8 +359,23 @@ int safe_atod(const char *s, double *ret_d) {
 return 0;
 }
 
+static size_t strcspn_escaped(const char *s, const char *reject) {
+bool escaped = false;
+size_t n;
+
+for (n=0; s[n]; n++) {
+if (escaped)
+escaped = false;
+else if (s[n] == '\\')
+escaped = true;
+else if (strchr(reject, s[n]))
+return n;
+}
+return n;
+}
+
 /* Split a string into words. */
-char *split(const char *c, size_t *l, const char *separator, char **state) {
+char *split(const char *c, size_t *l, const char *separator, bool quoted, char 
**state) {
 char *current;
 
 current = *state ? *state : (char*) c;
@@ -369,70 +384,19 @@ char *split(const char *c, size_t *l, const char 
*separator, char **state) {
 return NULL;
 
 current += strspn(current, separator);
-*l = strcspn(current, separator);
-*state = current+*l;
-
-return (char*) current;
-}
-
-/* Split a string into words, but consider strings enclosed in '' and
- *  as words even if they include spaces. */
-char *split_quoted(const char *c, size_t *l, char **state) {
-const char *current, *e;
-bool escaped = false;
-
-assert(c);
-assert(l);
-assert(state);
-
-current = *state ? *state : c;
-
-current += strspn(current, WHITESPACE);
-
-if (*current == 0)
+if (!*current)
 return NULL;
 
-else if (*current == '\'') {
-current ++;
-
-for (e = current; *e; e++) {
-if (escaped)
-escaped = false;
-else if (*e == '\\')
-escaped = true;
-else if (*e == '\'')
-break;
-}
-
-*l = e-current;
-*state = (char*) (*e == 0 ? e : e+1);
-
-} else if (*current == '\') {
-current ++;
-
-for (e = current; *e; e++) {
-if (escaped)
-escaped = false;
-else if (*e == '\\')
-escaped = true;
-else if (*e == '\')
-break;
-}
-
-*l = e-current;
-*state = (char*) (*e == 0 ? e : e+1);
-
+if (quoted  strchr(\'\, *current)) {
+char quotechar = *(current++);
+*l = strcspn_escaped(current, (char[]){quotechar, '\0'});
+*state = current+*l+1;
+} else if (quoted) {
+*l = strcspn_escaped(current, separator);
+*state = current+*l;
 } else {
-for (e = current; *e; e++) {
-if (escaped)
-escaped = false;
-else if (*e == '\\')
-escaped = true;
-else if (strchr(WHITESPACE, *e))
-break;
-}
-*l = e-current;
-*state = (char*) e;
+*l = strcspn(current, separator);
+*state = current+*l;
 }
 
 return (char*) current;
diff --git a/src/shared/util.h b/src/shared/util.h
index 311315d..92a8a31 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -182,17 +182,22 @@ static inline int safe_atoi64(const char *s, int64_t 
*ret_i) {
 return safe_atolli(s, (long long int*) ret_i);
 }
 
-char *split(const char *c, size_t *l, const char *separator, char **state);
-char *split_quoted(const char *c, size_t *l, char **state);
+char *split(const char *c, size_t *l, const char *separator, bool quoted, char 
**state);
 
 #define FOREACH_WORD(word, length, s, state)\
-for ((state) = NULL, (word) = split((s), (length), WHITESPACE, 
(state)); (word); (word) = split((s), (length), WHITESPACE, (state)))
+_FOREACH_WORD(word, length, s, WHITESPACE, false, state)
 
 #define FOREACH_WORD_SEPARATOR(word, length, s, separator, state)   \
-for ((state) = NULL, (word) = split((s), (length), (separator), 
(state)); (word); (word) = split((s), (length), (separator), (state)))
+_FOREACH_WORD(word, length, s, separator, false, state)
 
 #define FOREACH_WORD_QUOTED(word, length, s, state) \
-for ((state) = NULL, (word) = split_quoted((s), (length),

Re: [systemd-devel] [RFC] networkd/udev: match on 'ethernet' devices

On Sat, Jan 4, 2014 at 2:00 AM, Greg KH gre...@linuxfoundation.org wrote:
 On Sat, Jan 04, 2014 at 01:44:08AM +0100, Tom Gundersen wrote:
 On Sat, Jan 4, 2014 at 1:22 AM, Greg KH gre...@linuxfoundation.org wrote:
  On Fri, Jan 03, 2014 at 08:54:17PM +0100, Tom Gundersen wrote:
  Hi,
 
  I just pushed a change[0] which allows the match syntax
  Type=ethernet to match on network devices without a DEVTYPE.
 
  We had a discussion on IRC whether we should call it Type=wired or
  Type=ethernet. I think the former may be more intuitive, but the
  latter seems to be more in line with what is done elsewhere (connman
  calls it ethernet, and udev prefixes the devices names with 'en').
 
  Any thoughts?
 
  Any reason why the kernel can't be setting this value in the first place
  so you don't have to rely on it being null?
 
  Seems like a kernel issue.

 I asked Marcel the same thing earlier. He said:

 [Friday 03 January 2014] [19:58:22] holtmann  Because you have to
 touch every single driver to get this done properly.
 [Friday 03 January 2014] [19:58:34] holtmann  So DEVTYPE= means it
 is wired Ethernet.
 [Friday 03 January 2014] [19:58:42] holtmann  If that is not true,
 then that is a bug.

 From my point of view, I'd prefer the kernel just doing the right
 thing (and I'd be happy to write the patches), but my impression was
 that that's not going to happen...(?)

 It shouldn't need to be set for every driver, wireless doesn't work this
 way, so there is lots of precident for how to do this in only a few
 lines of kernel code :)

 Also, if we start setting DEVTYPE=ethernet in the kernel, I guess apps
 (such as connman) that relies on DEVTYPE=(null) to detect ethernet
 devices will break.

 Ah, that's a different problem, we can't break userspace.

 Ugh, well, if you want me to try to make this change, I will.

If you see a way to make it happen, that'd be great (and I can revert
this patch).

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [RFC] networkd/udev: match on 'ethernet' devices

2014-01-03 Thread Marcel Holtmann

Hi Tom,

 I just pushed a change[0] which allows the match syntax
 Type=ethernet to match on network devices without a DEVTYPE.
 
 We had a discussion on IRC whether we should call it Type=wired or
 Type=ethernet. I think the former may be more intuitive, but the
 latter seems to be more in line with what is done elsewhere (connman
 calls it ethernet, and udev prefixes the devices names with 'en').
 
 Any thoughts?
 
 Any reason why the kernel can't be setting this value in the first place
 so you don't have to rely on it being null?
 
 Seems like a kernel issue.
 
 I asked Marcel the same thing earlier. He said:
 
 [Friday 03 January 2014] [19:58:22] holtmann  Because you have to
 touch every single driver to get this done properly.
 [Friday 03 January 2014] [19:58:34] holtmann  So DEVTYPE= means it
 is wired Ethernet.
 [Friday 03 January 2014] [19:58:42] holtmann  If that is not true,
 then that is a bug.
 
 From my point of view, I'd prefer the kernel just doing the right
 thing (and I'd be happy to write the patches), but my impression was
 that that's not going to happen...(?)
 
 It shouldn't need to be set for every driver, wireless doesn't work this
 way, so there is lots of precident for how to do this in only a few
 lines of kernel code :)
 
 Also, if we start setting DEVTYPE=ethernet in the kernel, I guess apps
 (such as connman) that relies on DEVTYPE=(null) to detect ethernet
 devices will break.
 
 Ah, that's a different problem, we can't break userspace.
 
 Ugh, well, if you want me to try to make this change, I will.
 
 If you see a way to make it happen, that'd be great (and I can revert
 this patch).

we can easily update ConnMan to handle DEVTYPE= and DEVTYPE=ethernet. That is 
an easy change and will keep things working. As I explained in the other reply, 
the main reason for DEVTYPE=something is to detect that it is a network 
device that needs a management entity before it becomes useful.

You also need to check the ARPHRD_ type of an interface to make sure it is 
actually Ethernet and not just rely on what DEVTYPE= is telling you. We are 
having the case with Bluetooth right now that you get DEVTYPE=bluetooth, but 
one of them is Ethernet emulation via BNEP and the other raw IP via 6loWPAN.

Regards

Marcel

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [RFC] networkd/udev: match on 'ethernet' devices