[systemd-devel] [PATCH 0/3] SELinuxContext configuration, v2
This series of patch implement a SELinuxContext configuration item, whose usage is explained in the first mail. This patch series take in account the feedback received on http://lists.freedesktop.org/archives/systemd-devel/2013-December/015875.html Michael Scherer (3): Add SELinuxContext configuration item Ignore the setting SELinuxContext if selinux is not enabled Add support for ignoring errors on SELinuxContext by prefixing it with -, like for others settings. man/systemd.exec.xml | 13 + src/core/dbus-execute.c | 1 + src/core/execute.c| 29 + 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, 52 insertions(+), 2 deletions(-) -- 1.8.5.3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/3] Ignore the setting SELinuxContext if selinux is not enabled
---
src/core/execute.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/core/execute.c b/src/core/execute.c
index c02c768..474a4af 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -1569,7 +1569,7 @@ int exec_spawn(ExecCommand *command,
}
}
#ifdef HAVE_SELINUX
-if (context->selinux_context) {
+if (context->selinux_context && use_selinux()) {
err =
security_check_context(context->selinux_context);
if (err < 0) {
r = EXIT_SELINUX_CONTEXT;
--
1.8.5.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/3] Add SELinuxContext configuration item
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.
---
man/systemd.exec.xml | 11 +++
src/core/dbus-execute.c | 1 +
src/core/execute.c| 27 +++
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, 48 insertions(+), 2 deletions(-)
diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index 7eaf52b..4281c03 100644
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -951,6 +951,17 @@
+SELinuxContext=
+
+Set the SELinux context of the
+executed process. If set, this will override
the
+automated domain transition. However, the
policy
+still need to autorize the transition. See
+
setexeccon3
+for details.
+
+
+
IgnoreSIGPIPE=
Takes a boolean
diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index d2bbda2..438bf59 100644
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -419,6 +419,7 @@ const sd_bus_vtable bus_exec_vtable[] = {
SD_BUS_PROPERTY("PrivateDevices", "b", bus_property_get_bool,
offsetof(ExecContext, private_devices), 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("SELinuxContext", "s", NULL, offsetof(ExecContext,
selinux_context), 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 91e4352..c02c768 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -47,6 +47,10 @@
#include
#endif
+#ifdef HAVE_SELINUX
+#include
+#endif
+
#include "execute.h"
#include "strv.h"
#include "macro.h"
@@ -1564,6 +1568,20 @@ int exec_spawn(ExecCommand *command,
goto fail_child;
}
}
+#ifdef HAVE_SELINUX
+if (context->selinux_context) {
+err =
security_check_context(context->selinux_context);
+if (err < 0) {
+r = EXIT_SELINUX_CONTEXT;
+goto fail_child;
+}
+err = setexeccon(context->selinux_context);
+if (err < 0) {
+r = EXIT_SELINUX_CONTEXT;
+goto fail_child;
+}
+}
+#endif
}
err = build_environment(context, n_fds, watchdog_usec, home,
username, shell, &our_env);
@@ -1722,6 +1740,9 @@ void exec_context_done(ExecContext *c) {
free(c->utmp_id);
c->utmp_id = NULL;
+free(c->selinux_context);
+c->selinux_context = NULL;
+
free(c->syscall_filter);
c->syscall_filter = NULL;
}
@@ -2091,6 +2112,12 @@ void exec_context_dump(ExecContext *c, FILE* f, const
char *prefix) {
fprintf(f,
"%sUtmpIdentifier: %s\n",
prefix, c->utmp_id);
+
+if (c->selinux_context)
+fprintf(f,
+"%sSELinuxContext: %s\n",
+prefix, c->selinux_context);
+
}
void exec_status_start(ExecStatus *s, pid_t pid) {
diff --git a/src/core/execute.h b/src/core/execute.h
index 4851152..be811a9 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -133,6 +133,8 @@ struct ExecContext {
char *utmp_id;
+char *selinux_context;
+
char **read_write_dirs, **read_only_di
[systemd-devel] [PATCH 3/3] Add support for ignoring errors on SELinuxContext by prefixing it with -, like for others settings.
Also remove call to security_check_context, as this doesn't serve anything,
since
setexeccon will fail anyway.
---
man/systemd.exec.xml | 4 +++-
src/core/execute.c | 14 --
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index 4281c03..ecf48a7 100644
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -956,7 +956,9 @@
Set the SELinux context of the
executed process. If set, this will override
the
automated domain transition. However, the
policy
-still need to autorize the transition. See
+still need to autorize the transition. This
directive
+is ignored if SELinux is disabled. If prefixed
by -,
+all errors will be ignored. See
setexeccon3
for details.
diff --git a/src/core/execute.c b/src/core/execute.c
index 474a4af..a4b3405 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -72,6 +72,7 @@
#include "fileio.h"
#include "unit.h"
#include "async.h"
+#include "selinux-util.h"
#define IDLE_TIMEOUT_USEC (5*USEC_PER_SEC)
#define IDLE_TIMEOUT2_USEC (1*USEC_PER_SEC)
@@ -1570,13 +1571,14 @@ int exec_spawn(ExecCommand *command,
}
#ifdef HAVE_SELINUX
if (context->selinux_context && use_selinux()) {
-err =
security_check_context(context->selinux_context);
-if (err < 0) {
-r = EXIT_SELINUX_CONTEXT;
-goto fail_child;
+bool ignore = false;
+char* c = context->selinux_context;
+if (c[0] == '-') {
+c++;
+ignore = true;
}
-err = setexeccon(context->selinux_context);
-if (err < 0) {
+err = setexeccon(c);
+if (err < 0 && !ignore) {
r = EXIT_SELINUX_CONTEXT;
goto fail_child;
}
--
1.8.5.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Howto run systemd within a linux container
On Wed, Feb 05, 2014 at 11:44:33PM +0100, Richard Weinberger wrote: > Hi! > > We're heavily using Linux containers in our production environment. > As modern Linux distributions move forward to systemd have to make sure that > systemd works within our containers. > > Sadly we're facing issues with cgroups. > Our testbed consists of openSUSE 13.1 with Linux 3.13.1 and libvirt 1.2.1. > > In a plain setup systemd stops immediately because it is unable to > create the cgroup hierarchy. > Mostly because the container uid 0 is in a user namespace and has no > rights to do that. FYI I have succesfully run Fedora 19 with systemd inside a container with libvirt LXC, however, I did *not* enable user namespaces. Every time I try user namespaces I find some other bug in either the kernel or libvirt, so I wouldn't be surprised if yet more breakage has occurred in user namepsaces :-( > Next try, fool systemd by mounting a tmpfs to /sys/fs/cgroup/systemd/. > This seems to work. openSUSE boots, I can start/stop services... > Shutdown hangs forever, had no time to investigate so far. > > But is this tmpfs hack the correct way to run systemd in a container? > I really don't think so. Yeah that really shouldnt' be needed. When libvirt runs a container it creates a cgroup just for that container to run in, and systemd should be able to create its hierarchy under that location. That said, I wonder if libvirt is perhaps forgetting to chown() the cgroup to the UID/GID you've mapped for the root user. That would certainly prevent systemd using it and could cause the sort of pain you see. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Howto run systemd within a linux container
On Thu, Feb 06, 2014 at 10:55:01AM +, Daniel P. Berrange wrote: > On Wed, Feb 05, 2014 at 11:44:33PM +0100, Richard Weinberger wrote: > > Hi! > > > > We're heavily using Linux containers in our production environment. > > As modern Linux distributions move forward to systemd have to make sure that > > systemd works within our containers. > > > > Sadly we're facing issues with cgroups. > > Our testbed consists of openSUSE 13.1 with Linux 3.13.1 and libvirt 1.2.1. > > > > In a plain setup systemd stops immediately because it is unable to > > create the cgroup hierarchy. > > Mostly because the container uid 0 is in a user namespace and has no > > rights to do that. > > FYI I have succesfully run Fedora 19 with systemd inside a container > with libvirt LXC, however, I did *not* enable user namespaces. Every > time I try user namespaces I find some other bug in either the kernel > or libvirt, so I wouldn't be surprised if yet more breakage has > occurred in user namepsaces :-( Those bugs should now be fixed, if you don't enable the option, how are we supposed to know what is left to be done? :) greg k-h ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Howto run systemd within a linux container
On Thu, Feb 06, 2014 at 04:33:22PM +0100, Greg KH wrote: > On Thu, Feb 06, 2014 at 10:55:01AM +, Daniel P. Berrange wrote: > > On Wed, Feb 05, 2014 at 11:44:33PM +0100, Richard Weinberger wrote: > > > Hi! > > > > > > We're heavily using Linux containers in our production environment. > > > As modern Linux distributions move forward to systemd have to make sure > > > that > > > systemd works within our containers. > > > > > > Sadly we're facing issues with cgroups. > > > Our testbed consists of openSUSE 13.1 with Linux 3.13.1 and libvirt 1.2.1. > > > > > > In a plain setup systemd stops immediately because it is unable to > > > create the cgroup hierarchy. > > > Mostly because the container uid 0 is in a user namespace and has no > > > rights to do that. > > > > FYI I have succesfully run Fedora 19 with systemd inside a container > > with libvirt LXC, however, I did *not* enable user namespaces. Every > > time I try user namespaces I find some other bug in either the kernel > > or libvirt, so I wouldn't be surprised if yet more breakage has > > occurred in user namepsaces :-( > > Those bugs should now be fixed, if you don't enable the option, how are > we supposed to know what is left to be done? :) I have in fact been building my own kernels for Fedora with user namespaces enabled to debug / test this and have reported all the bugs I found so far. Just saying that with the track record of bugs since the userns code first merged, I wouldn't be surprised if there were still more things to iron out as we try more real world apps like systemd. Regads, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] tty: Set correct tty name in 'active' sysfs attribute
On 02/05/2014 01:53 PM, David Herrmann wrote:
> Hi
>
> On Wed, Feb 5, 2014 at 11:11 AM, Hannes Reinecke wrote:
>> The 'active' sysfs attribute should refer to the currently
>> active tty devices the console is running on, not the currently
>> active console.
>> The console structure doesn't refer to any device in sysfs,
>> only the tty the console is running on has.
>> So we need to print out the tty names in 'active', not
>> the console names.
>>
>> Cc: Lennart Poettering
>> Cc: Kay Sievers
>> Signed-off-by: Werner Fink
>> Signed-off-by: Hannes Reinecke
>> ---
>> drivers/tty/tty_io.c | 14 --
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
>> index c74a00a..17db8ca 100644
>> --- a/drivers/tty/tty_io.c
>> +++ b/drivers/tty/tty_io.c
>> @@ -3545,9 +3545,19 @@ static ssize_t show_cons_active(struct device *dev,
>> if (i >= ARRAY_SIZE(cs))
>> break;
>> }
>> - while (i--)
>> + while (i--) {
>> + const struct tty_driver *driver;
>> + const char *name = cs[i]->name;
>> + int index = cs[i]->index;
>> +
>> + driver = cs[i]->device(cs[i], &index);
>> + if (driver) {
>> + index += driver->name_base;
>> + name = driver->name;
>> + }
>> count += sprintf(buf + count, "%s%d%c",
>> -cs[i]->name, cs[i]->index, i ? ' ':'\n');
>> +name, index, i ? ' ':'\n');
>> + }
>
> Nice catch and indeed, systemd already relies on these names to be
> identical to their char-dev name. Fortunately, VTs and most serial
> devices register the console with the same name as the TTY, so we're
> fine.
> Two minor nitpicks:
> 1) Could you use tty_line_name() instead of sprintf()? It's in the
> same file and avoids duplicating the name_base logic.
Ok. Not that it makes the patch nicer, but hey.
> 2) Does it make sense to print the console-name if ->device() returns
> NULL? Seems weird if we print console-names and tty-names in the same
> attribute. It's unlikely that it causes problems, but there might be
> conflicts.
>
This is basically a fallback; this is the old behaviour, which still
might be called for when coming across a tty which just has a stub
for the ->device callback.
It's not that the '->device' callback is used that frequently, so I
wouldn't be surprised here.
Meanwhile I've sent a new patch, reviews are welcome there.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] udev: Adding zram to inappropriate block device
On Thu, Feb 06, 2014 at 03:58:59AM +0100, Zbigniew Jędrzejewski-Szmek wrote: > On Thu, Feb 06, 2014 at 03:27:07AM +0100, Greg KH wrote: > > On Thu, Feb 06, 2014 at 01:31:37AM +0100, Zbigniew Jędrzejewski-Szmek wrote: > > > Patch applied. > > > > > > On Mon, Feb 03, 2014 at 10:33:37AM +, "Jóhann B. Guðmundsson" wrote: > > > > On 02/03/2014 09:36 AM, Holger Schurig wrote: > > > > >>with unit type ending in .zswap > > > > >No, not another unit type. Instead better amend .swap unit types to > > > > >also know about ZRAM. > > > > > > > > > >However, isn't this a bit early? Shouldn't move ZRAM first move out of > > > > >staging? > > > > > > > > Ofcourse but when it does move out of staging we could have sorted > > > > this implementation detail out which basically boils down to where > > > > to set the partition sizes for the zram partitions ( > > > > tmpfiles.d/zram-conf or /etc/zram.d/zram-conf ) > > > > > > > > Do we want a script that basically just set this size based on > > > > available memory per core in the udev rule. > > > > > > > > factor=25 > > > > num_cpus=$(grep -c processor /proc/cpuinfo) > > > > memtotal=$(grep MemTotal /proc/meminfo | sed 's/[^0-9]\+//g') > > > > mem_by_cpu=$(($memtotal/$num_cpus*$factor/100*1024)) > > > > echo $mem_by_cpu > > > > > > [Side note: I'm reading Documentation/blockdev/zram.txt... > > > It says "1. modprobe zram num_devices=4". I can't help thinking that > > > we went over this with /dev/loop-control and others... Since this > > > is a new interface, why does it repeat the same pitfall of not > > > having a control device?] > > > > We can always change this now, quick, before 3.14 is out. > > > > What would a control device help with here? > Right now you have to decide before loading the module how many > devices you want. And also when trying to use a device (any device), > you have to look for one. The same issues as with loop. Given that the code doesn't have the ability to dynamically add/remove devices, I think we are stuck with this, right? greg k-h ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCHv2] tty: Set correct tty name in 'active' sysfs attribute
On 02/06/2014 04:29 PM, Greg Kroah-Hartman wrote: > On Thu, Feb 06, 2014 at 03:27:43PM +0100, Hannes Reinecke wrote: >> The 'active' sysfs attribute should refer to the currently >> active tty devices the console is running on, not the currently >> active console. > > That's not what Documentation/ABI/sysfs-tty says: > Shows the list of currently configured > > console devices, like 'tty1 ttyS0'. > > The last entry in the file is the active > > device connected to /dev/console. > > The file supports poll() to detect virtual > > console switches. > The problem is indeed with 'console devices'. There is no such thing; you only have tty devices where the console is running on. >> The console structure doesn't refer to any device in sysfs, >> only the tty the console is running on has. > > That sentance doesn't make sense. > >> So we need to print out the tty names in 'active', not >> the console names. > > But that doesn't match the documentation. > > What exactly are you trying to "fix" here? What is the problem that the > current file has that is broken? And as you are changing what this file > means, what will break if the information in the file changes? > systemd is using the 'active' sysfs attribute to figure out on which _tty_ device to start a getty on. As soon as the console name and the tty name are different you have no means of figuring out which _device_ to open. AFAICS the console 'device' (ie the current entry in 'active') doesn't have _any_ equivalent in sysfs; it just so happens that for most console drivers the tty driver name is identical. But this is not a requirement, and fails for drivers which have a different device for the console and the tty. EG on S/390 the 3270 tty has the devices /dev/3270/tty1 but the console driver announces the name 'tty3270'. So as per current rules the 'active' attribute contains tty32700 which correct as per documentation, but doesn't have _any_ equivalent in sysfs. Martin has the grubby details here. But of course, the documentation should be updated to match the new behavior. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] fstrim "cron" job
On Sun, Dec 22, 2013 at 11:06:19AM +0100, Tom Gundersen wrote: > On Sat, Dec 21, 2013 at 7:11 PM, Chris Murphy wrote: > > > > On Dec 21, 2013, at 6:44 AM, Kay Sievers wrote: > > > >> Trimming should be the job of the filesystem, not for a nasty cron > >> job. We do not want to support legacy filesystems with upstream > >> shipped systemd units. > >> > >> Also, util-linux must not ship such policy, it's a collection of > >> tools, not a system policy carry-out. > > > > Well it's the job of the file system, the device mapper, the block layer, > > the ATA driver, the controller and then the drive. And at the bottom of > > this stack, the drive specification, is flawed. We're not going to see the > > file systems doing this in ideal fashion, none of them set discard by > > default, until everything below is properly enabling asynchronous queued > > TRIM. > > > > So the question is whether it makes sense to design a work around for what > > amount to legacy devices (even though they are still being bought and sold > > today), or entirely ignore this (automatic) optimization for the life of > > the devices and leave it up to the user to set such things. > > > >> We need to support fsck because it's needed for integrity and using > >> filesystems that need, but running trim is just an optimization. We do > >> not want the bugs for these filesystems triggered by the systemd > >> package. > > > > It seems systemd now parses fstab and can second guess its contents, e.g. > > it will ignore fs_passno for Btrfs, so even if it's a non-zero value, > > systemd doesn't cause fsck to go looking for an fsck.btrfs. > > > > But it does for xfs, which likewise doesn't need fsck at all. > > We don't actually check for btrfs, but simply skip any checking when > /sbin/fsck. does not exist. > > > I don't know if these optimizations really belong in systemd or rather in a > > smarter fsck to keep a list of file systems that do and don't need fsck > > performed on them prior to remount as rw. > > I'd argue that the systemd behavior of ignoring missing helpers should > just be moved to fsck... OK, I have improved fsck, so it does not print any error message if fsck. does not exist and the filesystem type is not in "really wanted" set of the filesystems (the set was defined many years ago by Ted and it's mostly about extN;-). # blkid -s TYPE /dev/sdb /dev/sdb: TYPE="btrfs" # fsck /dev/sdb; echo $? fsck from util-linux 2.24.184-663b-dirty 0 Karel -- Karel Zak http://karelzak.blogspot.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] fstrim "cron" job
On Thu, Feb 6, 2014 at 4:51 PM, Karel Zak wrote: > On Sun, Dec 22, 2013 at 11:06:19AM +0100, Tom Gundersen wrote: >> On Sat, Dec 21, 2013 at 7:11 PM, Chris Murphy >> wrote: >> > >> > On Dec 21, 2013, at 6:44 AM, Kay Sievers wrote: >> > >> >> Trimming should be the job of the filesystem, not for a nasty cron >> >> job. We do not want to support legacy filesystems with upstream >> >> shipped systemd units. >> >> >> >> Also, util-linux must not ship such policy, it's a collection of >> >> tools, not a system policy carry-out. >> > >> > Well it's the job of the file system, the device mapper, the block layer, >> > the ATA driver, the controller and then the drive. And at the bottom of >> > this stack, the drive specification, is flawed. We're not going to see the >> > file systems doing this in ideal fashion, none of them set discard by >> > default, until everything below is properly enabling asynchronous queued >> > TRIM. >> > >> > So the question is whether it makes sense to design a work around for what >> > amount to legacy devices (even though they are still being bought and sold >> > today), or entirely ignore this (automatic) optimization for the life of >> > the devices and leave it up to the user to set such things. >> > >> >> We need to support fsck because it's needed for integrity and using >> >> filesystems that need, but running trim is just an optimization. We do >> >> not want the bugs for these filesystems triggered by the systemd >> >> package. >> > >> > It seems systemd now parses fstab and can second guess its contents, e.g. >> > it will ignore fs_passno for Btrfs, so even if it's a non-zero value, >> > systemd doesn't cause fsck to go looking for an fsck.btrfs. >> > >> > But it does for xfs, which likewise doesn't need fsck at all. >> >> We don't actually check for btrfs, but simply skip any checking when >> /sbin/fsck. does not exist. >> >> > I don't know if these optimizations really belong in systemd or rather in >> > a smarter fsck to keep a list of file systems that do and don't need fsck >> > performed on them prior to remount as rw. >> >> I'd argue that the systemd behavior of ignoring missing helpers should >> just be moved to fsck... > > OK, I have improved fsck, so it does not print any error message if > fsck. does not exist and the filesystem type is not in "really > wanted" set of the filesystems (the set was defined many years ago by > Ted and it's mostly about extN;-). > > # blkid -s TYPE /dev/sdb > /dev/sdb: TYPE="btrfs" > > # fsck /dev/sdb; echo $? > fsck from util-linux 2.24.184-663b-dirty > 0 Thanks! Cheers, Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCHv2] tty: Set correct tty name in 'active' sysfs attribute
On Thu, Feb 06, 2014 at 04:44:20PM +0100, Hannes Reinecke wrote: > On 02/06/2014 04:29 PM, Greg Kroah-Hartman wrote: > > On Thu, Feb 06, 2014 at 03:27:43PM +0100, Hannes Reinecke wrote: > >> The 'active' sysfs attribute should refer to the currently > >> active tty devices the console is running on, not the currently > >> active console. > > > > That's not what Documentation/ABI/sysfs-tty says: > > Shows the list of currently configured > > > > console devices, like 'tty1 ttyS0'. > > > > The last entry in the file is the active > > > > device connected to /dev/console. > > > > The file supports poll() to detect virtual > > > > console switches. > > > The problem is indeed with 'console devices'. There is no such > thing; you only have tty devices where the console is running on. > > >> The console structure doesn't refer to any device in sysfs, > >> only the tty the console is running on has. > > > > That sentance doesn't make sense. > > > >> So we need to print out the tty names in 'active', not > >> the console names. > > > > But that doesn't match the documentation. > > > > What exactly are you trying to "fix" here? What is the problem that the > > current file has that is broken? And as you are changing what this file > > means, what will break if the information in the file changes? > > > systemd is using the 'active' sysfs attribute to figure out on which > _tty_ device to start a getty on. > As soon as the console name and the tty name are different > you have no means of figuring out which _device_ to open. > AFAICS the console 'device' (ie the current entry in 'active') > doesn't have _any_ equivalent in sysfs; it just so happens that for > most console drivers the tty driver name is identical. > But this is not a requirement, and fails for drivers which have a > different device for the console and the tty. > > EG on S/390 the 3270 tty has the devices > > /dev/3270/tty1 > > but the console driver announces the name 'tty3270'. > So as per current rules the 'active' attribute contains > > tty32700 > > which correct as per documentation, but doesn't have _any_ > equivalent in sysfs. > > Martin has the grubby details here. > > But of course, the documentation should be updated to match the new > behavior. Ok, care to send an updated version, that fixes the Documentation as well? If Kay agrees that this is the correct solution, I'll be glad to take it. 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] udev: Adding zram to inappropriate block device
On 02/06/2014 03:39 PM, Greg KH wrote: >Right now you have to decide before loading the module how many >devices you want. And also when trying to use a device (any device), >you have to look for one. The same issues as with loop. Given that the code doesn't have the ability to dynamically add/remove devices, I think we are stuck with this, right? This may come as a completely stupid question but if the code is expected to be able to dynamically add/remove devices should not upstream ( kernel ) nack inclusion of zram until it does? JBG ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] udev: Adding zram to inappropriate block device
On Thu, Feb 06, 2014 at 04:54:59PM +, "Jóhann B. Guðmundsson" wrote: > > On 02/06/2014 03:39 PM, Greg KH wrote: > >>>Right now you have to decide before loading the module how many > >>>devices you want. And also when trying to use a device (any device), > >>>you have to look for one. The same issues as with loop. > >Given that the code doesn't have the ability to dynamically add/remove > >devices, I think we are stuck with this, right? > > This may come as a completely stupid question but if the code is expected > to be able to dynamically add/remove devices should not upstream ( kernel ) > nack inclusion of zram until it does? I don't think the code is expected to be able to do that, have you looked at it and seen where it does? 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] udev: Adding zram to inappropriate block device
Am 06.02.2014 18:45, schrieb Greg KH: > On Thu, Feb 06, 2014 at 04:54:59PM +, "Jóhann B. Guðmundsson" wrote: >> >> On 02/06/2014 03:39 PM, Greg KH wrote: > Right now you have to decide before loading the module how many > devices you want. And also when trying to use a device (any device), > you have to look for one. The same issues as with loop. >>> Given that the code doesn't have the ability to dynamically add/remove >>> devices, I think we are stuck with this, right? >> >> This may come as a completely stupid question but if the code is expected >> to be able to dynamically add/remove devices should not upstream ( kernel ) >> nack inclusion of zram until it does? > > I don't think the code is expected to be able to do that, have you > looked at it and seen where it does? please re-read both posts in the last one clearly a "not" fails fixed version: >> This may come as a completely stupid question but if the code is *not* >> expected >> to be able to dynamically add/remove devices should not upstream ( kernel ) >> nack inclusion of zram until it does? signature.asc Description: OpenPGP digital signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] systemd crashes if locale.conf contains invalid utf8 string
In the parse_env_file_push() and load_env_file_push() functions, there
are two assert() call to check if the key or value parameters are utf8 valid.
If the strings aren't utf8 valid, assert does abort.
These function are used early by systemd to parse some files. For
example '/etc/locale.conf'. In my case this file contained a not utf8
sequence, which is bad, but systemd crashed during the boot, which
is even worse !
The enclosed patch removes the assert and return -EINVAL if the
sequence is invalid. This is possible because the caller of these
function [1] checks the errors.
So the check of an invalid utf8 sequence is still performed, but
systemd doesn't crash anymore and logs the error.
BR
G.Baroncelli
[1] parse_env_file_internal(), invoked by load_env_file() and
parse_env_file()
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
diff --git a/src/shared/fileio.c b/src/shared/fileio.c
index ede8819..38af34b 100644
--- a/src/shared/fileio.c
+++ b/src/shared/fileio.c
@@ -534,35 +534,41 @@ fail:
static int parse_env_file_push(const char *filename, unsigned line,
const char *key, char *value, void *userdata) {
-assert(utf8_is_valid(key));
-if (value && !utf8_is_valid(value))
+const char *k;
+va_list* ap = (va_list*) userdata;
+va_list aq;
+
+if (!utf8_is_valid(key)) {
+log_error("%s:%u: invalid UTF-8 for key '%s', ignoring.",
+ filename, line, key);
+return -EINVAL;
+}
+
+if (value && !utf8_is_valid(value)) {
/* FIXME: filter UTF-8 */
-log_error("%s:%u: invalid UTF-8 for key %s: '%s', ignoring.",
+log_error("%s:%u: invalid UTF-8 value for key %s: '%s',
ignoring.",
filename, line, key, value);
-else {
-const char *k;
-va_list* ap = (va_list*) userdata;
-va_list aq;
+return -EINVAL;
+}
-va_copy(aq, *ap);
+va_copy(aq, *ap);
-while ((k = va_arg(aq, const char *))) {
-char **v;
+while ((k = va_arg(aq, const char *))) {
+char **v;
-v = va_arg(aq, char **);
+v = va_arg(aq, char **);
-if (streq(key, k)) {
-va_end(aq);
-free(*v);
-*v = value;
-return 1;
-}
+if (streq(key, k)) {
+va_end(aq);
+free(*v);
+*v = value;
+return 1;
}
-
-va_end(aq);
}
+va_end(aq);
+
free(value);
return 0;
}
@@ -586,26 +592,31 @@ int parse_env_file(
static int load_env_file_push(const char *filename, unsigned line,
const char *key, char *value, void *userdata) {
-assert(utf8_is_valid(key));
+char ***m = userdata;
+char *p;
+int r;
-if (value && !utf8_is_valid(value))
+if (!utf8_is_valid(key)) {
+log_error("%s:%u: invalid UTF-8 for key '%s', ignoring.",
+ filename, line, key);
+return -EINVAL;
+}
+
+if (value && !utf8_is_valid(value)) {
/* FIXME: filter UTF-8 */
-log_error("%s:%u: invalid UTF-8 for key %s: '%s', ignoring.",
+log_error("%s:%u: invalid UTF-8 value for key %s: '%s',
ignoring.",
filename, line, key, value);
-else {
-char ***m = userdata;
-char *p;
-int r;
+return -EINVAL;
+}
-p = strjoin(key, "=", strempty(value), NULL);
-if (!p)
-return -ENOMEM;
+p = strjoin(key, "=", strempty(value), NULL);
+if (!p)
+return -ENOMEM;
-r = strv_push(m, p);
-if (r < 0) {
-free(p);
-return r;
-}
+r = strv_push(m, p);
+if (r < 0) {
+free(p);
+return r;
}
free(value);
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] man: cryptsetup now allows partition device file in system mode
--- man/crypttab.xml | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/man/crypttab.xml b/man/crypttab.xml index 5f386e5..c563851 100644 --- a/man/crypttab.xml +++ b/man/crypttab.xml @@ -305,14 +305,7 @@ Use TrueCrypt in system encryption mode. This implies -tcrypt. - -Please note that when using this mode, the -whole device needs to be given in the second -field instead of the partition. For example: if -/dev/sda2 is the system -encrypted TrueCrypt patition, /dev/sda -has to be given. + tcrypt. -- 1.8.5.3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] Add SELinuxContext configuration item
In order to maximize consistency with newly committed options in systemd-nspawn, would it make sense to allow independent configuration of the process and file labels instead? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd crashes if locale.conf contains invalid utf8 string
+1 on no crashing with invalid user input ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/7] logind: close races on user and session states during login
Currently the user and session states are not stable, they are affected
by several races during login:
1) session state:
To get the session state the function session_get_state() is used.
Opening state:
At login the D-Bus CreateSession() method will call session_start() which
calls user_start() and session_start_scope() to queue the scope job and
set the session->scope_job
=> session_get_state() == SESSION_OPENING (correct)
Then execution will continue from session_send_create() which is called
by the D-Bus match_job_removed() signal. math_job_removed() will check if
this is the session scope and if this is the previously queued scope job.
If so it will clear the session->scope_job
=> session_get_state() == SESSION_CLOSING (incorrect)
(session closing since fifo_fd == -1)
So scope job has finished and scope was created successfully, later the
session_send_create_reply() will wait for the session scope *and* the
user service to be successfully created.
/* user service is still pending */
if (s->scope_job || s->user->service_job))
return 0
=> session_get_state() == SESSION_CLOSING (incorrect)
else
session_create_fifo()/* fifo_fd finally created */
=> session_get_state() == SESSION_ACTIVE (correct)
To sum it up, current state during login:
"SESSION_OPENING"=>"SESSION_CLOSING"x2=>"SESSION_ACTIVE"
To fix the session state and remove the two incorrect SESSION_CLOSING,
we do not clear up the "session->scope_job" when we detect that this is
the session scope, we setup a temporary variable "scope_job" that will
get the current value of "session->scope_job" and update it if
necessary.
Add a new "active" variable to check if the session scope and user
service jobs are still being created.
Update session_jobs_replay() and session_send_create_reply() function to
receive the "opening" variable as an argument, so it will still wait for
the scope and service jobs to finish before creating the session fifo.
The "session->scope_job" will be cleared when session_jobs_reply()
finishes. This ensures that the state will just go from:
"SESSION_OPENING" => "SESSION_ACTIVE"
2) user state:
To get the user state the function user_get_state() is used.
I'll add that the user_get_state() and session_get_state() do not have
the same logic when it comes to sessions, this will just add ambiguity.
user_get_state() calls session_is_active() before checking
session_get_state(), and session_is_active() will return true right from
the start since the logic is set during D-Bus CreateSession(). This will
we be fixed in the followup patches.
Opening state:
At login we have session_start() which calls user_start()
here we already:
=> user_get_state() == USER_ACTIVE (incorrect)
(not fixed in this patch)
user_start() calls:
user_start_slice() queue the slice and set user->slice_job
user_start_service() queue the service and set user->service_job
=> user_get_state() == USER_OPENING (correct)
Then execution will continue from session_send_create() which is called
by the D-Bus match_job_removed() signal. math_job_removed() will check if
these are the user service and slice and if they are the previously queued
jobs. If so it will clear the: user->service_job and user->slice_job
=> user_get_state() == USER_ACTIVE (incorrect)
(incorrect since the fifo_fd has not been created,
here the state should stay USER_OPENING)
Later when the user service is created successfully,
session_send_create_reply() will also wait for the session scope to be
created. If so then session_send_create_reply() will continue and call
session_create_fifo()
=> user_get_state() == USER_ACTIVE (correct)
(fifo_fd was created)
To fix this, we use the same logic as used to fix session states. In
order to remove the incorrect state when the fifo_fd is not created but
the state shows USER_ACTIVE, we do not clear the "user->service_job" and
"user->slice_job" right away, we store the state in a temporary variable
"service_job" and update it later if we detect that this is the user
service.
The new "active" variable will be used to check if the session scope and
user service are still being created. If so we'll wait for the next
match_job_removed() signal and continue, otherwise we proceed with
session_jobs_reply() and session_send_create_reply() in order to notify
clients.
---
src/login/logind-dbus.c | 44 ++---
src/login/logind-session-dbus.c | 8 +---
src/login/logind-session.h | 2 +-
3 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 7b050fb..0560707 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -1919,7 +1919,9 @@ const sd_bus_vtable manager_vtable[] = {
SD_BUS_VTABLE_END
};
-static int session_jobs_reply(Sessio
[systemd-devel] [PATCH 1/7] logind: add function session_jobs_reply() to unify the create reply
The session_send_create_reply() function which notifies clients about
session creation is used for both session and user units. Unify the
shared code in a new function session_jobs_reply().
The session_save() will be called unconditionally on sessions since it
does not make sense to only call it if '!session->started', this will
also allow to update the session state as soon as possible.
---
src/login/logind-dbus.c | 46 --
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 4745961..7b050fb 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -1919,6 +1919,27 @@ const sd_bus_vtable manager_vtable[] = {
SD_BUS_VTABLE_END
};
+static int session_jobs_reply(Session *s, const char *unit, const char
*result) {
+int r = 0;
+
+assert(s);
+assert(unit);
+
+if (!s->started)
+return r;
+
+if (streq(result, "done"))
+r = session_send_create_reply(s, NULL);
+else {
+_cleanup_bus_error_free_ sd_bus_error e = SD_BUS_ERROR_NULL;
+
+sd_bus_error_setf(&e, BUS_ERROR_JOB_FAILED, "Start job for
unit %s failed with '%s'", unit, result);
+r = session_send_create_reply(s, &e);
+}
+
+return r;
+}
+
int match_job_removed(sd_bus *bus, sd_bus_message *message, void *userdata,
sd_bus_error *error) {
const char *path, *result, *unit;
Manager *m = userdata;
@@ -1958,18 +1979,9 @@ int match_job_removed(sd_bus *bus, sd_bus_message
*message, void *userdata, sd_b
session->scope_job = NULL;
}
-if (session->started) {
-if (streq(result, "done"))
-session_send_create_reply(session, NULL);
-else {
-_cleanup_bus_error_free_ sd_bus_error e =
SD_BUS_ERROR_NULL;
-
-sd_bus_error_setf(&e, BUS_ERROR_JOB_FAILED,
"Start job for unit %s failed with '%s'", unit, result);
-session_send_create_reply(session, &e);
-}
-} else
-session_save(session);
+session_jobs_reply(session, unit, result);
+session_save(session);
session_add_to_gc_queue(session);
}
@@ -1987,17 +1999,7 @@ int match_job_removed(sd_bus *bus, sd_bus_message
*message, void *userdata, sd_b
}
LIST_FOREACH(sessions_by_user, session, user->sessions) {
-if (!session->started)
-continue;
-
-if (streq(result, "done"))
-session_send_create_reply(session, NULL);
-else {
-_cleanup_bus_error_free_ sd_bus_error e =
SD_BUS_ERROR_NULL;
-
-sd_bus_error_setf(&e, BUS_ERROR_JOB_FAILED,
"Start job for unit %s failed with '%s'", unit, result);
-session_send_create_reply(session, &e);
-}
+session_jobs_reply(session, unit, result);
}
user_save(user);
--
1.8.3.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 0/7] logind: close races on user and session states
Summary: Currently logind will not clear sessions on logout. The bug is confirmed for getty and ssh logins. This series is preparation for next patches to address that bug, it does not fix it. However, this series also fixe real races on user and session states. This ensures that user_save() and session_save() functions will write the correct user and session state to the state files. The logind bug was already discussed here: http://lists.freedesktop.org/archives/systemd-devel/2014-January/015968.html Patches were proposed, but they failed to address the bug since there are other problems related to user and session states: http://lists.freedesktop.org/archives/systemd-devel/2014-January/016372.html A first version to fix these race conditions on user and sessions states was proposed: http://lists.freedesktop.org/archives/systemd-devel/2014-January/016373.html This series is a v2, it will close all the discovered races on user and session states. The commit logs will tell you the story of each case. Now as noted above, this series fix real races and in the same time it is needed to fix the logind bug where sessions are not cleaned after logouts. Proposed plan to clean logind closed sessions: 1) Make the user and session states stable (this series fix it). 2) Improve session_check_gc() and user_check_gc() to check if: the state is closing and the cgroup is empty. 3) Push session and user into the gc during logout, in pam_systemd method_release_session() Now I've a patch that implements 2) and 3) on top of 1), sometimes it will work and successfully clean closed sessions, and sometimes it will not. This is due to a race when we close the session and try to terminate session processes and in the same time we are trying to see if the cgroup is empty... which is another problem on its own. So here are the patches, please consider this series since it will fix real races and it will make sure that user_save() and session_save() will write the correct state to the state files. Patches 1,6,7 are code cleanup. Patches 2,3,4,5 close races on user and session states. Djalal Harouni (7): 0001 logind: add function session_jobs_reply() to unify the create reply unify shared code in a single function. 0002 logind: close races on user and session states during login 0003 logind: close races on session state at logout 0004 logind: close races on user state at logout close race conditions on user and session states. 0005 logind: just call session_get_state() to get the session state make user_get_state() consistent with session_get_state() 0006 logind: add user_is_opening() and session_is_opening() 0007 logind: do not call session_jobs_reply() on CLOSING make sure to not call session_send_create_reply() when jobs finish during closing state. src/login/logind-dbus.c | 87 +++ src/login/logind-session-dbus.c | 11 --- src/login/logind-session.c | 10 +- src/login/logind-session.h | 4 +++- src/login/logind-user.c | 20 +--- src/login/logind-user.h | 3 +++ 6 files changed, 99 insertions(+), 36 deletions(-) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/7] logind: close races on session state at logout
To get the state of the session, the session_get_state() is used.
This function will check if the session->scope_job is set then it will
automatically return SESSION_OPENING. This is buggy in the context of
session closing:
At logout or D-Bus TerminateSession() fifo_fd is removed:
=> session_get_state() == SESSION_CLOSING (correct)
Then we have session_stop() which calls session_stop_scope(), this
will queue the scope job to be removed and set the session->scope_job
=> session_get_state() == SESSION_OPENING (incorrect)
After the scope job is removed the state will be again correct
=> session_get_state() == SESSION_CLOSING (correct)
To fix this and make sure that the state will always be SESSION_CLOSING
we add a flag that is used to differentiate between SESSION_OPENING and
SESSION_CLOSING.
The 'scope_opening' flag will be set to true only during real session
opening in session_start_scope(), and it will be set to false just after
the session fifo fd was successfully created, which means that during
session closing it will be false.
And update session_get_state() to check if the 'scope_opening' is true
before returning the SESSION_OPENING, if it is not then SESSION_CLOSING
will be returned which is the correct behaviour.
---
src/login/logind-session-dbus.c | 3 +++
src/login/logind-session.c | 4 +++-
src/login/logind-session.h | 1 +
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 54db864..099ade6 100644
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -670,6 +670,9 @@ int session_send_create_reply(Session *s, sd_bus_error
*error, bool opening) {
if (fifo_fd < 0)
return fifo_fd;
+/* Clean this up as soon as we have the fifo_fd */
+s->scope_opening = false;
+
/* Update the session state file before we notify the client
* about the result. */
session_save(s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index bec59c0..848e8a1 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -496,6 +496,8 @@ static int session_start_scope(Session *s) {
free(s->scope_job);
s->scope_job = job;
+/* session scope is being created */
+s->scope_opening = true;
}
}
@@ -880,7 +882,7 @@ void session_add_to_gc_queue(Session *s) {
SessionState session_get_state(Session *s) {
assert(s);
-if (s->scope_job)
+if (s->scope_opening)
return SESSION_OPENING;
if (s->fifo_fd < 0)
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index ebe3902..205491a 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -110,6 +110,7 @@ struct Session {
bool in_gc_queue:1;
bool started:1;
+bool scope_opening:1; /* session scope is being created */
sd_bus_message *create_message;
--
1.8.3.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 4/7] logind: close races on user state at logout
To get the state of the user, the user_get_state() is used.
This function will check if the user->slice_job or the user->service_job
are set then it will automatically return USER_OPENING. This is buggy
in the context of user closing:
At logout or D-Bus TerminateUser() calls user_stop()
user_stop() => session_stop() on sessions => session_stop_scope()
=> user_get_state() == USER_CLOSING or USER_ACTIVE or USER_ONLINE
(incorrect)
(depends on session closing execution and if we have finished the
session scope jobs or not, if all the scopes are being queued then
their session->scope_job will be set which means all sessions are in
the OPENING_STATE, so user state will be USER_ONLINE).
The previous patch improves session_get_state() logic, and if scopes
are being queued during logout then sessions are in SESSION_CLOSING
state which makes user_get_state() return USER_CLOSING.
So at user_stop()
user_stop_slice() queues the slice and sets user->slice_job
user_stop_service() queues the service and sets user->service_job
=> user_get_state() == USER_OPENING (incorrect)
After the slice and service jobs are removed the state will be again
correct.
=> user_get_state() == USER_CLOSING (correct)
To fix this and make sure that the state will always be USER_CLOSING
we add a flag that is used to differentiate between USER_OPENING and
USER_CLOSING when 'slice_job' and 'service_job' are set.
The 'slice_opening' flag for 'user->slice_job' and 'service_opening'
for 'user->service_job' are set to true only during real user opening,
later if the service and slice were successfully created their
corresponding 'opening' flag will be set to false, which means that
during user closing they are already false.
Update user_get_state() to check if 'slice_opening' or
'service_opening' are set, if so return USER_OPENING.
---
src/login/logind-dbus.c | 2 ++
src/login/logind-user.c | 6 +-
src/login/logind-user.h | 2 ++
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 0560707..24482fd 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2017,11 +2017,13 @@ int match_job_removed(sd_bus *bus, sd_bus_message
*message, void *userdata, sd_b
/* Clean this up after session_jobs_reply() */
free(user->service_job);
user->service_job = NULL;
+user->service_opening = false;
}
if (streq_ptr(path, user->slice_job)) {
free(user->slice_job);
user->slice_job = NULL;
+user->slice_opening = false;
}
user_save(user);
diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index bdb6915..8183721 100644
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -352,6 +352,8 @@ static int user_start_slice(User *u) {
free(u->slice_job);
u->slice_job = job;
+/* User slice is being created */
+u->slice_opening = true;
}
}
@@ -385,6 +387,8 @@ static int user_start_service(User *u) {
free(u->service_job);
u->service_job = job;
+/* User service is being created */
+u->service_opening = true;
}
}
@@ -637,7 +641,7 @@ UserState user_get_state(User *u) {
assert(u);
-if (u->slice_job || u->service_job)
+if (u->slice_opening || u->service_opening)
return USER_OPENING;
LIST_FOREACH(sessions_by_user, i, u->sessions) {
diff --git a/src/login/logind-user.h b/src/login/logind-user.h
index 0062880..ac43361 100644
--- a/src/login/logind-user.h
+++ b/src/login/logind-user.h
@@ -61,6 +61,8 @@ struct User {
bool in_gc_queue:1;
bool started:1;
+bool service_opening:1; /* User service is being created */
+bool slice_opening:1; /* User slice is being created */
LIST_HEAD(Session, sessions);
LIST_FIELDS(User, gc_queue);
--
1.8.3.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 5/7] logind: just call session_get_state() to get the session state
In function user_get_state() remove the session_is_active() check, just
count on the session_get_state() function to get the correct session
state.
session_is_active() may return true before starting the session scope and
user service, this means it will return true even before the creation of
the session fifo_fd which will produce incorrect states.
Another point is that session_get_state() will check if the fifo_fd was
created before checking if the session is active, this is the correct
behaviour since the session fifo_fd should be considered the point of
session states synchronization.
So be consistent and follow the session_get_state() logic.
---
src/login/logind-user.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index 8183721..9ed216d 100644
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -637,6 +637,7 @@ void user_add_to_gc_queue(User *u) {
UserState user_get_state(User *u) {
Session *i;
+SessionState session_state;
bool all_closing = true;
assert(u);
@@ -645,9 +646,12 @@ UserState user_get_state(User *u) {
return USER_OPENING;
LIST_FOREACH(sessions_by_user, i, u->sessions) {
-if (session_is_active(i))
+/* session_get_state() will check for fifo_fd */
+session_state = session_get_state(i);
+
+if (session_state == SESSION_ACTIVE)
return USER_ACTIVE;
-if (session_get_state(i) != SESSION_CLOSING)
+else if (session_state != SESSION_CLOSING)
all_closing = 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 6/7] logind: add user_is_opening() and session_is_opening()
Add the user_is_opening() and session_is_opening() functions. These
functions will check their appropriate 'opening' flag to see if we are
in the middel of the opening state.
This patch is preparation for the next patch which will use it to guard
match_job_remove() from calling session_jobs_reply() and
session_send_create_reply() during SESSION_CLOSING.
---
src/login/logind-session.c | 8 +++-
src/login/logind-session.h | 1 +
src/login/logind-user.c| 8 +++-
src/login/logind-user.h| 1 +
4 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 848e8a1..215f2b8 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -647,6 +647,12 @@ int session_finalize(Session *s) {
return r;
}
+bool session_is_opening(Session *s) {
+assert(s);
+
+return s->scope_opening;
+}
+
bool session_is_active(Session *s) {
assert(s);
@@ -882,7 +888,7 @@ void session_add_to_gc_queue(Session *s) {
SessionState session_get_state(Session *s) {
assert(s);
-if (s->scope_opening)
+if (session_is_opening(s))
return SESSION_OPENING;
if (s->fifo_fd < 0)
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index 205491a..964306d 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -129,6 +129,7 @@ void session_set_user(Session *s, User *u);
bool session_check_gc(Session *s, bool drop_not_started);
void session_add_to_gc_queue(Session *s);
int session_activate(Session *s);
+bool session_is_opening(Session *s);
bool session_is_active(Session *s);
int session_get_idle_hint(Session *s, dual_timestamp *t);
void session_set_idle_hint(Session *s, bool b);
diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index 9ed216d..cf9c948 100644
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -635,6 +635,12 @@ void user_add_to_gc_queue(User *u) {
u->in_gc_queue = true;
}
+bool user_is_opening(User *u) {
+assert(u);
+
+return u->slice_opening || u->service_opening;
+}
+
UserState user_get_state(User *u) {
Session *i;
SessionState session_state;
@@ -642,7 +648,7 @@ UserState user_get_state(User *u) {
assert(u);
-if (u->slice_opening || u->service_opening)
+if (user_is_opening(u))
return USER_OPENING;
LIST_FOREACH(sessions_by_user, i, u->sessions) {
diff --git a/src/login/logind-user.h b/src/login/logind-user.h
index ac43361..2728168 100644
--- a/src/login/logind-user.h
+++ b/src/login/logind-user.h
@@ -75,6 +75,7 @@ void user_add_to_gc_queue(User *u);
int user_start(User *u);
int user_stop(User *u);
int user_finalize(User *u);
+bool user_is_opening(User *u);
UserState user_get_state(User *u);
int user_get_idle_hint(User *u, dual_timestamp *t);
int user_save(User *u);
--
1.8.3.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 7/7] logind: do not call session_jobs_reply() on CLOSING
match_job_removed() signal is triggered when queued jobs finish during
session opening or closing.
Calling session_jobs_reply() during opening is valid, but during
session closing does not make sense.
The session_send_create_reply() function which is called by
session_jobs_reply() is able to detect if it was not called during
open time by checking the 'session->create_message'. However, making
session_jobs_reply() check session_is_opening() and user_is_opening()
is more comprehensive.
---
src/login/logind-dbus.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 24482fd..4b71d9e 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -1930,6 +1930,10 @@ static int session_jobs_reply(Session *s, const char
*unit, const char *result,
if (!s->started)
return r;
+/* Don't call me if this is not opening state */
+if (!session_is_opening(s) && !user_is_opening(s->user))
+return r;
+
if (streq(result, "done"))
r = session_send_create_reply(s, NULL, opening);
else {
@@ -2010,6 +2014,7 @@ int match_job_removed(sd_bus *bus, sd_bus_message
*message, void *userdata, sd_b
* still being created this will be set to true,
* otherwise it will be false */
active = service_job || !!session->scope_job;
+
session_jobs_reply(session, unit, result, active);
}
--
1.8.3.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] man: cryptsetup now allows partition device file in system mode
On Thu, Feb 6, 2014 at 8:33 PM, Jan Janssen wrote: > --- > man/crypttab.xml | 9 + > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/man/crypttab.xml b/man/crypttab.xml > index 5f386e5..c563851 100644 > --- a/man/crypttab.xml > +++ b/man/crypttab.xml > @@ -305,14 +305,7 @@ > > Use TrueCrypt in system > encryption mode. This implies > -tcrypt. > - > -Please note that when using this mode, > the > -whole device needs to be given in the second > -field instead of the partition. For example: > if > -/dev/sda2 is the system > -encrypted TrueCrypt patition, > /dev/sda > -has to be given. > + tcrypt. > > > When/what release did this change? A reference in the commit message would be useful. Cheers, Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
