Re: [systemd-devel] [BUG] too many rfkill services
В Fri, 21 Nov 2014 01:26:36 +0100 Lennart Poettering lenn...@poettering.net пишет: On Thu, 20.11.14 19:56, Lukasz Stelmach (stl...@poczta.fm) wrote: I talked to the kernel guys at my office and they told me that it is quite usual (at least for USB devices, and my wlan and bt are USB) that devices are stopped and unregistered in the kernel before a system is suspended end reported as completely new ones with increased numbers after machine resumes. So, I have now added some code that adds BindsTo= for the device unit to the service. This won't fix much though, as the service is likely to fail in ExecStop= because it cannot find the device anymore. Yes, you are right. It accumulates the same services but now failed instead of active. Hmm ... should not systemd inform service that device it is BoundTo is gone? I mean, while service may need to do some cleanup in this case, it is obvious that it cannot access device which no more exists. This would allow graceful exit, instead of returning error. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] networkd: integrate LLDP
On Sun, Nov 23, 2014 at 10:15:10AM +0530, Susant Sahani wrote: This patch integrates LLDP with networkd. In Fedora, we already have LLDP receiver/broadcaster – ladvd. It has this neat feature of abusing ifAlias by putting endpoint name there. This gives port information right at ip l output: 3: enp5s0: BROADCAST,MULTICAST,UP,LOWER_UP mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000 link/ether 10:78:d2:cc:7e:b0 brd ff:ff:ff:ff:ff:ff alias connected to Core2-3b-24p (Fa0/7) Would it be possible to implement this in networkd? -- Tomasz Torcz RIP is irrevelant. Spoofing is futile. xmpp: zdzich...@chrome.pl Your routes will be aggreggated. -- Alex Yuriev ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemctl list-dependencies: consider BindsTo as well?
Thanks for fixing this. May I suggest to adjust man systemctl accordingly, too? Necessary parts would be the one about option --reverse and command list-dependencies. As for the latter I'd suggest another change while you're at it. Phrase required and wanted units is rather ambiguous as it might make think of WantedBy and RequiredBy which don't apply here, if I didn't get it wrong. I for one think it would be better to simply state the parameters themselves, much as it's done with --reverse already, e. g. something like Show units stated by options Requires, Wants or BindsTo in unit NAME. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/2] networkd: integrate LLDP
On Sun, Nov 23, 2014 at 1:01 PM, Tomasz Torcz to...@pipebreaker.pl wrote: On Sun, Nov 23, 2014 at 10:15:10AM +0530, Susant Sahani wrote: This patch integrates LLDP with networkd. In Fedora, we already have LLDP receiver/broadcaster – ladvd. It has this neat feature of abusing ifAlias by putting endpoint name there. This gives port information right at ip l output: 3: enp5s0: BROADCAST,MULTICAST,UP,LOWER_UP mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000 link/ether 10:78:d2:cc:7e:b0 brd ff:ff:ff:ff:ff:ff alias connected to Core2-3b-24p (Fa0/7) Would it be possible to implement this in networkd? lldpd has the same feature... In fact, I'm curious what advantages will networkd's implementation have over the existing lldpd and ladvd? -- 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] [PATCH 2/2] networkd: integrate LLDP
On Sun, Nov 23, 2014 at 12:01 PM, Tomasz Torcz to...@pipebreaker.pl wrote: On Sun, Nov 23, 2014 at 10:15:10AM +0530, Susant Sahani wrote: This patch integrates LLDP with networkd. In Fedora, we already have LLDP receiver/broadcaster – ladvd. It has this neat feature of abusing ifAlias by putting endpoint name there. This gives port information right at ip l output: 3: enp5s0: BROADCAST,MULTICAST,UP,LOWER_UP mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000 link/ether 10:78:d2:cc:7e:b0 brd ff:ff:ff:ff:ff:ff alias connected to Core2-3b-24p (Fa0/7) Would it be possible to implement this in networkd? Definitely possible. However, any suggestions on how to avoid stepping on the toes of other abusers of ifAlias, or is ladvd simply ignoring this? We definitely want this information easily accessible through networkctl, so maybe we won't need to touch ifAlias (though I'm absolutely not opposed to the idea as long as we don't gratuitously break other users). Cheers, Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2] smack: introduce new SmackLabelAccess option
On Fri, Nov 21, 2014 at 03:16:01PM +0900, WaLyong Cho wrote: In case of systemd has _ label and run as root, if a service file has User= option and the command line file has a special SMACK label then systemd will fail to access to given file. SMACK label is ignored for root uid processes. But if a service has a User= then systemd will call setresuid() in the child process. After then it no more root uid. So it should have some of accessable label for the command. To set the before the uid is changed, introduce new SmackLabelAccess=. ^ This provides the motivation. Please add a paragraph which describes the functionality. I'm still bothered by the name SmackLabelAccess. To recap the discussion, SMACK64EXEC is set on a file and it is an analogue of setuid bit, except that it changes the 'current' smack label instead of the UID. The analogy goes pretty far, since the value set with User= may be overriden by the setuid bit on the file, and SmackLabelAccess= may be be overriden by SMACK64EXEC on the file. We use User= to set the user, so the name SmackLabel= would be the most appropriate. But like Lennart said, SmackLabel= is already used to label the socket file, so we need something different. By process of elimination you arrived as SmackLabelAccess=. I don't like the name because Access is misleading, because it suggests that the label is only used when starting the process. But the normal case is that SMACK64EXEC is *not* set, so the process will run with this label until the end. Please just call it SmackProcessLabel. --- man/systemd.exec.xml | 15 +++ src/core/dbus-execute.c | 19 + src/core/execute.c| 11 src/core/execute.h| 3 +++ src/core/load-fragment-gperf.gperf.m4 | 7 +++-- src/core/load-fragment.c | 50 +++ src/core/load-fragment.h | 1 + src/shared/exit-status.h | 1 + src/shared/smack-util.c | 20 ++ src/shared/smack-util.h | 1 + 10 files changed, 126 insertions(+), 2 deletions(-) diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index e9af4ab..5381946 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -1138,6 +1138,21 @@ /varlistentry varlistentry + termvarnameSmackLabelAccess=/varname/term + +listitemparaSet the SMACK security +label to access given executable file in +varnameExecStart=/varname. If my analysis above is correct this should be reworded - it's not just for access. This option only has effect with +varnameUser=/varname. Is this entirely true? If User= is not given, and SMACK64EXEC is not set, then this will determine the label of the process iiuc. Because, SMACK access checking is ignored +for root uid processes. If varnameSmackLabelAccess=/varname is +specified with varnameUser=/varname, forked child systemd process +set its /proc/$PID/attr/current to specified label in +varnameSmackLabelAccess=/varname. This directive is ignored if +SMACK is disabled. If prefixed by literal-/literal, all errors +will be ignored./para/listitem Please describe the functionality in higher level language, what the option does. Don't go into the details of SMACK. Please mention that empty rhs can be used to unset. +/varlistentry + +varlistentry termvarnameIgnoreSIGPIPE=/varname/term listitemparaTakes a boolean diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 9276da4..0764a42 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -508,6 +508,24 @@ static int property_get_apparmor_profile( return sd_bus_message_append(reply, (bs), c-apparmor_profile_ignore, c-apparmor_profile); } +static int property_get_smack_label_access( +sd_bus *bus, +const char *path, +const char *interface, +const char *property, +sd_bus_message *reply, +void *userdata, +sd_bus_error *error) { + +ExecContext *c = userdata; + +assert(bus); +assert(reply); +assert(c); + +return sd_bus_message_append(reply, (bs), c-smack_label_access_ignore, c-smack_label_access); +} + static int property_get_personality( sd_bus *bus,
Re: [systemd-devel] [PATCH v2 0/2] Empty environment variables in unit files work
On Thu, Nov 20, 2014 at 09:18:22PM +0100, Iago López Galeiras wrote: Clarified commit message (thanks Koen Kooi) Iago López Galeiras (2): test: support empty environment variables in unit files update TODO TODO | 2 -- src/test/test-unit-file.c | 22 ++ 2 files changed, 22 insertions(+), 2 deletions(-) Applied. I squashed the two patches together though. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd-run checks path on host before running on container
On Sun, Nov 23, 2014 at 02:40:29AM +0100, Zbigniew Jędrzejewski-Szmek wrote: On Sun, Nov 23, 2014 at 12:14:32AM +0100, Zbigniew Jędrzejewski-Szmek wrote: On Sat, Nov 22, 2014 at 02:47:49PM +0100, David Herrmann wrote: Hi On Fri, Nov 21, 2014 at 5:00 AM, Peter Hutterer peter.hutte...@who-t.net wrote: I was playing around with systemd-nspawn and systemd-run. The latter doesn't seem to let me run a command that solely exists on the container. simple way of reproducing: drop a file foo into the container, then on the host run systemd-run -M mycontainer /path/to/foo I expected this to run foo on the container. It does, but checks for the command to exist locally first and fails. A simple touch /path/to/foo; chmod +x $_ is sufficient to bypass that check, but that feels somewhat odd. I pushed a partial fix which makes find_binary() ignore errors for non-local absolute paths. Something which checks the path remotely might be nicer, but I'm not sure if it's worth the hassle. Looking at this again, I think we should resolve the paths remotely. We could redefine the dbus call to do $PATH resolution on paths without any slashes, or we could a new call. I like redefining the existing one more. I don't think anyone will care. See also https://bugs.freedesktop.org/show_bug.cgi?id=86555, which is a similar problem with systemd-nspawn. from a user's perspective having systemd-run fail if the remote path doesn't exist would certainly be nice. Except it'd be a change in behaviour and may break scripts, but that's something you'd have to decide - I'm not affected here :) thanks for the quick fix Cheers, Peter ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] dbus: return non-zero return value in the case that prefix won't match
On Thu, Nov 20, 2014 at 04:06:18PM +0100, Lukas Nykryn wrote: strv_extend returns 0 in the case of success which means that else if (bus_track_deserialize_item(m-deserialized_subscribed, l) == 0) log_warning(Unknown serialization item '%s', l); will be printed when value is added correctly. Applied. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] serialization bug, swap bug, etc.
On Thu, Nov 20, 2014 at 09:13:02AM +0200, Mantas Mikulėnas wrote: ~ I'm getting this on every reload: systemd[1]: Unknown serialization item 'subscribed=:1.1' Lukas' patch should fix that. Doesn't seem to break anything though. ~ I'm also getting this on every reload: systemd[1]: [/usr/lib/systemd/system/systemd-journald.service:24] Failed to parse capability in bounding set, ignoring: CAP_AUDIT_READ I suppose I can ignore the message. I see that cap_audit_read was added to kernel 3.16, but unfortunately it doesn't exist in the current libcap release (libcap 2.24). Yeah, that's annoying. We could add a wrapper around cap_from_name... ~ If there are two .swap units for the same partition (one made by fstab-generator, another by gpt-generator), systemd tries to swapon it twice, resulting in swapon failed: Device or resource busy. IIUC, the problem is that swapons are called concurently, and one fails. If they were started sequentially, systemd would notice that they both refer to the same device and make one of the follow the other. What I'm guessing happens is that initially the units use different device names, so they are not merged. Could you post both units (systemctl cat would probably be best), so we can see what is going wrong? Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 2/5] mount: check options as well as fstype for network mounts
When creating a new mount unit after an event on /proc/self/mountinfo, check the mount options as well as the fstype to determine if this is a remote mount that requires network access. Signed-off-by: Chris Leech cle...@redhat.com --- src/core/mount.c | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 74a1da8..81e9fde 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -63,16 +63,20 @@ static const UnitActiveState state_translation_table[_MOUNT_STATE_MAX] = { static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *userdata); static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, void *userdata); +static bool mount_needs_network(const char *options, const char *fstype) { +if (mount_test_option(options, _netdev)) +return true; + +if (fstype fstype_is_network(fstype)) +return true; + +return false; +} + static bool mount_is_network(MountParameters *p) { assert(p); -if (mount_test_option(p-options, _netdev)) -return true; - -if (p-fstype fstype_is_network(p-fstype)) -return true; - -return false; +return mount_needs_network(p-options, p-fstype); } static bool mount_is_bind(MountParameters *p) { @@ -1403,8 +1407,7 @@ static int mount_add_one( if (m-running_as == SYSTEMD_SYSTEM) { const char* target; -target = fstype_is_network(fstype) ? SPECIAL_REMOTE_FS_TARGET : SPECIAL_LOCAL_FS_TARGET; - +target = mount_needs_network(options, fstype) ? SPECIAL_REMOTE_FS_TARGET : SPECIAL_LOCAL_FS_TARGET; r = unit_add_dependency_by_name(u, UNIT_BEFORE, target, NULL, true); if (r 0) goto fail; -- 1.9.3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 5/5] mount: auto-detect iSCSI and FCoE as requiring network
This adds auto detection for iSCSI and some FCoE drivers and treats mounts to file-systems on those devices as remote-fs. Signed-off-by: Chris Leech cle...@redhat.com --- src/core/mount.c | 168 +++ 1 file changed, 158 insertions(+), 10 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 513dcec..52a4ba7 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -26,6 +26,7 @@ #include signal.h #include libmount.h #include sys/inotify.h +#include libudev.h #include manager.h #include unit.h @@ -44,6 +45,7 @@ #include bus-errors.h #include exit-status.h #include def.h +#include udev-util.h static const UnitActiveState state_translation_table[_MOUNT_STATE_MAX] = { [MOUNT_DEAD] = UNIT_INACTIVE, @@ -64,20 +66,166 @@ static const UnitActiveState state_translation_table[_MOUNT_STATE_MAX] = { static int mount_dispatch_timer(sd_event_source *source, usec_t usec, void *userdata); static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, void *userdata); -static bool mount_needs_network(const char *options, const char *fstype) { +static bool scsi_host_is(struct udev_device *host, const char *type) { +_cleanup_free_ char *path; +struct stat st; + +assert(host); +assert(type); + +/* a type_host device created by the scsi transport class + * should exists for all the transport types looked for */ + +if (asprintf(path, %s/%s_host, udev_device_get_syspath(host), type) 0) { +log_oom(); +return false; +} +return stat(path, st) == 0 S_ISDIR(st.st_mode); +} + +static bool scsi_host_is_network(struct udev_device *block) { +struct udev_device *host = NULL; +struct udev *udev; + +assert(block); +udev = udev_device_get_udev(block); + +host = udev_device_get_parent_with_subsystem_devtype(block, scsi, scsi_host); +if (!host) +return false; + +/* iSCSI */ +if (scsi_host_is(host, iscsi)) +return true; + +/* FCoE appears as an FC host with a parent FCoE Ctlr device + * This will at least detect the software fcoe module and bnx2fc on kernels = 3.5 */ +if (scsi_host_is(host, fc)) { +_cleanup_free_ char *fc_host_path = NULL; +_cleanup_udev_device_unref_ struct udev_device *fc_host = NULL; +struct udev_device *fcoe_ctlr = NULL; + +if (asprintf(fc_host_path, %s/fc_host/%s, udev_device_get_syspath(host), udev_device_get_sysname(host)) 0) { +log_oom(); +return false; +}; +fc_host = udev_device_new_from_syspath(udev, fc_host_path); +fcoe_ctlr = udev_device_get_parent_with_subsystem_devtype(fc_host, fcoe, fcoe_ctlr); +if (fcoe_ctlr) +return true; +} + +return false; +} + +static int dot_filter(const struct dirent *d) { +if (streq(., d-d_name) || streq(.., d-d_name)) +return 0; +return 1; +} + +static inline bool is_partition(struct udev_device *block) { +const char *devtype; + +devtype = udev_device_get_devtype(block); +if (streq(devtype, partition)) { +return true; +} +return false; +} + +static bool block_needs_network(struct udev_device *block) { +struct udev_device *parent = NULL; +_cleanup_free_ char *slaves_path = NULL; +_cleanup_free_ struct dirent **slaves = NULL; +struct udev *udev; +int n, i; +bool rc = false; + +assert(block); +udev = udev_device_get_udev(block); + +if (scsi_host_is_network(block)) +return true; + +/* if this is a partition, check the parent device */ + +if (is_partition(block)) { +parent = udev_device_get_parent(block); +if (parent block_needs_network(parent)) +return true; +} + +/* if this block device has slaves check them as well + * this handles DM maps including LVM and multipath */ + +if (asprintf(slaves_path, %s/slaves, udev_device_get_syspath(block)) 0) { +log_oom(); +return false; +} + +n = scandir(slaves_path, slaves, dot_filter, alphasort); +for (i = 0; i n; i++) { +_cleanup_udev_device_unref_ struct udev_device *slave = NULL; +_cleanup_free_ char *newpath = NULL; +_cleanup_free_ char *rp = NULL; + +if (asprintf(newpath, %s/%s, slaves_path, slaves[i]-d_name) 0) { +log_oom(); +goto free_the_slaves; +} +rp = realpath(newpath, NULL); +if (!rp) { +
[systemd-devel] [PATCH v2 0/5] mount unit handling improvments with libmount
This patch set is an attempt to use libmount for mount unit handling, in order to address the issues I raised with the _netdev option and remote-fs ordering not working as expected. In addition, given the feedback on my previous posting I went ahead and implemented auto-detection of iSCSI and the FCoE drivers that need a functioning network interface and userspace support. It's somewhat based on the transport detection from lsblk, but using libudev and with a different FCoE check. Changes in v2: Removed all existing mountinfo parsing code that was previously left as a fall-back for building without libmount, which is now a hard build requirement. Added iSCSI and FCoE auto-detection. Chris Leech (5): mount: use libmount to enumerate /proc/self/mountinfo mount: check options as well as fstype for network mounts mount: monitor for utab changes with inotify mount: add remote-fs dependencies if needed after change mount: auto-detect iSCSI and FCoE as requiring network .travis.yml| 2 +- Makefile.am| 4 +- README | 1 + configure.ac | 10 ++ src/core/build.h | 7 ++ src/core/manager.c | 2 +- src/core/manager.h | 2 + src/core/mount.c | 309 - 8 files changed, 282 insertions(+), 55 deletions(-) -- 1.9.3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 4/5] mount: add remote-fs dependencies if needed after change
This is an attempt to add it the remote-fs dependencies to a mount unit if the options change, like when the utab options are picked up after mountinfo has already been processed. It just adds the remote-fs dependencies, leaving the local-fs ones in place. With this change I always get mount units with proper remote-fs dependencies when mounted with the _netdev option. Signed-off-by: Chris Leech cle...@redhat.com --- src/core/mount.c | 13 + 1 file changed, 13 insertions(+) diff --git a/src/core/mount.c b/src/core/mount.c index ef45115..513dcec 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1433,6 +1433,19 @@ static int mount_add_one( } } +if (m-running_as == SYSTEMD_SYSTEM) { +const char* target; + +target = mount_needs_network(options, fstype) ? SPECIAL_REMOTE_FS_TARGET : NULL; +/* _netdev option may have shown up late, or on a + * remount. Add remote-fs dependencies, even though + * local-fs ones may already be there */ +if (target) { +unit_add_dependency_by_name(u, UNIT_BEFORE, target, NULL, true); +load_extras = true; +} +} + if (u-load_state == UNIT_NOT_FOUND) { u-load_state = UNIT_LOADED; u-load_error = 0; -- 1.9.3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 3/5] mount: monitor for utab changes with inotify
Parsing the mount table with libmount races against the mount command, which will handle the actual mounting before updating utab. This means the poll event on /proc/self/mountinfo can kick of a reparse in systemd before the utab information is available. This change adds in an additional event source using inotify to watch for changes to utab. It only watches for IN_MOVED_TO events, matching libmount behavior of always overwriting this file using rename(2). This does add a second pass through the mount table parsing when utab is updated. Signed-off-by: Chris Leech cle...@redhat.com --- src/core/manager.c | 2 +- src/core/manager.h | 2 ++ src/core/mount.c | 50 -- 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 2bc1058..e71a96a 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -548,7 +548,7 @@ int manager_new(SystemdRunningAs running_as, bool test_run, Manager **_m) { m-idle_pipe[0] = m-idle_pipe[1] = m-idle_pipe[2] = m-idle_pipe[3] = -1; -m-pin_cgroupfs_fd = m-notify_fd = m-signal_fd = m-time_change_fd = m-dev_autofs_fd = m-private_listen_fd = m-kdbus_fd = -1; +m-pin_cgroupfs_fd = m-notify_fd = m-signal_fd = m-time_change_fd = m-dev_autofs_fd = m-private_listen_fd = m-kdbus_fd = m-utab_inotify_fd = -1; m-current_job_id = 1; /* start as id #1, so that we can leave #0 around as null-like value */ m-ask_password_inotify_fd = -1; diff --git a/src/core/manager.h b/src/core/manager.h index ab72548..bdeea14 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -182,6 +182,8 @@ struct Manager { /* Data specific to the mount subsystem */ FILE *proc_self_mountinfo; sd_event_source *mount_event_source; +int utab_inotify_fd; +sd_event_source *mount_utab_event_source; /* Data specific to the swap filesystem */ FILE *proc_swaps; diff --git a/src/core/mount.c b/src/core/mount.c index 81e9fde..ef45115 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -25,6 +25,7 @@ #include sys/epoll.h #include signal.h #include libmount.h +#include sys/inotify.h #include manager.h #include unit.h @@ -1548,11 +1549,13 @@ static void mount_shutdown(Manager *m) { assert(m); m-mount_event_source = sd_event_source_unref(m-mount_event_source); +m-mount_utab_event_source = sd_event_source_unref(m-mount_utab_event_source); if (m-proc_self_mountinfo) { fclose(m-proc_self_mountinfo); m-proc_self_mountinfo = NULL; } +m-utab_inotify_fd = safe_close(m-utab_inotify_fd); } static int mount_get_timeout(Unit *u, uint64_t *timeout) { @@ -1592,12 +1595,32 @@ static int mount_enumerate(Manager *m) { goto fail; } +if (m-utab_inotify_fd 0) { +m-utab_inotify_fd = inotify_init1(IN_NONBLOCK|IN_CLOEXEC); +if (m-utab_inotify_fd 0) +goto fail_with_errno; + +r = inotify_add_watch(m-utab_inotify_fd, /run/mount, IN_MOVED_TO); +if (r 0) +goto fail_with_errno; + +r = sd_event_add_io(m-event, m-mount_utab_event_source, m-utab_inotify_fd, EPOLLIN, mount_dispatch_io, m); +if (r 0) +goto fail; + +r = sd_event_source_set_priority(m-mount_utab_event_source, -10); +if (r 0) +goto fail; +} + r = mount_load_proc_self_mountinfo(m, false); if (r 0) goto fail; return 0; +fail_with_errno: +r = -errno; fail: mount_shutdown(m); return r; @@ -1609,11 +1632,34 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, int r; assert(m); -assert(revents EPOLLPRI); +assert(revents (EPOLLPRI | EPOLLIN)); /* The manager calls this for every fd event happening on the * /proc/self/mountinfo file, which informs us about mounting - * table changes */ + * table changes + * This may also be called for /run/mount events */ + +if (fd == m-utab_inotify_fd) { +char inotify_buffer[sizeof(struct inotify_event) + NAME_MAX + 1]; +struct inotify_event *event; +char *p; +int rescan = 0; + +while ((r = read(fd, inotify_buffer, sizeof(inotify_buffer))) 0) { +for (p = inotify_buffer; p inotify_buffer + r; ) { +event = (struct inotify_event *) p; +/* only care about changes to utab, but we have + * to monitor the directory to reliably get + * notifications
[systemd-devel] [PATCH v2 1/5] mount: use libmount to enumerate /proc/self/mountinfo
This lets libmount add in user options from /run/mount/utab, like _netdev which is needed to get proper ordering against remote-fs.target Signed-off-by: Chris Leech cle...@redhat.com --- .travis.yml | 2 +- Makefile.am | 4 +++- README | 1 + configure.ac | 10 + src/core/build.h | 7 ++ src/core/mount.c | 65 ++-- 6 files changed, 52 insertions(+), 37 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7e5251c..4ea2bc2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ compiler: - gcc before_install: - sudo apt-get update -qq - - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev libcap-dev libblkid-dev libpam-dev libcryptsetup-dev libaudit-dev libacl1-dev libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev libqrencode-dev libmicrohttpd-dev gtk-doc-tools gperf python2.7-dev + - sudo apt-get install autotools-dev automake autoconf libtool libdbus-1-dev libcap-dev libblkid-dev libmount-dev libpam-dev libcryptsetup-dev libaudit-dev libacl1-dev libattr1-dev libselinux-dev liblzma-dev libgcrypt-dev libqrencode-dev libmicrohttpd-dev gtk-doc-tools gperf python2.7-dev script: ./autogen.sh ./configure --enable-gtk-doc --enable-gtk-doc-pdf make V=1 sudo ./systemd-machine-id-setup make check make distcheck after_failure: cat test-suite.log notifications: diff --git a/Makefile.am b/Makefile.am index 3f9f3fa..042fb0e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1149,6 +1149,7 @@ libsystemd_core_la_CFLAGS = \ $(KMOD_CFLAGS) \ $(APPARMOR_CFLAGS) \ $(SECCOMP_CFLAGS) \ + $(MOUNT_CFLAGS) \ -pthread libsystemd_core_la_LIBADD = \ @@ -1161,7 +1162,8 @@ libsystemd_core_la_LIBADD = \ $(AUDIT_LIBS) \ $(KMOD_LIBS) \ $(APPARMOR_LIBS) \ - $(SECCOMP_LIBS) + $(SECCOMP_LIBS) \ + $(MOUNT_LIBS) if HAVE_SECCOMP libsystemd_core_la_LIBADD += \ diff --git a/README b/README index aefb349..84c6ed4 100644 --- a/README +++ b/README @@ -106,6 +106,7 @@ REQUIREMENTS: glibc = 2.14 libcap +libmount = 2.20 (from util-linux) libseccomp = 1.0.0 (optional) libblkid = 2.20 (from util-linux) (optional) libkmod = 15 (optional) diff --git a/configure.ac b/configure.ac index 2c8be53..ecd53a4 100644 --- a/configure.ac +++ b/configure.ac @@ -427,6 +427,15 @@ fi AM_CONDITIONAL(HAVE_BLKID, [test $have_blkid = yes]) # -- +have_libmount=no +PKG_CHECK_MODULES(MOUNT, [ mount = 2.20 ], +[AC_DEFINE(HAVE_LIBMOUNT, 1, [Define if libmount is available]) have_libmount=yes], have_libmount=no) +if test x$have_libmount = xno; then +AC_MSG_ERROR([*** libmount support required but libraries not found]) +fi +AM_CONDITIONAL(HAVE_LIBMOUNT, [test $have_libmount = yes]) + +# -- have_seccomp=no AC_ARG_ENABLE(seccomp, AS_HELP_STRING([--disable-seccomp], [Disable optional SECCOMP support])) if test x$enable_seccomp != xno; then @@ -1375,6 +1384,7 @@ AC_MSG_RESULT([ efi: ${have_efi} kmod:${have_kmod} blkid: ${have_blkid} +libmount:${have_libmount} dbus:${have_dbus} nss-myhostname: ${have_myhostname} gudev: ${enable_gudev} diff --git a/src/core/build.h b/src/core/build.h index 24873ab..4f639ed 100644 --- a/src/core/build.h +++ b/src/core/build.h @@ -117,6 +117,12 @@ #define _BLKID_FEATURE_ -BLKID #endif +#ifdef HAVE_LIBMOUNT +#define _LIBMOUNT_FEATURE_ +LIBMOUNT +#else +#define _LIBMOUNT_FEATURE_ -LIBMOUNT +#endif + #ifdef HAVE_ELFUTILS #define _ELFUTILS_FEATURE_ +ELFUTILS #else @@ -152,6 +158,7 @@ _LZ4_FEATURE_ \ _SECCOMP_FEATURE_ \ _BLKID_FEATURE_ \ +_LIBMOUNT_FEATURE_\ _ELFUTILS_FEATURE_\ _KMOD_FEATURE_\ _IDN_FEATURE_ diff --git a/src/core/mount.c b/src/core/mount.c index 8b787f6..74a1da8 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -24,6 +24,7 @@ #include mntent.h #include sys/epoll.h #include signal.h +#include libmount.h #include manager.h #include unit.h @@ -1492,55 +1493,47 @@ fail: return r; } +static inline void mnt_free_table_p(struct libmnt_table **tb) { +mnt_free_table(*tb); +} + +static inline void mnt_free_iter_p(struct libmnt_iter **itr) { +mnt_free_iter(*itr); +} + static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) {
Re: [systemd-devel] serialization bug, swap bug, etc.
On Mon, Nov 24, 2014 at 2:59 AM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Thu, Nov 20, 2014 at 09:13:02AM +0200, Mantas Mikulėnas wrote: ~ If there are two .swap units for the same partition (one made by fstab-generator, another by gpt-generator), systemd tries to swapon it twice, resulting in swapon failed: Device or resource busy. IIUC, the problem is that swapons are called concurently, and one fails. If they were started sequentially, systemd would notice that they both refer to the same device and make one of the follow the other. What I'm guessing happens is that initially the units use different device names, so they are not merged. Could you post both units (systemctl cat would probably be best), so we can see what is going wrong? Right, after all, since .swap unit names directly depend on device names, then it's pretty much guaranteed that the latter will differ... What I have is: --- # /run/systemd/generator/dev-disk-by\x2dpartlabel-swap.swap # Automatically generated by systemd-fstab-generator [Unit] SourcePath=/etc/fstab Documentation=man:fstab(5) man:systemd-fstab-generator(8) [Swap] What=/dev/disk/by-partlabel/swap Options=rw --- --- # /run/systemd/generator.late/dev-sda7.swap # Automatically generated by systemd-gpt-auto-generator [Unit] Description=Swap Partition Documentation=man:systemd-gpt-auto-generator(8) [Swap] What=/dev/sda7 --- -- 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] [PATCH v2] smack: introduce new SmackLabelAccess option
On 11/24/2014 02:36 AM, Zbigniew Jędrzejewski-Szmek wrote: On Fri, Nov 21, 2014 at 03:16:01PM +0900, WaLyong Cho wrote: In case of systemd has _ label and run as root, if a service file has User= option and the command line file has a special SMACK label then systemd will fail to access to given file. SMACK label is ignored for root uid processes. But if a service has a User= then systemd will call setresuid() in the child process. After then it no more root uid. So it should have some of accessable label for the command. To set the before the uid is changed, introduce new SmackLabelAccess=. ^ This provides the motivation. Please add a paragraph which describes the functionality. I'm still bothered by the name SmackLabelAccess. To recap the discussion, SMACK64EXEC is set on a file and it is an analogue of setuid bit, except that it changes the 'current' smack label instead of the UID. The analogy goes pretty far, since the value set with User= may be overriden by the setuid bit on the file, and SmackLabelAccess= may be be overriden by SMACK64EXEC on the file. We use User= to set the user, so the name SmackLabel= would be the most appropriate. But like Lennart said, SmackLabel= is already used to label the socket file, so we need something different. By process of elimination you arrived as SmackLabelAccess=. I don't like the name because Access is misleading, because it suggests that the label is only used when starting the process. But the normal case is that SMACK64EXEC is *not* set, so the process will run with this label until the end. Please just call it SmackProcessLabel. I'm agree. I will change. --- man/systemd.exec.xml | 15 +++ src/core/dbus-execute.c | 19 + src/core/execute.c| 11 src/core/execute.h| 3 +++ src/core/load-fragment-gperf.gperf.m4 | 7 +++-- src/core/load-fragment.c | 50 +++ src/core/load-fragment.h | 1 + src/shared/exit-status.h | 1 + src/shared/smack-util.c | 20 ++ src/shared/smack-util.h | 1 + 10 files changed, 126 insertions(+), 2 deletions(-) diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index e9af4ab..5381946 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -1138,6 +1138,21 @@ /varlistentry varlistentry + termvarnameSmackLabelAccess=/varname/term + +listitemparaSet the SMACK security +label to access given executable file in +varnameExecStart=/varname. If my analysis above is correct this should be reworded - it's not just for access. Yes, if the file has SMACK64EXEC then the label only be used for access checking. But if that file has no SMACK64EXEC then the label is also set as the subject of executed process. The label is used also for checking so I'd like to just add when if the file has no SMACK64EXEC then the executed process will have specified label. This option only has effect with +varnameUser=/varname. Is this entirely true? If User= is not given, and SMACK64EXEC is not set, then this will determine the label of the process iiuc. You are totally right. I just focused on accessing to execute. As you said, if the file has no SMACK64EXEC and SmackProcessLabel= is given then the executed process will has given label. I will change this. Because, SMACK access checking is ignored +for root uid processes. If varnameSmackLabelAccess=/varname is +specified with varnameUser=/varname, forked child systemd process +set its /proc/$PID/attr/current to specified label in +varnameSmackLabelAccess=/varname. This directive is ignored if +SMACK is disabled. If prefixed by literal-/literal, all errors +will be ignored./para/listitem Please describe the functionality in higher level language, what the option does. Don't go into the details of SMACK. OK, I will. Please mention that empty rhs can be used to unset. Oh, I didn't imagine this. But I can't find some other proper operation. So I think if rhs was empty then just do like the config was not given. Really thanks to read. WaLyong +/varlistentry + +varlistentry termvarnameIgnoreSIGPIPE=/varname/term listitemparaTakes a boolean diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 9276da4..0764a42 100644 ---