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

[Marcel Holtmann]

Hi Tom,

>> 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= 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.
> 
> So the usecase I see for the DEVTYPE is quite different. I'm simply
> interested in using it to let the administrator classify devices in an
> intuitive and predictable way, so in this case (I think) we actually
> do want to use Type=bluetooth for all bluetooth devices, regardless of
> the actual protocols they speak. Also, having DEVTYPE=ethernet set by
> the kernel becomes quite useful as there is no 'magic' going on, and
> the admin can simply read 'udevadm info' to know whether or not a
> given device will match.

I am all for doing a DEVTYPE=ethernet. It was just not that simple when I 
introduce DEVTYPE for network interfaces. Maybe things have also changed now.

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

[Tom Gundersen]

On Sat, Jan 4, 2014 at 3:15 AM, Marcel Holtmann  wrote:
> 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= 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.

So the usecase I see for the DEVTYPE is quite different. I'm simply
interested in using it to let the administrator classify devices in an
intuitive and predictable way, so in this case (I think) we actually
do want to use Type=bluetooth for all bluetooth devices, regardless of
the actual protocols they speak. Also, having DEVTYPE=ethernet set by
the kernel becomes quite useful as there is no 'magic' going on, and
the admin can simply read 'udevadm info' to know whether or not a
given device will match.

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

[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]   Because you have to
>>> touch every single driver to get this done properly.
>>> [Friday 03 January 2014] [19:58:34]   So DEVTYPE= means it
>>> is wired Ethernet.
>>> [Friday 03 January 2014] [19:58:42]   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= 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

[Tom Gundersen]

On Sat, Jan 4, 2014 at 2:00 AM, Greg KH  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  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]   Because you have to
>> touch every single driver to get this done properly.
>> [Friday 03 January 2014] [19:58:34]   So DEVTYPE= means it
>> is wired Ethernet.
>> [Friday 03 January 2014] [19:58:42]   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

[Marcel Holtmann]

Hi Greg,

 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]   Because you have to
>> touch every single driver to get this done properly.
>> [Friday 03 January 2014] [19:58:34]   So DEVTYPE= means it
>> is wired Ethernet.
>> [Friday 03 January 2014] [19:58:42]   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 :)

when I did the initial DEVTYPE support, it was easy for “wlan” since the 
subsystem is taking care of creating the netdev. So you have one places for 
mac80211 drivers and some extra work for the cfg80211 only drivers. It was all 
minimal work. So was “bluetooth” since it is one central place and same applied 
to “wimax”. The “wwan” was lucky part since the drivers already did the 
FLAG_WWAN to pick the wwan%d ifname.

For Ethernet cards it was just the wild west. So I left them out of the 
picture. It is also more important to detect the devices that need extra 
management handling. Like WiFi, Bluetooth, 3G/LTE etc. The indication of 
DEVTYPE here is mainly that nobody goes and tries to run DHCP until another 
management entity tell you the device is ready. For Ethernet or DEVTYPE=“” you 
do not need to know who manages it since there is nobody.

Regards

Marcel

___
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

[Simon Peeters]

---
 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), 

[systemd-devel] [PATCH 2/5] util: CACHED_METHOD macro

[Simon Peeters]

the CACHED_METHOD macro is used to cache the return value of a method.

example:

CACHED_METHOD(bool, a_cached_method) {
return some_dificult_and_slow_stuff();
}
---
 src/shared/apparmor-util.c | 16 -
 src/shared/ima-util.c  | 11 +++--
 src/shared/macro.h | 16 +
 src/shared/smack-util.c| 15 +---
 src/shared/util.c  | 60 +-
 5 files changed, 46 insertions(+), 72 deletions(-)

diff --git a/src/shared/apparmor-util.c b/src/shared/apparmor-util.c
index 2b85da1..805d624 100644
--- a/src/shared/apparmor-util.c
+++ b/src/shared/apparmor-util.c
@@ -25,17 +25,9 @@
 #include "fileio.h"
 #include "apparmor-util.h"
 
