[systemd-devel] [PATCH 2/3] resolved: fix CID 1237549 Unchecked return value
---
src/resolve/resolved-dns-scope.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c
index 1664b13..25e0d9e 100644
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -386,7 +386,7 @@ int dns_scope_llmnr_membership(DnsScope *s, bool b) {
* one. This is necessary on some devices, such as
* veth. */
if (b)
-setsockopt(fd, IPPROTO_IP, IP_DROP_MEMBERSHIP, mreqn,
sizeof(mreqn));
+(void) setsockopt(fd, IPPROTO_IP, IP_DROP_MEMBERSHIP,
mreqn, sizeof(mreqn));
if (setsockopt(fd, IPPROTO_IP, b ? IP_ADD_MEMBERSHIP :
IP_DROP_MEMBERSHIP, mreqn, sizeof(mreqn)) 0)
return -errno;
@@ -402,7 +402,7 @@ int dns_scope_llmnr_membership(DnsScope *s, bool b) {
return fd;
if (b)
-setsockopt(fd, IPPROTO_IPV6, IPV6_DROP_MEMBERSHIP,
mreq, sizeof(mreq));
+(void) setsockopt(fd, IPPROTO_IPV6,
IPV6_DROP_MEMBERSHIP, mreq, sizeof(mreq));
if (setsockopt(fd, IPPROTO_IPV6, b ? IPV6_ADD_MEMBERSHIP :
IPV6_DROP_MEMBERSHIP, mreq, sizeof(mreq)) 0)
return -errno;
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/3] log: 1237557 Unchecked return value from library
fix 1237557 Unchecked return value from library
---
src/shared/log.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/shared/log.c b/src/shared/log.c
index 1c589ac..e7237ba 100644
--- a/src/shared/log.c
+++ b/src/shared/log.c
@@ -122,7 +122,7 @@ static int create_log_socket(int type) {
timeval_store(tv, 10 * USEC_PER_MSEC);
else
timeval_store(tv, 10 * USEC_PER_SEC);
-setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, tv, sizeof(tv));
+(void) setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, tv, sizeof(tv));
return fd;
}
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/3] bus-socket: fix CID 996290 Unchecked return value
---
src/libsystemd/sd-bus/bus-socket.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/libsystemd/sd-bus/bus-socket.c
b/src/libsystemd/sd-bus/bus-socket.c
index d124d9a..662bf1c 100644
--- a/src/libsystemd/sd-bus/bus-socket.c
+++ b/src/libsystemd/sd-bus/bus-socket.c
@@ -610,10 +610,10 @@ void bus_socket_setup(sd_bus *b) {
/* Enable SO_PASSCRED + SO_PASSEC. We try this on any
* socket, just in case. */
enable = !b-bus_client;
-setsockopt(b-input_fd, SOL_SOCKET, SO_PASSCRED, enable,
sizeof(enable));
+(void) setsockopt(b-input_fd, SOL_SOCKET, SO_PASSCRED, enable,
sizeof(enable));
enable = !b-bus_client (b-attach_flags KDBUS_ATTACH_SECLABEL);
-setsockopt(b-input_fd, SOL_SOCKET, SO_PASSSEC, enable,
sizeof(enable));
+(void) setsockopt(b-input_fd, SOL_SOCKET, SO_PASSSEC, enable,
sizeof(enable));
/* Increase the buffers to 8 MB */
fd_inc_rcvbuf(b-input_fd, SNDBUF_SIZE);
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] localed: validate set-x11-keymap input
Lennart Poettering lenn...@poettering.net writes:
On Tue, 04.11.14 12:05, Jan Synacek (jsyna...@redhat.com) wrote:
diff --git a/src/locale/localectl.c b/src/locale/localectl.c
index d4a2d29..8f9e4da 100644
--- a/src/locale/localectl.c
+++ b/src/locale/localectl.c
@@ -46,6 +46,7 @@
#include virt.h
#include fileio.h
#include locale-util.h
+#include xkb-util.h
static bool arg_no_pager = false;
static bool arg_ask_password = true;
@@ -389,14 +390,7 @@ static int set_x11_keymap(sd_bus *bus, char **args,
unsigned n) {
static int list_x11_keymaps(sd_bus *bus, char **args, unsigned n) {
_cleanup_fclose_ FILE *f = NULL;
_cleanup_strv_free_ char **list = NULL;
-char line[LINE_MAX];
-enum {
-NONE,
-MODELS,
-LAYOUTS,
-VARIANTS,
-OPTIONS
-} state = NONE, look_for;
+enum keymap_state look_for;
While we don#t follow this rule too strictly we usually define typdefs
for enums and name them in CamelCase. Hence, please rename this type
KeymapState instead of enum keymap_state.
That said, is state really the right name here? Shouldn't it be
field or component or item or so?
!streq_ptr(variant, c-x11_variant) ||
!streq_ptr(options, c-x11_options)) {
+_cleanup_free_ char *msg = NULL;
if ((layout !string_is_safe(layout)) ||
(model !string_is_safe(model)) ||
@@ -1050,6 +1052,12 @@ static int method_set_x11_keyboard(sd_bus *bus,
sd_bus_message *m, void *userdat
free_and_strdup(c-x11_options, options) 0)
return -ENOMEM;
+r = xkb_validate_keymaps(model, layout, variant, options,
msg);
+if (r 0) {
+log_error(Failed to validate X11 keyboard layout:
%s, strerror(-r));
+return sd_bus_error_set(error, SD_BUS_ERROR_FAILED,
msg);
+}
+
Please return the error back to the client instead of logging it
away. Use sd_bus_error_setf() for that.
The errno part of the error is logged and the digestible by the user
part is returned in the error message. I did this in exactly the same
way as it was already there a few lines below (x11_write_data()
call). Are you sure I should change it? If yes, both log_error()s should
probably be changed.
Use SD_BUS_ERROR_INVALID_ARGS as error id.
+
+static char **xkb_parse_argument(const char *arg)
+{
+_cleanup_free_ char *input;
+char *token;
+char **result = NULL;
+int r;
+
+assert(arg);
+
+input = strdup(arg);
+if (!input)
+return NULL;
+
+token = strsep(input, ,);
+while(token) {
+r = strv_extend(result, token);
+if (r 0)
+return NULL;
+token = strsep(input, ,);
+}
+
+return result;
+}
Please place the opening { of a function body on the same line as the
function declaration.
Oops, old habit, sorry, will fix.
I figure strv_split() does exactly the same thing, please use that.
That's what I had originally used. The problem is that it throws away
the empty parts, which is what I don't want. I need it to return the
empty strings after splitting as well. That's why I wrote my own that
uses strsep(). Since I figured it's probably not needed anywhere else, I
wrote it locally instead of introducing a new shared strv_something
function.
+int xkb_get_keymaps(char ***list, enum keymap_state look_for, const char
*layout_prefix)
+{
+_cleanup_fclose_ FILE *f;
+char line[LINE_MAX];
+enum keymap_state state = NONE;
+int r;
+
+f = fopen(/usr/share/X11/xkb/rules/base.lst, re);
+if (!f) {
+log_error(Failed to open keyboard mapping list. %m);
+return -errno;
+}
This should probably become a function call that returns errors but
doesn't log about them, leaving that to the caller.
Again, are you sure? The logged error is very local to what the code is
trying to do.
+int xkb_validate_keymaps(const char *model,
+ const char *layouts_arg,
+ const char *variants_arg,
+ const char *options_arg,
+ char **error)
+{
+int r;
+
+if (layouts_arg) {
+_cleanup_strv_free_ char **layouts_list = NULL;
+char **it, **layouts;
+int i = 0;
+
+r = xkb_get_keymaps(layouts_list, LAYOUTS, NULL);
+if (r 0)
+return r;
+
+layouts = strv_split(layouts_arg, ,);
+if (!layouts)
+return log_oom();
+
+STRV_FOREACH(it,
[systemd-devel] [PATCH 1/2] namespace:Unchecked return value from library
fix:
CID 1237553 (#1 of 6): Unchecked return value from library
(CHECKED_RETURN
CID 1237553 (#3 of 6): Unchecked return value from library
(CHECKED_RETURN)
CID 1237553 (#4 of 6): Unchecked return value from library
(CHECKED_RETURN)
CID 1237553 (#5 of 6): Unchecked return value from library
(CHECKED_RETURN
CID 1237553 (#6 of 6): Unchecked return value from library
(CHECKED_RETURN)
---
src/core/namespace.c | 44 +---
1 file changed, 37 insertions(+), 7 deletions(-)
diff --git a/src/core/namespace.c b/src/core/namespace.c
index 4bc288d..94a8088 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -157,14 +157,24 @@ static int mount_dev(BindMount *m) {
return -errno;
dev = strappenda(temporary_mount, /dev);
-mkdir(dev, 0755);
+r = mkdir(dev, 0755);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
if (mount(tmpfs, dev, tmpfs, MS_NOSUID|MS_STRICTATIME, mode=755)
0) {
r = -errno;
goto fail;
}
devpts = strappenda(temporary_mount, /dev/pts);
-mkdir(devpts, 0755);
+r = mkdir(devpts, 0755);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
if (mount(/dev/pts, devpts, NULL, MS_BIND, NULL) 0) {
r = -errno;
goto fail;
@@ -174,7 +184,7 @@ static int mount_dev(BindMount *m) {
symlink(pts/ptmx, devptmx);
devshm = strappenda(temporary_mount, /dev/shm);
-mkdir(devshm, 01777);
+r = mkdir(devshm, 01777);
r = mount(/dev/shm, devshm, NULL, MS_BIND, NULL);
if (r 0) {
r = -errno;
@@ -182,15 +192,30 @@ static int mount_dev(BindMount *m) {
}
devmqueue = strappenda(temporary_mount, /dev/mqueue);
-mkdir(devmqueue, 0755);
+r = mkdir(devmqueue, 0755);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
mount(/dev/mqueue, devmqueue, NULL, MS_BIND, NULL);
devkdbus = strappenda(temporary_mount, /dev/kdbus);
-mkdir(devkdbus, 0755);
+r = mkdir(devkdbus, 0755);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
mount(/dev/kdbus, devkdbus, NULL, MS_BIND, NULL);
devhugepages = strappenda(temporary_mount, /dev/hugepages);
-mkdir(devhugepages, 0755);
+r = mkdir(devhugepages, 0755);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
mount(/dev/hugepages, devhugepages, NULL, MS_BIND, NULL);
devlog = strappenda(temporary_mount, /dev/log);
@@ -289,7 +314,12 @@ static int mount_kdbus(BindMount *m) {
}
root = strappenda(temporary_mount, /kdbus);
-mkdir(root, 0755);
+r = mkdir(root, 0755);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
if (mount(tmpfs, root, tmpfs, MS_NOSUID|MS_STRICTATIME,
mode=777) 0) {
r = -errno;
goto fail;
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261
Unchecked return value from library
---
src/tty-ask-password-agent/tty-ask-password-agent.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c
b/src/tty-ask-password-agent/tty-ask-password-agent.c
index e6dc84b..c4cd387 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -376,7 +376,9 @@ static int wall_tty_block(void) {
return -ENOMEM;
mkdir_parents_label(p, 0700);
-mkfifo(p, 0600);
+r = mkfifo(p, 0600);
+if (r 0)
+return -errno;
fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
if (fd 0)
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] localed: validate set-x11-keymap input
Jan Synacek jsyna...@redhat.com writes:
Lennart Poettering lenn...@poettering.net writes:
On Tue, 04.11.14 12:05, Jan Synacek (jsyna...@redhat.com) wrote:
+int xkb_validate_keymaps(const char *model,
+ const char *layouts_arg,
+ const char *variants_arg,
+ const char *options_arg,
+ char **error)
+{
+int r;
+
+if (layouts_arg) {
+_cleanup_strv_free_ char **layouts_list = NULL;
+char **it, **layouts;
+int i = 0;
+
+r = xkb_get_keymaps(layouts_list, LAYOUTS, NULL);
+if (r 0)
+return r;
+
+layouts = strv_split(layouts_arg, ,);
+if (!layouts)
+return log_oom();
+
+STRV_FOREACH(it, layouts) {
+_cleanup_strv_free_ char **variants_list = NULL;
+bool variants_left = true;
+int n;
+
+if (!strv_find(layouts_list, *it)) {
+r = asprintf(error, Requested layout '%s'
not available.\n, *it);
+if (r 0)
+return log_oom();
+return -EINVAL;
+}
+
+if (variants_left variants_arg) {
+_cleanup_strv_free_ char **variants;
+
+r = xkb_get_keymaps(variants_list,
VARIANTS, *it);
+if (r 0)
+return r;
Hmm, reading the file over and over and over again sounds less than
ideal. Maybe we should intrdouce a struct here and then make
xkb_get_keymaps() return an array of structs really?
That sounds ok, I'll see what I can do. I wanted to preserve as much of
the original code as I could, but maybe it wasn't the right decision.
After thinking about it, I can put the layout and its variants into a
structure, but... When parsing /usr/share/X11/xkb/rules/base.lst, is
it safe to depend on the ordering of all the components in the file? I
mean, is it safe to expect layouts being order before variants in that
file? Or can it be vice versa? The code would have to be more general if
one cannot depend on that order, and I think it would be quite ugly. The
strv implementation doesn't have that problem.
--
Jan Synacek
Software Engineer, Red Hat
signature.asc
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] localed: validate set-x11-keymap input
Lennart Poettering lenn...@poettering.net writes:
On Tue, 04.11.14 12:05, Jan Synacek (jsyna...@redhat.com) wrote:
One more addition:
+}
+
+int xkb_get_keymaps(char ***list, enum keymap_state look_for, const char
*layout_prefix)
+{
+_cleanup_fclose_ FILE *f;
+char line[LINE_MAX];
+enum keymap_state state = NONE;
+int r;
+
+f = fopen(/usr/share/X11/xkb/rules/base.lst, re);
+if (!f) {
+log_error(Failed to open keyboard mapping list. %m);
+return -errno;
+}
+
+FOREACH_LINE(line, f, break) {
+char *l, *w;
+
+l = strstrip(line);
+
+if (isempty(l))
+continue;
+
+if (l[0] == '!') {
+if (startswith(l, ! model))
+state = MODELS;
+else if (startswith(l, ! layout))
+state = LAYOUTS;
+else if (startswith(l, ! variant))
+state = VARIANTS;
+else if (startswith(l, ! option))
+state = OPTIONS;
+else
+state = NONE;
+
+continue;
+}
+
+if (state != look_for)
+continue;
+
+w = l + strcspn(l, WHITESPACE);
+
+if (layout_prefix) {
+char *e;
+
+if (*w == 0)
+continue;
+
+*w = 0;
+w++;
+w += strspn(w, WHITESPACE);
+
+e = strchr(w, ':');
+if (!e)
+continue;
+
+*e = 0;
+
+if (!streq(w, layout_prefix))
+continue;
+} else
+*w = 0;
+
+r = strv_extend(list, l);
+if (r 0)
+return log_oom();
I think, while we are at it, this should really be reworked to use
GREEDY_REALLOC. See strv_split_quoted() for an example.
Could you please explain why? str_extend() uses realloc_multiply()
inside, which, to me, seems to be ok for this case.
+}
+
+if (strv_isempty(*list)) {
+log_error(Couldn't find any entries.); /* TODO: improve
error message */
+return -ENOENT;
+}
+
+return 0;
Lennart
--
Lennart Poettering, Red Hat
--
Jan Synacek
Software Engineer, Red Hat
signature.asc
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] localed: validate set-x11-keymap input
On Tue, 11.11.14 13:34, Jan Synacek (jsyna...@redhat.com) wrote: +r = strv_extend(list, l); +if (r 0) +return log_oom(); I think, while we are at it, this should really be reworked to use GREEDY_REALLOC. See strv_split_quoted() for an example. Could you please explain why? str_extend() uses realloc_multiply() inside, which, to me, seems to be ok for this case. strv_extend() increases the size of the strv one-by-one. GREEDY_REALLOC grows the array exponentially. Given that you use strv_extend() here to add quite a number of entries to the array it is hence a good idea to make use of GREEDY_REALLOC in favour of strv_extend(). strv_extend() is really something to avoid unless you know the number of entries stays low. If the strv might grow to more than a handful of entries it's a wise idea to use GREEDY_REALLOC instead. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] syntax error in master
See:
http://build.gnome.org/continuous/buildmaster/builds/2014/11/11/23/build/log-systemd.txt
Perhaps something like this?
diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index c5aeaac..afee131 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -1466,11 +1466,11 @@ int main(int argc, char *argv[]) {
if (k == -ECONNRESET)
r = 0;
else {
-
-k = process_policy(a, b, m, p,
ucred);
-if (k 0) {
-r = k;
-log_error(Failed to
send message: %s, strerror(-r));
+k = process_policy(a,
b, m, p, ucred);
+if (k 0) {
+r = k;
+
log_error(Failed to send message: %s, strerror(-r));
+}
}
goto finish;
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] syntax error in master
On 11/11/2014 03:31 PM, Colin Walters wrote:
See:
http://build.gnome.org/continuous/buildmaster/builds/2014/11/11/23/build/log-systemd.txt
Perhaps something like this?
My bad, sorry. I had a hunk confusion due to rebasing.
It's already reverted in master.
Daniel
diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index c5aeaac..afee131 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -1466,11 +1466,11 @@ int main(int argc, char *argv[]) {
if (k == -ECONNRESET)
r = 0;
else {
-
-k = process_policy(a, b, m,
p, ucred);
-if (k 0) {
-r = k;
-log_error(Failed to
send message: %s, strerror(-r));
+k =
process_policy(a, b, m, p, ucred);
+if (k 0) {
+r = k;
+
log_error(Failed to send message: %s, strerror(-r));
+}
}
goto finish;
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Detecting inactive sessions
On Wed, 29.10.14 15:45, Bastien Nocera (had...@hadess.net) wrote: For a very specific definition of inactive. I'm looking at a way for the iio-sensor-proxy at: https://github.com/hadess/iio-sensor-proxy to suspend reading from accelerometers (or maybe to turn them off), when all the sessions are locked and the screens turned off. This would usually mean that I would enable reading from the sensor if one session is active and stop reading if none are active. Is this correct? Is it up to the session manager (eg. gnome-session) to tell us whether a session is active or not, or do I have this backwards? logind knows when sessions are active and not. Note that access control to such devices should really not be per-session, but per-user. Meaning that a user should get access to the device as long as he has at least one session active. That said, I am not sure I really grok what iio-sensor-proxy is doing, and whether doing it involving both uinput and uevents is really such a great idea. I am tempted to say that we should probably add support for the orientation sensors to logind, and abstract them away in logind so that only simple high-level rotation change events are sent out. I am pretty sure that orientation is something pretty much all desktop environments really want to know about, and as logind is really a system service these days that only exists for the purpose of making writing of desktop environments easy I think adding the orientation stuff to logind wouldn't be too far off. And I figure we need it for the userspace console too in one way or another... Orientation is pretty much a property of a seat anyway, and I figure it should be exposed as one, too. Also, we really should figure out a logic where the desktop subscribes to orientation changes and we only in that case even do the IIO access, rather than pushing the IIO events up the stack even when nobody is listening. I am willing to take a patch for this, but then again, as I own a Yoga I might look into this myself too one day. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd and power management
On Wed, 29.10.14 13:00, Daniel Hollocher (danielholloc...@gmail.com) wrote: Hey folks, I'm a not expert here, so please forgive the low quality/interest of my question. I'm curious what the ideal systemd way is to set various power management settings in the /sys tree. For me personally, I'm looking to set sampling_down_factor as without it, ondemand has terrible performance on my particular computer (a 10-30% loss compared to performance or conservative). Currently, Ubuntu uses a sysv init script to set ondemand after boot, and I could edit that. It would be cool to know the ideal systemd way, that could also be aware of power saving stuff. From googling, it seems that tempfiles or sysctrl is not the way to go, since those only happen at boot. Udev? The examples I've found seem to make basic usage of udev to detect power changes, and then drop to a script to do the bulk of the work. Is that it? Yes, sysctl.d and tmpfiles.d (with the f option) are the way we propose this is done. Note that in general we try to follow the rule that what is good for mobile use cannot hurt in plugged-in mode either, hence we have no special support for reconfiguring the system depending on plug state. systemd does not listen to power plug events. upower does however, hence you could check if you can hook things into that. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemctl show environment quoting
On Wed, 29.10.14 09:40, Alexandre Detiste (alexandre.deti...@gmail.com) wrote: Hi, I stumbled on this: $ systemctl cat cron-crontab-pi-0 | grep Environment Environment=A=a a MAILTO=system-c...@mailinator.com B=b b C=c c $ systemctl show cron-crontab-pi-0 -p Environment Environment=A=a a MAILTO=system-c...@mailinator.com B=b b C=c c - the quotes are gone. Is this done by design, or a bug in systemctl show ? My simple parser could be abused if someone hid a MAILTO= inside an other env variable. https://github.com/systemd-cron/systemd-cron/blob/master/src/bin/mail_on_failure Here this won't hurt, but this may causes security problems elsewhere. I made some minimal changes to git now: http://cgit.freedesktop.org/systemd/systemd/commit/?id=27e9c5af817147ea1c678769e45e83f2e4b4ae96 This tries to improve things a bit, but I figure it might break stuff for you. So previously we'd just output the strings as is, separated by spaces, and suffixed by a single newline. If the string contained spaces this would create an ambiguigity when trying to parse this. Now, completely changing the ouput by enclosing everything in and escaping the to \ inside the strings appears wrong to me, since it's a major compat break. I hence went the other way, and will now escape spaces and newlines inside the strings to the usualy C \x012 syntax. This means spaces now become \x020. This makes the output reversible, but of course looks awful if env vars really contain spaces... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] 'systemctl poweroff' no longer shuts down system -- instead, reboots ?
On Mon, 10.11.14 16:22, grantksupp...@operamail.com (grantksupp...@operamail.com) wrote: On Mon, Nov 10, 2014, at 04:07 PM, Lennart Poettering wrote: On Thu, 30.10.14 12:30, grantksupp...@operamail.com (grantksupp...@operamail.com) wrote: exec of systemctl poweroff causes, as reported, a reboot/restart but, exec of systemctl --force poweroff otoh, does shut down the machine, though not gracefully This got recently fixed in git. Please verify. I'm a bit confused. In #systemd, I'd been told that this was not relevant to my specific issue, and that the problem was solely with my distro. So, here's where I've been discussing it: Bug 903560 - opensuse 13.1 ( + Kernel:Stable 3.17.2 + systemd 210) exec of `shutdown` restarts machine instead https://bugzilla.suse.com/show_bug.cgi?id=903560#c40 Is the fixed-in-git you are referencing relevant to the issue with systemd discussed at opensuse? Oh yeah, 210 is too old. On the upstream ML we usually discuss only more recent problems, which are exposed upstream. Hence, please contact the Suse folks for more help on the issue, or check if a current systemd version fails. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd-cron: retrigger generator after /var is mounted
On Thu, 30.10.14 00:27, Alexandre Detiste (alexandre.deti...@gmail.com) wrote: Is systemctl daemon-reload really synchronous, or does it return before the reload if effectively done ? (I saw the --no-block argument that make me fear this) It is synchronous. --no-block mostly applies to start and stop and suchlike, i.e. operations that involve jobs, which daemon-reload really doesn't. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] localed: validate set-x11-keymap input
On Tue, 11.11.14 10:03, Jan Synacek (jsyna...@redhat.com) wrote:
+r = xkb_validate_keymaps(model, layout, variant, options,
msg);
+if (r 0) {
+log_error(Failed to validate X11 keyboard
layout: %s, strerror(-r));
+return sd_bus_error_set(error,
SD_BUS_ERROR_FAILED, msg);
+}
+
Please return the error back to the client instead of logging it
away. Use sd_bus_error_setf() for that.
The errno part of the error is logged and the digestible by the user
part is returned in the error message. I did this in exactly the same
way as it was already there a few lines below (x11_write_data()
call). Are you sure I should change it? If yes, both log_error()s should
probably be changed.
Validation errors with stuff the caller passed in should be pased back
to the caller. System errors that the caller is not really responsible
for should end up in the logs. If we cannot write files into /etc
(because read-only or full or whatever) then this is not the caller's
fault, hence this should be logged. But if he passes complete rubbish
data, then it is.
I figure strv_split() does exactly the same thing, please use that.
That's what I had originally used. The problem is that it throws away
the empty parts, which is what I don't want. I need it to return the
empty strings after splitting as well. That's why I wrote my own that
uses strsep(). Since I figured it's probably not needed anywhere else, I
wrote it locally instead of introducing a new shared strv_something
function.
Hmm, I see.
Maybe we should extend strv_split() with a bool (or flags field) that
allows a slight alteration of the allgorithm so that it does not
collapse multiple separators into one.
+int xkb_get_keymaps(char ***list, enum keymap_state look_for, const char
*layout_prefix)
+{
+_cleanup_fclose_ FILE *f;
+char line[LINE_MAX];
+enum keymap_state state = NONE;
+int r;
+
+f = fopen(/usr/share/X11/xkb/rules/base.lst, re);
+if (!f) {
+log_error(Failed to open keyboard mapping list. %m);
+return -errno;
+}
This should probably become a function call that returns errors but
doesn't log about them, leaving that to the caller.
Again, are you sure? The logged error is very local to what the code is
trying to do.
Well, while this is not strictly followed the calls in share/*.c area
usually of the non-logging kind...
But this doesn't matter, it's OK if you leave it like it is right now
in your patch.
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] localed: validate set-x11-keymap input
On Tue, 11.11.14 13:29, Jan Synacek (jsyna...@redhat.com) wrote: Hmm, reading the file over and over and over again sounds less than ideal. Maybe we should intrdouce a struct here and then make xkb_get_keymaps() return an array of structs really? That sounds ok, I'll see what I can do. I wanted to preserve as much of the original code as I could, but maybe it wasn't the right decision. After thinking about it, I can put the layout and its variants into a structure, but... When parsing /usr/share/X11/xkb/rules/base.lst, is it safe to depend on the ordering of all the components in the file? I mean, is it safe to expect layouts being order before variants in that file? Or can it be vice versa? The code would have to be more general if one cannot depend on that order, and I think it would be quite ugly. The strv implementation doesn't have that problem. Writing a parser that doesn't mind about the order doesn't appear too complex to me... Would need a simply state machine that changes state each time one of the ! headers is encountered. And then, maintain a Set for each type of field, and keep adding entries to it, ignoring duplicates. And if you iterate through the variants and find a layout that you haven't seen before just add the layout to the set, too. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] sysusers: Preserve ownership and mode on /etc/passwd and friends
On Wed, 29.10.14 14:20, Colin Guthrie (co...@mageia.org) wrote: When running sysusers we would clobber file ownership and permissions on the files /etc/passwd, /etc/group and /etc/[g]shadow. This simply preserves the ownership and mode if existing files are found. I figure turning this into a new function that is just called a couple of times would be much nicer than repeating the same lines multiple times... Done that now. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] systemd-service for each user
Hi! I don't know, for which words I have to search to find a solution for my Problem. On my server i created a mailserver.target, on which all relevant services for the mailserver depend. So i can [re]start and stop all services (exim4, cyrus, saslauthd, amavis...) only with systemctl start|restart|stop mailserver.target The thing is, i have users not each with ~/.fetchmailrc So i've written a unit /etc/systemd/system/fetchmail@.service [Unit] Description=fetchmail for User %i BindsTo=mailserver.target After=cyrus-imapd.service exim4.service ConditionFileNotEmpty=/home/%i/.fetchmailrc [Service] Type=simple User=%i Environment=FETCHMAILUSER=%i ExecStart=/usr/bin/fetchmail --nodetach #PIDFile=/home/%i/.fetchmail.pid [Install] WantedBy=mailserver.target But i don't know, how to start this unit for each user. systemctl start mailserver should scan each user and start fetchmail@user1.service, fetchmail@user2.service... Is this possible with systemd? Or should i create a shell-script to do the job? greetings Jakob -- http://xundeenergie.at http://verkehrsloesungen.wordpress.com/ http://cogitationum.wordpress.com/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd-service for each user
On Tue, Nov 11, 2014 at 7:52 PM, Jakob Schürz wertsto...@nurfuerspam.de wrote: Hi! I don't know, for which words I have to search to find a solution for my Problem. On my server i created a mailserver.target, on which all relevant services for the mailserver depend. So i can [re]start and stop all services (exim4, cyrus, saslauthd, amavis...) only with systemctl start|restart|stop mailserver.target But i don't know, how to start this unit for each user. systemctl start mailserver should scan each user and start fetchmail@user1.service, fetchmail@user2.service... Is this possible with systemd? Or should i create a shell-script to do the job? I'm not sure I understand the point of having fetchmail download *from localhost*... it looks like it should be completely independent from cyrus/etc. Anyway, there is no command to scan each user. To start multiple units at once, make the target want all of them individually: mailserver.target.wants/fetchmail@user1.service, and so on. -- Mantas Mikulėnas graw...@gmail.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd-service for each user
Am 2014-11-11 um 20:35 schrieb Mantas Mikulėnas: On Tue, Nov 11, 2014 at 7:52 PM, Jakob Schürz wertsto...@nurfuerspam.de wrote: Hi! I don't know, for which words I have to search to find a solution for my Problem. On my server i created a mailserver.target, on which all relevant services for the mailserver depend. So i can [re]start and stop all services (exim4, cyrus, saslauthd, amavis...) only with systemctl start|restart|stop mailserver.target But i don't know, how to start this unit for each user. systemctl start mailserver should scan each user and start fetchmail@user1.service, fetchmail@user2.service... Is this possible with systemd? Or should i create a shell-script to do the job? I'm not sure I understand the point of having fetchmail download *from localhost*... it looks like it should be completely independent from cyrus/etc. My configuration here is: I have a Mailserver running on my laptop. So i can use different mailclients to see my emails. (It's an Imap-server). So fetchmail is running on my machine to fetch mails from all my mail-accounts. Also for the other Users on this machine. Anyway, there is no command to scan each user. To start multiple units at once, make the target want all of them individually: mailserver.target.wants/fetchmail@user1.service, and so on. I know that point. I have to do this individually. But i like a solution which detects and run this automatically... Maybe it's a good solution to connect this to some script, which adds and removes users... Ok. Thanks for this hint. I'll think about it. Because i have to add new users to cyrus at all an sasldb... Jakob ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemctl show environment quoting
I made some minimal changes to git now: http://cgit.freedesktop.org/systemd/systemd/commit/?id=27e9c5af817147ea1c678769e45e83f2e4b4ae96 Thanks ! This tries to improve things a bit, but I figure it might break stuff for you. No it doesn't break anything since sendmail already forbid spaces in MAILTO. The 3 lines of error handling that check for loose words will become dead code; but we need to keep those for backward compatibility. This may help other people too: https://lists.fedoraproject.org/pipermail/devel/2014-July/200859.html I will now escape spaces and newlines inside the strings to the usualy C \x012 syntax. This means spaces now become \x020. As I understand 'escaped = xescape(str, \n );' will let the '@' unaffected : cool. This makes the output reversible, but of course looks awful if env vars really contain spaces... I guess from the man page this fits nicely with the spirit of this sub-command. e.g. : display of ExecStart= that looks like a JSON thingy. Alexandre Detiste ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Should systemd-logind provide a DM-independent mechanism for handling guest accounts?
It would be fairly easy to setup pam_namespace for the guest user to provide a temporary /tmp and ~/. Now, just like we do for xguest. Then you could setup the login account to use no password and the guest_u user and allow users onto the system. This would get you most of the things you want. The problems would be in having multiple users get access to the machine at the same time. For this you need something that generates a UID on the fly for the user. I would expect a fairly simple pam module could be done for this. One problem with this though would be a user might log in as guest user but endup getting the guest134 user account. This means you would want some kind of sssd interaction, so a user executing id or ls -lZ ~/ Would see all of his files and processes running as guest. Taking advantages of other namespaces to setup additional containment might also be interesting especially the pid namespace. On 11/10/2014 04:36 PM, Lennart Poettering wrote: On Mon, 10.11.14 16:41, Laércio de Sousa (laercioso...@sme-mogidascruzes.sp.gov.br) wrote: Hi there! Currently there are few alternatives for implementing guest accounts in Linux systems. I know only two: an AppArmor-based approach implemented in LightDM, and a SELinux-based approach implemented in Fedora's package xguest that works with GDM. There's no option for console guest login (should it be needed?). I was thinking if systemd-logind could handle itself guest accounts in the future, making it available for use by any display manager (and even console logins, who knows?). What do you think about it? I figure this pays into the whole concept of dynamic users, which we really want to have eventually, to deal with dynamic allocation of UIDs for user namespacing in container managers, for allocating per-seat users for gdm login screens, and then also for your usecase, i.e. to implement guest users that go away entirely on logout. So yeah, it's definitely something we want, and I figure it should be added to the systemd project in some way. Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
