[systemd-devel] fstab, rootfs on btrfs
In Fedora 20, by default anaconda sets fs_passno in fstab to 1 for / on btrfs. During offline updates, this is causing systemd-fstab-generator to freak out not finding fsck.btrfs. https://bugzilla.redhat.com/show_bug.cgi?id=1034563 For some time I've been suggesting that fstab should use fs_passno 0 for btrfs file systems: https://bugzilla.redhat.com/show_bug.cgi?id=862871 But because of this suggestion by an XFS dev, I'm wondering if that's not a good idea. Or if we should expect some smarter behavior on the part of systemd (now or in the future) when it comes to devices that take a long time to appear? http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg29231.html It doesn't seem to me that for file systems that don't require an fs check, that fstab should indicate it does require an fs check, just to inhibit hissy fits by other processes not liking that the device is missing. But maybe I'm missing something. Chris Murphy ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] man/systemd-socket-proxyd.xml src/socket-proxy TODO
On Wed, Nov 27, 2013 at 12:03 PM, Lennart Poettering wrote: > Could you please explain what the usecase here is? Why is this better > than having two socket units with two proxy services? Right now, it's because separate services cannot exist in the same network namespace with another service. The main motivation for this change was a desire within CoreOS to run etcd within a network-namespaced service. With the need to only map one port in, that would be equivalent to the network namespaced nginx example. But, they need to map *two* ports in and to distinct destination ports within the namespace. That was actually impossible before, where socket-proxyd would map all inherited sockets to the same destination. This change allows forcing a single socket-proxyd to inherit only one socket each. >> +/usr/bin/systemd-socket-proxyd --listener=3 localhost:8080 & >> +/usr/bin/systemd-socket-proxyd --listener=4 localhost:8443 & >> +wait]]> > > If our examples suggest that people should write shell scripts that fork > things, then this is usually an indication that we need to rethink what > we are doing here. In fact, running multiple manually forked off > processes inside the same service already sounds very wrong. We should > try to keep a 1:1 mapping between processes we fork and services they > run it. Agreed. I really don't want socket-proxyd directly involved in exec'ing of things, either. > What's the usecase here? > > If this is about running multiple things in the same Network namespace, > then there are certainly other, better ways to achieve the same. For > example, we could introduce JoinPrivateNetwork=$SERVICE or so which > would allow one service to join the network namespace of another. That > makes this much smoother and more powerful, too. That would be wonderful. I'd love to shed the shell scripts. > With that in place you could simply have three proxy instances, and make > them join the private network of your nginx instance and you should be > set? I think that would do exactly the trick. I didn't know it was possible to join into an existing namespace that way. It would be my pleasure to update the documentation to use something like that if it becomes available. I'd also enjoy dropping the --listener option. :-) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core/socket: we only want one event on standard sockets
On Wed, Nov 27, 2013 at 7:57 AM, Shawn Landden wrote: > Are you sure applications can handle the extra file descriptor of > passing both the sockfd > and the acceptfd in this case? I don't see why they wouldn't just do > the accept() themselves? > > Can you explain what you mean here, and how it differs from the > existing MaxConnections. Actually, MaxConnections was exactly what I was thinking of. I agree with you now on Distribute=true only being useful with Accept=false. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] man/systemd-socket-proxyd.xml src/socket-proxy TODO
On Sun, 24.11.13 16:47, David Strauss (strau...@kemper.freedesktop.org) wrote: > TODO |1 > man/systemd-socket-proxyd.xml| 80 > ++- > src/socket-proxy/socket-proxyd.c | 63 -- > 3 files changed, 121 insertions(+), 23 deletions(-) > > New commits: > commit adcf4c81c58511b67644e17fa743d1729d3c9ccf > Author: David Strauss > Date: Mon Nov 25 10:44:48 2013 +1000 > > socket-proxyd: Add --listener option for listener/destination > pairs. Could you please explain what the usecase here is? Why is this better than having two socket units with two proxy services? > +/usr/bin/systemd-socket-proxyd --listener=3 localhost:8080 & > +/usr/bin/systemd-socket-proxyd --listener=4 localhost:8443 & > +wait]]> If our examples suggest that people should write shell scripts that fork things, then this is usually an indication that we need to rethink what we are doing here. In fact, running multiple manually forked off processes inside the same service already sounds very wrong. We should try to keep a 1:1 mapping between processes we fork and services they run it. What's the usecase here? If this is about running multiple things in the same Network namespace, then there are certainly other, better ways to achieve the same. For example, we could introduce JoinPrivateNetwork=$SERVICE or so which would allow one service to join the network namespace of another. That makes this much smoother and more powerful, too. With that in place you could simply have three proxy instances, and make them join the private network of your nginx instance and you should be set? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core/socket: we only want one event on standard sockets
On Wed, 27.11.13 07:48, David Timothy Strauss (da...@davidstrauss.net) wrote: > > On Wed, Nov 27, 2013 at 6:23 AM, Shawn Landden wrote: > > I was worried that the fact that we never accept() the socket when using > > distribute (now I am convinced we shouldn't use it otherwise) > > I'm not sure what you mean here. Distribute-style functionality is > absolutely useful with Accept=true (to cap the number of simultaneous > connections) as well as Accept=false (to allow running of a process > pool of self-accepting, long-running workers). We already enforce a connection limit (MaxConnections=) for Accept=yes sockets... We did this since day one. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core/socket: we only want one event on standard sockets
On Tue, Nov 26, 2013 at 1:48 PM, David Timothy Strauss wrote: > On Wed, Nov 27, 2013 at 6:23 AM, Shawn Landden wrote: >> I was worried that the fact that we never accept() the socket when using >> distribute (now I am convinced we shouldn't use it otherwise) > > I'm not sure what you mean here. Distribute-style functionality is > absolutely useful with Accept=true (to cap the number of simultaneous > connections) Are you sure applications can handle the extra file descriptor of passing both the sockfd and the acceptfd in this case? I don't see why they wouldn't just do the accept() themselves? Can you explain what you mean here, and how it differs from the existing MaxConnections. > as well as Accept=false (to allow running of a process > pool of self-accepting, long-running workers). > >> would cause it to trigger multiple times for only one incoming connection, >> if the spawned thread never entered accept() (or we raced it), >> but reading this thread makes me think I don't fully understand the >> semantics of EPOLLET. > > There are some decent examples on the man page: > http://linux.die.net/man/7/epoll Yeah I had read that, and seen it in the kernel source. I am just confused and need to think about it some more. -Shawn ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core/socket: we only want one event on standard sockets
On Wed, Nov 27, 2013 at 6:23 AM, Shawn Landden wrote: > I was worried that the fact that we never accept() the socket when using > distribute (now I am convinced we shouldn't use it otherwise) I'm not sure what you mean here. Distribute-style functionality is absolutely useful with Accept=true (to cap the number of simultaneous connections) as well as Accept=false (to allow running of a process pool of self-accepting, long-running workers). > would cause it to trigger multiple times for only one incoming connection, > if the spawned thread never entered accept() (or we raced it), > but reading this thread makes me think I don't fully understand the > semantics of EPOLLET. There are some decent examples on the man page: http://linux.die.net/man/7/epoll ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core/socket: we only want one event on standard sockets
As we only reciece one event at a time, and dequeue it in the same go, yeah ONESHOT won't change anything. On Tue, Nov 26, 2013 at 9:33 AM, David Timothy Strauss wrote: > On Wed, Nov 27, 2013 at 2:32 AM, Lennart Poettering > wrote: >> Well, but EPOLLET only works correctly if each time an event is >> triggered we dispatch *all* possibly queued events on the fd, until >> EAGAIN is read again. But we don't do that, heck, if Listen=no is used >> we don''t even read a single incoming connection of the scket, we leave >> that to the daemon we spawn, but we cannot trust that. So, I don't get >> what EPOLLET is supposed to do good here? > > I should have more selectively quoted before answering. I was only > trying to answer why we'd continue having poll for the socket fds > after queuing the initial service. I was not trying to suggest EPOLLET > is appropriate for the goal of lazy "distribute" process pool > expansion. You've clearly correct about that. I was worried that the fact that we never accept() the socket when using distribute (now I am convinced we shouldn't use it otherwise) would cause it to trigger multiple times for only one incoming connection, if the spawned thread never entered accept() (or we raced it), but reading this thread makes me think I don't fully understand the semantics of EPOLLET. > ___ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core/socket: we only want one event on standard sockets
On Wed, Nov 27, 2013 at 2:32 AM, Lennart Poettering wrote: > Well, but EPOLLET only works correctly if each time an event is > triggered we dispatch *all* possibly queued events on the fd, until > EAGAIN is read again. But we don't do that, heck, if Listen=no is used > we don''t even read a single incoming connection of the scket, we leave > that to the daemon we spawn, but we cannot trust that. So, I don't get > what EPOLLET is supposed to do good here? I should have more selectively quoted before answering. I was only trying to answer why we'd continue having poll for the socket fds after queuing the initial service. I was not trying to suggest EPOLLET is appropriate for the goal of lazy "distribute" process pool expansion. You've clearly correct about that. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC][PATCH] conf-parser: distinguish between multiple sections with the same name
On Thu, 21.11.13 02:27, Tom Gundersen (t...@jklm.no) wrote: > > Maybe then back to labelled sections: > > > > [Address:foobar] > > Label=waldo > > Address=1.1.1.1 > > > > or so, so that the suffix "foobar" is purely an id that is by default > > disconnected from any setting? And then maybe optionally inherit it into > > Label= if Label= is not explicitly set? > > I think the section name should not have any other function/meaning. I > first tried to make 'section name'='Label', but I'm worried it might > be confusing: If section names are required, it means we now require > labeling, or force the admin to set "Label=" to disable it, which > seems a bit weird. It hink that would actually be OK. Doing something useful without explicit configuration sounds like a good approach. Inheriting the Label= from the section name if no explicit Label= is specified sounds quite OK to me... > Also, I find it asymmetric the way section names > for Addresses have this extra meaning, but for Routes or other types > of sections where there is no natural equivalent to Label=, they have > no meaning apart from a name. Well, we wouldn't really bind the section name only and exclusively to the label. It's just that when fields are not explicitly set they might inherit the section name, in some field-specific way. For Label= it would be quiet obvious, but for other things this might work too. For example, for the ipv6 tunnel stuff we could inherit the section name into the tunnel iface name or so, whatever comes up... > I'm really not convinced one way or the other, but I think my > preferred solution is: go with unnamed sections now, and if ever it > becomes necessary, introduce named ones additionally. Sounds OK. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
On Tue, 26.11.13 09:53, Colin Guthrie (gm...@colin.guthr.ie) wrote: > >> I'm proposing a simple goal: XDG_RUNTIME_DIR should always be that > >> matching the current uid. I can't think of any case where you'd > >> want it otherwise. > > > > That can't work. As the directory only exists when a real login session > > is around. su/sudo don't get their own login sessins, hence the dir > > doesn't necessarily exist and from the perspective of the code running > > in su/sudo the lifetime semantics of the dir wouldn't match any > > expections... > > Colin W's later patch did implement these semantics for the root user's > XDG_RUNTIME_DIR. It kept it around and didn't tidy it up. Doesn't this > solve the problem for the root user nicely (which is the primary > problem)? I am pretty sure we shouldn't exclude the root user's XDG_RUNTIME_DIR from the usual clean-up logic. Also, as mentioned, if you want su-l/sudo-i work, then please add the "force-new-session=1" switch to pam_systemd, that I offered to merge. > But regardless, why doesn't this code create the dirs if they don't > exist anyway? Sure, this doesn't deal with lifetime problem (i.e. > knowing when to delete the dirs again) which is highlighted when su'ing > to another non-root user, but I think there was some talk in this thread > of adding some kind of refcounting here to make this behave better. We should avoid creating dirs we don't know are cleaned up again. logind is the component that cleans up those dirs, hence it should also create them. > Could logind not learn to track these "nested-mini-sessions"? They all That's what "force-new-session=1" would do. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
On Tue, 26.11.13 07:19, Martin Pitt (martin.p...@ubuntu.com) wrote: Heya, > Lennart Poettering [2013-11-26 5:17 +0100]: > > That can't work. As the directory only exists when a real login session > > is around. su/sudo don't get their own login sessins, hence the dir > > doesn't necessarily exist and from the perspective of the code running > > in su/sudo the lifetime semantics of the dir wouldn't match any > > expections... > > Right, as long as they don't actually get one. I (and I think Colin) > argued that "su -"/"pkexec" should (just like ssh localhost), as they > run a full PAM stack which is like logging in. But let's agree to > disagree at this point. Disagree? I already offered that we add a new switch to pam_logind which is only specified in the su-l/sudo-i PAM snippets which tells logind to create an entirely new session, ignoring that the calling su is already part of an existing session. Let's say we call it "force-new-session=1" or so on the pam_systemd configuration like, and we set that for /etc/pam.d/su-l and /etc/pam.d/sudo-i but not in /etc/pam.d/su and /etc/pam.d/sudo. (Yes, Debian/Ubuntu would have to gain separate config snippet for the two cases to make this work, the way Fedora already has). Then, doing "su -" would basically get you an entirely new session, only access to the TTY you'd keep. Doing this correctly is not entirely trivial though as these new "limited sessions" should list the original tty device as their tty, but probably not the seat/vtnr info, since they still should never get access to any of the seats hardware... Again, the offer stands: I'd be willing to merge such a patch. That way the story is somewhat clear: a) by default audit and logind session ids match b) su/sudo will inherit almost all state from the parent with the exception of uid/gis, and get XDG_RUNTIME_DIR unset in order not to fuck things up c) su-l/sudo-i will get an almost entirely new session with no access to hardware but with access to the original TTY, and that's it. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] core/socket: we only want one event on standard sockets
On Tue, 26.11.13 14:44, David Timothy Strauss (da...@davidstrauss.net) wrote: > > On Tue, Nov 26, 2013 at 2:35 PM, Lennart Poettering > wrote: > > Not following here. What precisely does this fix, can you elaborate? > > > > We currently turn off the poll for the socket fds as soon as we queued > > the service socket, so that we don't get woken up anymore. What would > > EPOLLET do good here? > > I think he's been working on having the "distribute" pool scale up > on-demand, which would involve systemd getting events on new > connections coming into the listener socket. More specifically, I > think he intends to, as each new connection comes in, check if we've > maxed out the number of processes. If not, spin another one up. > Presumably, we'd drop the listener once the max-size pool is running. Well, but EPOLLET only works correctly if each time an event is triggered we dispatch *all* possibly queued events on the fd, until EAGAIN is read again. But we don't do that, heck, if Listen=no is used we don''t even read a single incoming connection of the scket, we leave that to the daemon we spawn, but we cannot trust that. So, I don't get what EPOLLET is supposed to do good here? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
On Tue, 2013-11-26 at 09:53 +, Colin Guthrie wrote: > Colin W's later patch did implement these semantics for the root user's > XDG_RUNTIME_DIR. It kept it around and didn't tidy it up. Doesn't this > solve the problem for the root user nicely (which is the primary problem)? Yes, I run "pkexec" to root every day, and would like if code that ran as root could just assume it was there. I may look at rebasing my patch on top of master again as a followup. > But regardless, why doesn't this code create the dirs if they don't > exist anyway? Sure, this doesn't deal with lifetime problem (i.e. > knowing when to delete the dirs again) Because the lifetime problem is important; daemons and the like use it to implement "run only once" semantics. At least if it's unset your daemon can say "Sorry" and abort. If we cleaned it up while the daemon was running, you could get two, and that could lead to data loss. > Could logind not learn to track these "nested-mini-sessions"? I think so, but: > Perhaps it is a problem tracking when one of these nested sessions > actually logs out? Or perhaps the a problem would be doing something > like su[mini-session]+screen+detatch+logout[mini-session]? The > mini-session would be closed and the runtime dir cleaned up even tho' > processes in screen were still needing it. Exactly. If we were going to claim to provide XDG_RUNTIME_DIR in these types of scenarios, we need to avoid it being cleaned up too early. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
On Tue, Nov 26, 2013 at 02:39:49PM +, Colin Guthrie wrote: > 'Twas brillig, and Dr. Werner Fink at 26/11/13 14:21 did gyre and gimble: > > On Tue, Nov 26, 2013 at 10:41:36AM +, Colin Guthrie wrote: > >> 'Twas brillig, and Martin Pitt at 26/11/13 06:19 did gyre and gimble: > >>> Hey Lennart, > >>> > >>> Lennart Poettering [2013-11-26 5:12 +0100]: > I implemented this now, using a different approach than Martin's > original patch (i.e. I don't think it is a good idea to involve stat() > here, instead let's just let logind pass all information to > pam_systemd). > >>> > >>> Thanks! > >> > >> Indeed, thanks for this! > >> > >> If anyone backports this fix to v208 (i.e. pre sd-bus) please share it > >> here. I'll likely do it just to have the "upstream-blessed" fix, but > >> doubt I'll get around it it until later in the week. > > > > I've backported it. > > Can you link to it or attach it please? Yep ... > > But during tests I've found that it does not help > > if the environment variable XDG_RUNTIME_DIR already exists before doing > > su. It will not unset but exported. > > That's expected. > > su does not do any env cleaning, su - does, sudo does, pkexec does. I'm aware ;) neverthelesss ... > > su's behaviour is to always not touch stuff and thus this is known and > expected and has always been a problem. > > Longer term we need to solve this more holistically (hence why I've > tried to get information about "how things should be done" in the future > to start helping lay the ground work for thise bits), but this is still > the best fix we have just now. > > For the specific case of su'ing to root, (which is the most common and > potentially problematic one), I will probably use Colin W's most recent > patch to have a static root runtime dir and for logind to set it. This > should fix XDG_RUNTIME_DIR when su'ing to root. I'm not so worried about > su'ing to other users (the damage that can be done is much more > limited), but longer term we do need to address that nicely too IMO > (which will likely need changes in su itself and a number of other places) ... I've a bug report here which indeed shows exactly the ``other'' problem. If you do not like the enhancement in my backport you may replace +} else { +(void) unsetenv("XDG_RUNTIME_DIR"); +r = pam_putenv(handle, "XDG_RUNTIME_DIR"); +if (r != PAM_SUCCESS && r != PAM_BAD_ITEM) { + pam_syslog(handle, LOG_ERR, "Failed to unset runtime dir."); +} with `}' ... otherwise both the pam_putenv() and unsetenv() are required to make sure that XDG_RUNTIME_DIR is removed. Werner -- "Having a smoking section in a restaurant is like having a peeing section in a swimming pool." -- Edward Burr Based on upstream baae0358f349870544884e405e82e4be7d8add9f | From: Lennart Poettering | Date: Tue, 26 Nov 2013 04:05:00 + | Subject: pam_systemd: do not set XDG_RUNTIME_DIR if the session's original user is not the same as the newly logged in one | It's better not to set any XDG_RUNTIME_DIR at all rather than one of a | different user. So let's do this. --- systemd-208/src/login/logind-dbus.c +++ systemd-208/src/login/logind-dbus.c 2013-11-26 13:37:05.730735774 + @@ -523,6 +523,7 @@ static int bus_manager_create_session(Ma DBUS_TYPE_OBJECT_PATH, &path, DBUS_TYPE_STRING, &session->user->runtime_path, DBUS_TYPE_UNIX_FD, &fifo_fd, +DBUS_TYPE_UINT32, &session->user->uid, DBUS_TYPE_STRING, &cseat, DBUS_TYPE_UINT32, &vtnr, DBUS_TYPE_BOOLEAN, &exists, --- systemd-208/src/login/logind-session-dbus.c +++ systemd-208/src/login/logind-session-dbus.c 2013-11-26 13:36:07.478236401 + @@ -755,6 +755,7 @@ int session_send_create_reply(Session *s DBUS_TYPE_OBJECT_PATH, &path, DBUS_TYPE_STRING, &s->user->runtime_path, DBUS_TYPE_UNIX_FD, &fifo_fd, +DBUS_TYPE_UINT32, &s->user->uid, DBUS_TYPE_STRING, &cseat, DBUS_TYPE_UINT32, &vtnr, DBUS_TYPE_BOOLEAN, &exists, --- systemd-208/src/login/pam-module.c +++ systemd-208/src/login/pam-module.c 2013-11-26 14:32:20.194235777 + @@ -93,24 +93,18 @@ static int get_user_data( assert(ret_username); assert(ret_pw); -r = audit_loginuid_from_pid(0, &uid); -if (r >= 0) -pw = pam_modutil_getpwuid(handle, uid); -else { -r = pam_get_user(handle, &username, NULL); -if (r != PAM_SUCCESS) { -pam_syslog(handle, LOG_E
Re: [systemd-devel] Question regarding the NotifyAccess parameter
> >One more issue I observed is - if I specify Restart=on-failure, if > > watchdog timer expire, it restart the service. But I can see that it > > create two processes rather than restarting the process. But if I do > > systemctl restart Myservice , it kills the previous instance of > > service and start a new service. Any pointers on why it happens so. This part has been already reported as a bug in May: http://lists.freedesktop.org/archives/systemd-devel/2013-May/011030.html Best to my knowledge, this has been fixed in systemd 203, 204, or 205 ... Please note that the link above does not contain the final bug fix. Some discussions followed which led to the final solution at a certain point. Follow the threads, you'll find it ... Best regards Marko Hoyer Software Group II (ADITG/SW2) Tel. +49 5121 49 6948 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
'Twas brillig, and Dr. Werner Fink at 26/11/13 14:21 did gyre and gimble: > On Tue, Nov 26, 2013 at 10:41:36AM +, Colin Guthrie wrote: >> 'Twas brillig, and Martin Pitt at 26/11/13 06:19 did gyre and gimble: >>> Hey Lennart, >>> >>> Lennart Poettering [2013-11-26 5:12 +0100]: I implemented this now, using a different approach than Martin's original patch (i.e. I don't think it is a good idea to involve stat() here, instead let's just let logind pass all information to pam_systemd). >>> >>> Thanks! >> >> Indeed, thanks for this! >> >> If anyone backports this fix to v208 (i.e. pre sd-bus) please share it >> here. I'll likely do it just to have the "upstream-blessed" fix, but >> doubt I'll get around it it until later in the week. > > I've backported it. Can you link to it or attach it please? > But during tests I've found that it does not help > if the environment variable XDG_RUNTIME_DIR already exists before doing > su. It will not unset but exported. That's expected. su does not do any env cleaning, su - does, sudo does, pkexec does. su's behaviour is to always not touch stuff and thus this is known and expected and has always been a problem. Longer term we need to solve this more holistically (hence why I've tried to get information about "how things should be done" in the future to start helping lay the ground work for thise bits), but this is still the best fix we have just now. For the specific case of su'ing to root, (which is the most common and potentially problematic one), I will probably use Colin W's most recent patch to have a static root runtime dir and for logind to set it. This should fix XDG_RUNTIME_DIR when su'ing to root. I'm not so worried about su'ing to other users (the damage that can be done is much more limited), but longer term we do need to address that nicely too IMO (which will likely need changes in su itself and a number of other places) Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Question regarding the NotifyAccess parameter
On Thu, Nov 21, 2013 at 08:56:25PM +0530, salil GK wrote: > Hello > >I have a service in shell script in which I am sending notification to > systemd using `systemd-notify WATCHDOG=1` command. What happens is - > systemd-notify will be a child process and in the systemd notification will > not be honoured if NotifyAccess is set to main. Is there any work around > for this. Use NotifyAccess=all? >One more issue I observed is - if I specify Restart=on-failure, if > watchdog timer expire, it restart the service. But I can see that it create > two processes rather than restarting the process. But if I do systemctl > restart Myservice , it kills the previous instance of service and start a > new service. Any pointers on why it happens so. That would be a significant bug! Can you post a short example which shows the bug? Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
On Tue, Nov 26, 2013 at 10:41:36AM +, Colin Guthrie wrote: > 'Twas brillig, and Martin Pitt at 26/11/13 06:19 did gyre and gimble: > > Hey Lennart, > > > > Lennart Poettering [2013-11-26 5:12 +0100]: > >> I implemented this now, using a different approach than Martin's > >> original patch (i.e. I don't think it is a good idea to involve stat() > >> here, instead let's just let logind pass all information to > >> pam_systemd). > > > > Thanks! > > Indeed, thanks for this! > > If anyone backports this fix to v208 (i.e. pre sd-bus) please share it > here. I'll likely do it just to have the "upstream-blessed" fix, but > doubt I'll get around it it until later in the week. I've backported it. But during tests I've found that it does not help if the environment variable XDG_RUNTIME_DIR already exists before doing su. It will not unset but exported. Werner -- "Having a smoking section in a restaurant is like having a peeing section in a swimming pool." -- Edward Burr ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd 208:trouble with inactive user sessions at non-seat0 seats
David, Looking at GDM debug and gdm-simple-slave.c source file, as well as "loginctl show-seat" output. I guess that GDM only requests user session activation for seats with CanMultiSession=yes, but currently systemd-logind still sets CanMultiSession=no for my non-seat0 seats. Does seat_can_multi_session() heuristics need some improvement since generic multi-session was introduced? I've applied your last patch. I confirm it solves my problem for now. Thanks for all, and keep the good work! CANTATE DOMINO CANTICUM NOVUM QUIA MIRABILIA FECIT Laércio 2013/11/26 David Herrmann > Hi > > On Tue, Nov 26, 2013 at 11:45 AM, Laércio de Sousa > wrote: > > Hi David! > > > > I just tested your patch, and unfortunately it didn't work. However, I > guess > > what could be the question. > > > > If I understand correctly, your patch applies for the case we have no > active > > session at all in a non-seat0 seat. However, I do have an active session > in > > my non-seat0 seat --- the one GDM opens to show the greeter. The > question is > > that, when a user logs in at a non-seat0 seat, GDM (or whatever) should > > activate automatically the new user session, sending the greeter session > to > > the background while it's in state "closing", as we have for seat0. In my > > current setup, it doesn't occur: in a non-seat0 seat, the "closing" > greeter > > session remains active, while the new user session starts at background > > (inactive). > > > > If I activate manually the user session (loginctl activate ), > the > > converse occurs: when the user logs out, its "closing" user session > remains > > active, while the new greeter session starts at background (inactive). > > > > Just for comparison: if I configure autologin for GDM (to avoid opening > the > > greeter session), or use multiseat-patched KDM (which doesn't open > greeter > > sessions), the user session starts at foreground, as expected, even > without > > this patch. > > That gdm behavior is actually weird. It should explicitly request the > new session to become active. There is no reason for logind to > *assume* the new session should be activated automatically.. hmm. > > The appended patch reverts the new behavior. Can you give it a try? I > actually cannot get gdm to pick up my additional seats.. and looking > into the monstrosity called gdm-source-base I have no clue what it > does. If you can confirm that this resets the behavior, I will apply > it so we don't break existing setups. > > I will figure out something else for new multi-session capable seats. > > Thanks > David > > > diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c > index b30c4ce..ca0e8d7 100644 > --- a/src/login/logind-seat.c > +++ b/src/login/logind-seat.c > @@ -413,8 +413,8 @@ int seat_attach_session(Seat *s, Session *session) { > seat_send_changed(s, "Sessions", NULL); > > /* On seats with VTs, the VT logic defines which session is > active. On > - * seats without VTs, we automatically activate the first > session. */ > -if (!seat_has_vts(s) && !s->active) > + * seats without VTs, we automatically activate new sessions. */ > +if (!seat_has_vts(s)) > seat_set_active(s, session); > > return 0; > ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] tree-wide usage of %m specifier instead of strerror(errno)
Thanks. Applied. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
'Twas brillig, and Martin Pitt at 26/11/13 06:19 did gyre and gimble: > Hey Lennart, > > Lennart Poettering [2013-11-26 5:12 +0100]: >> I implemented this now, using a different approach than Martin's >> original patch (i.e. I don't think it is a good idea to involve stat() >> here, instead let's just let logind pass all information to >> pam_systemd). > > Thanks! Indeed, thanks for this! If anyone backports this fix to v208 (i.e. pre sd-bus) please share it here. I'll likely do it just to have the "upstream-blessed" fix, but doubt I'll get around it it until later in the week. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] pam: Don't use loginuid [was: Re: Fix PAM module to not clobber XDG_RUNTIME_DIR with su]
'Twas brillig, and Lennart Poettering at 26/11/13 04:17 did gyre and gimble: > On Wed, 20.11.13 19:19, Colin Walters (walt...@verbum.org) wrote: > >>> So yeah, there your mix >>> and match is broken: >> >> I'm proposing a simple goal: XDG_RUNTIME_DIR should always be that >> matching the current uid. I can't think of any case where you'd >> want it otherwise. > > That can't work. As the directory only exists when a real login session > is around. su/sudo don't get their own login sessins, hence the dir > doesn't necessarily exist and from the perspective of the code running > in su/sudo the lifetime semantics of the dir wouldn't match any > expections... Colin W's later patch did implement these semantics for the root user's XDG_RUNTIME_DIR. It kept it around and didn't tidy it up. Doesn't this solve the problem for the root user nicely (which is the primary problem)? [The rest of this mail is more "asking daft questions" than any kind of complaint. I'm still struggling to see the problems of some proposed solutions] But regardless, why doesn't this code create the dirs if they don't exist anyway? Sure, this doesn't deal with lifetime problem (i.e. knowing when to delete the dirs again) which is highlighted when su'ing to another non-root user, but I think there was some talk in this thread of adding some kind of refcounting here to make this behave better. Could logind not learn to track these "nested-mini-sessions"? They all seem to go through the pam stack so what is the fundamental problem in adding some kind of tracking to them? The statement that "As the directory only exists when a real login session is around" appears to be a self-imposed limitation inside logind. Surely we just create the folder as needed - even on a su, track this mini-session. We don't need to expose it as a real session - nor really track the processes specifically (this isn't done now anyway), but just use it internally in any "how many sessions are there for this user" checks when nuking the runtime dir. This would solve the lifetime issue so it's possible to tidy up the runtime dirs properly only when the last user really does disappear. I'm sure I'm missing something but all this just seems to be being made more complicated than is necessary due to artificial problems that are just a product of the current implementation! Is this a route forward or is there a fundamental problem with that? I appreciate that not making it a proper session and tracking the processes properly would maybe feel nasty, but I'd still say it was a step in the right direction. Perhaps it is a problem tracking when one of these nested sessions actually logs out? Or perhaps the a problem would be doing something like su[mini-session]+screen+detatch+logout[mini-session]? The mini-session would be closed and the runtime dir cleaned up even tho' processes in screen were still needing it. If this is a problem case does screen need to be capable of registering it's own session (and I'd say it needs to be a full session here such that it doesn't really die when the containing session logs out). Would logind have to learn a new mode for creating detachable sessions for things like screen or would screen have to become setuid or something equally ugly here? I'd really appreciate it if someone who groks all this stuff could write up a "how it should be done" wiki page or something explaining what the "best practices" would be to approach solutions to: 1. starting a text shell as another user (either within a graphical env or not (and whether to run GUI apps and play sound as the other user or not). 2. starting a new, detachable session e.g. screen as the current user (possible done *after* 1. above). These are things people do and I think there are legitimate reasons for doing them - regardless, even if some of the cases only have illegitimate reasons, we need to be able to fail gracefully and, ideally, present a warning to the user. Making a stance and not supporting certain (stupid) setups only works when you actually *tell* the user that they can't do something and explain why and what the alternative is. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] tree-wide usage of %m specifier instead of strerror(errno)
Also for log_error() except where a specific error is specified e.g. errno ? strerror(errno) : "Some user specified message" --- TODO | 2 -- src/core/automount.c | 2 +- src/core/main.c| 2 +- src/core/manager.c | 2 +- src/core/mount-setup.c | 2 +- src/core/service.c | 4 ++-- src/initctl/initctl.c | 12 +--- src/journal/coredumpctl.c | 2 +- src/journal/journald-console.c | 4 ++-- src/journal/journald-kmsg.c| 2 +- src/udev/collect/collect.c | 6 +++--- src/udev/scsi_id/scsi_id.c | 2 +- src/udev/scsi_id/scsi_serial.c | 8 +++- src/udev/udev-rules.c | 2 +- 14 files changed, 23 insertions(+), 29 deletions(-) diff --git a/TODO b/TODO index d63e13e..653258a 100644 --- a/TODO +++ b/TODO @@ -824,8 +824,6 @@ Regularly: * Use PR_SET_PROCTITLE_AREA if it becomes available in the kernel -* %m in printf() instead of strerror(errno); - * pahole * set_put(), hashmap_put() return values check. i.e. == 0 doesn't free()! diff --git a/src/core/automount.c b/src/core/automount.c index 49a64b1..66e3d78 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -304,7 +304,7 @@ static int open_dev_autofs(Manager *m) { m->dev_autofs_fd = open("/dev/autofs", O_CLOEXEC|O_RDONLY); if (m->dev_autofs_fd < 0) { -log_error("Failed to open /dev/autofs: %s", strerror(errno)); +log_error("Failed to open /dev/autofs: %m"); return -errno; } diff --git a/src/core/main.c b/src/core/main.c index dbc98db..69d3a43 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -131,7 +131,7 @@ _noreturn_ static void crash(int sig) { pid = fork(); if (pid < 0) -log_error("Caught <%s>, cannot fork for core dump: %s", signal_to_string(sig), strerror(errno)); +log_error("Caught <%s>, cannot fork for core dump: %m", signal_to_string(sig)); else if (pid == 0) { struct rlimit rl = {}; diff --git a/src/core/manager.c b/src/core/manager.c index aa4baaa..65cb73c 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -307,7 +307,7 @@ static int enable_special_signals(Manager *m) { } else { /* Enable that we get SIGWINCH on kbrequest */ if (ioctl(fd, KDSIGACCEPT, SIGWINCH) < 0) -log_warning("Failed to enable kbrequest handling: %s", strerror(errno)); +log_warning("Failed to enable kbrequest handling: %m"); } return 0; diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c index 73c2698..c601c97 100644 --- a/src/core/mount-setup.c +++ b/src/core/mount-setup.c @@ -188,7 +188,7 @@ static int mount_one(const MountPoint *p, bool relabel) { p->type, p->flags, p->options) < 0) { -log_full((p->mode & MNT_FATAL) ? LOG_ERR : LOG_DEBUG, "Failed to mount %s: %s", p->where, strerror(errno)); +log_full((p->mode & MNT_FATAL) ? LOG_ERR : LOG_DEBUG, "Failed to mount %s: %m", p->where); return (p->mode & MNT_FATAL) ? -errno : 0; } diff --git a/src/core/service.c b/src/core/service.c index 28b1465..7c5d5d8 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3507,7 +3507,7 @@ static int service_enumerate(Manager *m) { d = opendir(path); if (!d) { if (errno != ENOENT) -log_warning("opendir(%s) failed: %s", path, strerror(errno)); +log_warning("opendir(%s) failed: %m", path); continue; } @@ -3540,7 +3540,7 @@ static int service_enumerate(Manager *m) { if (access(fpath, X_OK) < 0) { if (errno != ENOENT) -log_warning("access() failed on %s: %s", fpath, strerror(errno)); +log_warning("access() failed on %s: %m", fpath); continue; } diff --git a/src/initctl/initctl.c b/src/initctl/initctl.c index d6c..284319f 100644 --- a/src/initctl/initctl.c +++ b/src/initctl/initctl.c @@ -217,7 +217,7 @@ static int fifo_process(Fifo *f) { if (errno == EAGAIN) return 0; -log_warning("Failed to read from fifo: %s", strerror(errno)); +log_warning("Failed to read from fifo: %m"); return -1; } @@ -278,7 +278,7 @@ static int server_init(Server *s, unsigned n_sockets) { s->epoll_fd = epoll_create1(EPOLL_CLOEXEC);