-static int use_apparmor_cached = -1;
+CACHED_METHOD(bool, use_apparmor) {
+_cleanup_free_ char *p = NULL;
 
-bool use_apparmor(void) {
-
-if (use_apparmor_cached < 0) {
-_cleanup_free_ char *p = NULL;
-
-use_apparmor_cached =
-
read_one_line_file("/sys/module/apparmor/parameters/enabled", &p) >= 0 &&
-parse_boolean(p) > 0;
-}
-
-return use_apparmor_cached;
+return read_one_line_file("/sys/module/apparmor/parameters/enabled", 
&p) >= 0 &&
+   parse_boolean(p) > 0;
 }
diff --git a/src/shared/ima-util.c b/src/shared/ima-util.c
index 6c1954b..7f78311 100644
--- a/src/shared/ima-util.c
+++ b/src/shared/ima-util.c
@@ -22,13 +22,8 @@
 #include 
 
 #include "ima-util.h"
+#include "macro.h"
 
-static int use_ima_cached = -1;
-
-bool use_ima(void) {
-
-if (use_ima_cached < 0)
-use_ima_cached = access("/sys/kernel/security/ima/", F_OK) >= 
0;
-
-return use_ima_cached;
+CACHED_METHOD(bool, use_ima) {
+return access("/sys/kernel/security/ima/", F_OK) >=0;
 }
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 79bee03..e0c83d4 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -323,4 +323,20 @@ do {   
 \
 #endif
 #endif
 
+#define CACHED_METHOD(rettype, name) \
+_CACHED_METHOD(rettype, name, )
+#define CACHED_METHOD_THREAD(rettype, name) \
+_CACHED_METHOD(rettype, name, thread_local)
+#define _CACHED_METHOD(rettype, name, mod) \
+rettype _##name(void); \
+rettype name(void) { \
+static mod bool cached = false; \
+static mod rettype res; \
+if (_unlikely_(cached == false)) \
+res = _##name(); \
+cached = true; \
+return res; \
+} \
+rettype _##name(void)
+
 #include "log.h"
diff --git a/src/shared/smack-util.c b/src/shared/smack-util.c
index df194e0..5d3a0d8 100644
--- a/src/shared/smack-util.c
+++ b/src/shared/smack-util.c
@@ -27,21 +27,18 @@
 #include 
 #endif
 
+#include "macro.h"
 #include "smack-util.h"
 
-bool use_smack(void) {
 #ifdef HAVE_SMACK
-static int use_smack_cached = -1;
-
-if (use_smack_cached < 0)
-use_smack_cached = access("/sys/fs/smackfs/", F_OK) >= 0;
-
-return use_smack_cached;
+CACHED_METHOD(bool,use_smack) {
+return access("/sys/fs/smackfs/", F_OK) >= 0;
+}
 #else
+bool use_smack(void) {
 return false;
-#endif
-
 }
+#endif
 
 int smack_label_path(const char *path, const char *label) {
 #ifdef HAVE_SMACK
diff --git a/src/shared/util.c b/src/shared/util.c
index 50dac70..2350204 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -3198,13 +3198,8 @@ void columns_lines_cache_reset(int signum) {
 cached_lines = 0;
 }
 
-bool on_tty(void) {
-static int cached_on_tty = -1;
-
-if (_unlikely_(cached_on_tty < 0))
-cached_on_tty = isatty(STDOUT_FILENO) > 0;
-
-return cached_on_tty;
+CACHED_METHOD(bool, on_tty) {
+return isatty(STDOUT_FILENO) > 0;
 }
 
 int running_in_chroot(void) {
@@ -4569,13 +4564,8 @@ char *strjoin(const char *x, ...) {
 return r;
 }
 
-bool is_main_thread(void) {
-static thread_local int cached = 0;
-
-if (_unlikely_(cached == 0))
-cached = getpid() == gettid() ? 1 : -1;
-
-return cached > 0;
+CACHED_METHOD_THREAD(bool, is_main_thread) {
+return getpid() == gettid();
 }
 
 int block_get_whole_disk(dev_t d, dev_t *ret) {
@@ -5395,47 +5385,31 @@ void *xbsearch_r(const void *key, const void *base, 
size_t nmemb, size_t size,
 return NULL;
 }
 
-bool is_locale_utf8(void) {
+CACHED_METHOD(bool, is_locale_utf8) {
 const char *set;
-static int cached_answer = -1;
 
-if (cached_answer >= 0)
-goto out;
-
-if (!setlocale(LC_ALL, "")) {
-cached_answer = true;
-goto out;
-}
+if (!setlocale(LC_ALL, ""))
+return true;
 
 set = nl_langinfo(CODESET);
-if (!set) {
-cached_an

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

[Simon Peeters]

---
 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 5/5] strv: multiple cleanups

[Simon Peeters]

- 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

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

[Simon Peeters]

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_

[systemd-devel] various cleanups

[Simon Peeters]

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

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

[Greg KH]

On Sat, Jan 04, 2014 at 01:44:08AM +0100, Tom Gundersen wrote:
> On Sat, Jan 4, 2014 at 1:22 AM, Greg KH  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]   Because you have to
> touch every single driver to get this done properly.
> [Friday 03 January 2014] [19:58:34]   So DEVTYPE= means it
> is wired Ethernet.
> [Friday 03 January 2014] [19:58:42]   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.

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

[Tom Gundersen]

On Sat, Jan 4, 2014 at 1:22 AM, Greg KH  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]   Because you have to
touch every single driver to get this done properly.
[Friday 03 January 2014] [19:58:34]   So DEVTYPE= means it
is wired Ethernet.
[Friday 03 January 2014] [19:58:42]   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

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

[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] [PATCH v2] systemctl: improve readability on failed commands

[Zbigniew Jędrzejewski-Szmek]

On Fri, Jan 03, 2014 at 11:59:08PM +0100, Thomas H.P. Andersen wrote:
> From: Thomas Hindoe Paaboel Andersen 
> 
> 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

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

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen 

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] Multiseat session creation fail, VT number not 0

[Matthew Monaco]

On 01/03/2014 07:51 AM, David Herrmann wrote:
> Hi
> 
> On Fri, Jan 3, 2014 at 3:24 PM, Matthew Monaco  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

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

[Kay Sievers]

On Fri, Jan 3, 2014 at 6:21 PM, Zbigniew Jędrzejewski-Szmek
 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] [PATCH] Add SELinuxContext configuration item

[Michael Scherer]

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

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

[Tom Gundersen]

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]: 

___
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

[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] [PATCH 2/2] Update .mailmap file

[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  
 Kay Sievers  
 Kay Sievers  
 Kay Sievers  
+Kay Sievers  
 Greg KH 
 Greg KH  
 Greg KH  
@@ -36,7 +37,7 @@ Michal Soltys  
 Piter PUNK  
 Richard Hughes  
 Robby Workman  
-Shawn Landden 
+Shawn Landden  
 Simon Peeters 
 Tobias Klauser  
 Miklos Vajna  
@@ -50,6 +51,7 @@ Ted Ts'o 
 Tobias Klauser 
 Tobias Klauser  
 Tobias Klauser  
+Tobias Klauser  
 Patrick Mansfield 
 Christophe Varoqui 
 Daniel Stekloff 
-- 
1.8.5.2.229.g4448466

___
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

[Michael Scherer]

Le vendredi 03 janvier 2014 à 11:48 -0500, Daniel J Walsh a écrit :
> On 01/03/2014 09:16 AM, Michael Scherer wrote:

> Well thinking about this again, I think still to the single label.  Lets not
> break the field up into multiple labels.
> 
> 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.

I fail to follow you. The current code use setexecon, and this is quite
selinux specific. What would be the equivalent for apparmor, for smack
and others ?


-- 
Michael Scherer


signature.asc
Description: This is a digitally signed message part
___
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

[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

Re: [systemd-devel] Apparmor profile switching support

[Michael Scherer]

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

[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 Scherer
>>> 
>>> 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

[systemd-devel] Apparmor profile switching support

[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

[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.

[misc]

From: Michael Scherer 

---
 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 
 
 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] [PATCH 2/2] Add AppArmor profile switching

[misc]

From: Michael Scherer 

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 @@
 
 
 
+
AppArmorProfile=
+
+Take 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.
+
+
+
+
 IgnoreSIGPIPE=
 
 Takes 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 
b/src/core/load-fragment-gperf.gperf.m4
index a5033b2..d5d891e 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/sr

Re: [systemd-devel] [RFC] Initial libsystemd-asyncns commit

[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 

> 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

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

[David Herrmann]

Hi

On Fri, Jan 3, 2014 at 3:24 PM, Matthew Monaco  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

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

[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] [PATCH] Add SELinuxContext configuration item

[Michael Scherer]

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 Scherer
> >
> > 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

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

[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

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

[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 0/2] logind: make sure that closed sessions will be cleaned

[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

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

[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 Scherer

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

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

[Michael Scherer]

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 Scherer
> >> >
> >> >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 4/4] gdbus: Add basic kdbus tests

[Karol Lewandowski]

On 12/20/2013 04:23 PM, Yin Kangkai wrote:

Hi!

Sorry for late response, I've been out of office the last week(s).

> On 2013-11-21, 12:33 +0100, Karol Lewandowski wrote:
>> +TESTS
>> +=
>> +
>> +* Build test binaries:
>> +
>> +  cd gio/tests
>> +  make
>> +
>> +* Set variable to use custom library and to use kdbus as session bus:
>> +
>> +  export LD_LIBRARY_PATH=absolute_path_to_glib_libs_with_kdbus_patch
>> +  export DBUS_SESSION_BUS_ADDRESS=kdbus:path=/dev/kdbus/0-kdbus/bus
>> +
>> +* Run test binary server in terminal 1:
>> +
>> +  ./gdbus-example-kdbus-server
>> +
>> +* Run test binary client in terminal 2:
>> +
>> +  ./gdbus-example-kdbus-client
>> +
>> +* Sample client app output:
>> +
>> +  elapsed time : 0.265072 s
>> +  elapsed time : 0.264353 s
>> +  elapsed time : 0.305062 s
>> +  elapsed time : 0.343710 s
>> +  elapsed time : 0.451501 s
>> +  elapsed time : 1.109851 s
>> +  elapsed time : 8.278217 s
> 
> Will it be more interesting to show some real benchmarking numbers? ;)
> 
>  - results with vanilla GIO;
>  - results with GIO with kdbus transport, talking to kdbus
>dbus-daemon;
>  - results with GIO with kdbus transport, talking through systemd-bus-proxyd
>to systemd-bus-driverd;

I agree this is very much needed.  Currently, due to major changes in
kdbus, we are redoing some of the gio-kdbus stuff again. Lukasz Skalski
will share result in following days.

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

Re: [systemd-devel] [RFT] DHCPv4 support in networkd

[Patrik Flykt]

On Thu, 2014-01-02 at 16:52 +0100, Tom Gundersen wrote:
> On Thu, Jan 2, 2014 at 4:48 PM, Reindl Harald  wrote:
> >
> >
> > Am 02.01.2014 16:41, schrieb Tom Gundersen:
> >> On Thu, Jan 2, 2014 at 4:37 PM, Reindl Harald  
> >> wrote:
> >>> the problems are that if someone comes back with his Apple notebook
> >>> this crap starts to using the old ip-address and triggering all sorts
> >>> of alarms, firewall-rules and so on
> >>
> >> Hm, sounds odd. This protocol is precisely meant to avoid that sort of
> >> problem (by detecting whether or not you are connecting to the same
> >> network). I heard that some old Apple devices used a more naive
> >> protocol that would indeed just reuse the old IP... When did you last
> >> experience this? Any clue about what hardware/software version it was
> >> causing the problem?
> >
> > 2013, OSX 10.6, the first Mac Book Pro generation not supported
> > by OSX > 10.6 as far as i know, one bought a few months later
> > would be supported
> >
> > given that this machines are not that old and expensive they
> > will exist longer here and there (yes i know about the securtiy
> > nightmare but in that context OSX should be banned at all)
> 
> Thanks, I'll try to dig into this a bit before implementing anything
> (and anyway, I expect this to be configurable if we add it).

So from the DHCP point of view, once we'll get around to implement
Init-Reboot, let's not set up the IP address before a positive response
is received from the DHCP server. Unless RFC 4436 will give some other
indications that it indeed is the same network.

Cheers,

Patrik


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

Re: [systemd-devel] [PATCH 00/11] Finalize initial DHCP support

[Patrik Flykt]

On Fri, 2013-12-20 at 09:47 -0800, Marcel Holtmann wrote:
> Hi Tom,
> 
> >>> The first seven patches fix a few issues with the current code.
> >>> 
> >>> Patch 09 adds DHCP lease renewing support when timer T1 triggers.
> Using
> >>> the UDP socket sending implementation in patch 08, the DHCP lease
> >>> renewal is unicasted to the DHCP server. This means that
> systemd-network
> >>> should have applied the acquired IP address and default route to
> the
> >>> proper interface before timer T1 triggers.
> >> 
> >> this could become racy and we might end up in funny cases if the
> lease is really small. I think networkd and the DHCP need some way of
> communicating a) I set the IP you told me and/or b) we have T1
> triggering, have you set the address or should I just redo the DHCP
> process.
> > 
> > Makes sense to me for networkd to call (something like)
> > sd_dhcp_client_address_configured(client, true) whenever it has
> > successfully set the addresses/routes. I.e., I'd go with option (a).
> > Or is there a reason to go with (b) that I'm not seeing?
> 
> the case I see is that T1 is triggered, but the IP address is not set.
> Then of course we should not be setting it anymore since it might not
> stay valid.

Leases with lifetime less than 10 seconds are not accepted by the code
currently. With the default T1 of half lease time, this gives 5 seconds
to react. Now the funny part is that the server can suggest other values
for T1 and T2 that the code will use, so yes, the server can try to
suggest a 1s T1 expiry time.

If the address is not set at the time of T1 expiring, it is treated as a
temprorary error and T1 retry is rescheduled with a minimum time of 60
seconds, this from section 4.4.5 in RFC 2131. Thus both T2 and the whole
lifetime will expire before that with a short lease < 60s. If T2 is
reached, the DHCP Request is broadcasted anyway, so if the address is
not set even at this point, a new DHCP Request is still sent.

The somewhat fuzzy idea was to use T1 from the server if given and to
set the next T1 timeout to the minimum 60 seconds proposed by the RFC if
the previous reacquisition fails.

With this the code should still work nicely with short lease timeouts.
But more testing with really short lease times are anyway appropriate.

Cheers,

Patrik


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