[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#775265: unblock: systemd/215-10



control: retitle  -1 unblock: systemd/215-10
control: tags -1 - moreinfo

Am 15.01.2015 um 07:26 schrieb Niels Thykier:
> Control: tags -1 -d-i +moreinfo
> 
> On 2015-01-15 01:25, Michael Biebl wrote:
>> Am 13.01.2015 um 17:04 schrieb Niels Thykier:
>>> [...]
>> A user reported a nasty regression via IRC regarding this patch.
>> For SysV init scripts having a .sh extension, we create a foo.service ->
>> foo.service symlink, and subsequently, systemctl start/stop/restart
>> foo.service will fail:
>>
>> [...]
>>
>>
>> Will need to fix that in a follow-up upload.
>>
>> Sorry for this.
>>
> 
> Thanks for the warning! :)
> 
> I have retracted my hint.  Please remove the moreinfo when you have a
> revised version.


Martin uploaded -10 in the meantime, fixing this issue (#775889,
#775404), along with two other fixes:


>   * Fix journal forwarding to syslog in containers without CAP_SYS_ADMIN.
>     (Closes: #775067)

>   * Use common-session-noninteractive in systemd-user's PAM config, instead of
>     common-session. The latter can include PAM modules like libpam-mount which
>     expect to be called just once and/or interactively, which already happens
>     for login, ssh, or the display-manager. Add pam_systemd.so explicitly, as
>     it's not included in -noninteractive, but is always required (and
>     idempotent). There is no net change on systemd which don't use manually
>     installed PAM modules. (Closes: #739676)


It's been in unstable for a few days without any reports so far
regarding regressions.

Therefor, I feel comfortable to see 215-10 unblocked and I've retitled
the bug report accordingly.

As a heads up, it's very likely, that we have one or two more uploads
before the release.
That said, it's definitely better, to have 215-10 in testing now, then
waiting for those upcoming uploads.


The debdiff between 215-9 and 215-10 is attached. For individiual
changesets, please see [1]



Regards,
Michael


[1] http://anonscm.debian.org/cgit/pkg-systemd/systemd.git/log/
-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
diff --git a/debian/changelog b/debian/changelog
index 61ef555..d1c734d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,25 @@
+systemd (215-10) unstable; urgency=medium
+
+  [ Martin Pitt ]
+  * sysv-generator: Handle .sh suffixes when translating Provides:.
+    (Closes: #775889)
+  * sysv-generator: Make real units overwrite symlinks generated by Provides:
+    from other units. Fixes failures due to presence of backup or old init.d
+    scripts. (Closes: #775404)
+  * Fix journal forwarding to syslog in containers without CAP_SYS_ADMIN.
+    (Closes: #775067)
+
+  [ Christian Kastner ]
+  * Use common-session-noninteractive in systemd-user's PAM config, instead of
+    common-session. The latter can include PAM modules like libpam-mount which
+    expect to be called just once and/or interactively, which already happens
+    for login, ssh, or the display-manager. Add pam_systemd.so explicitly, as
+    it's not included in -noninteractive, but is always required (and
+    idempotent). There is no net change on systemd which don't use manually
+    installed PAM modules. (Closes: #739676)
+
+ -- Martin Pitt <mpitt@debian.org>  Wed, 21 Jan 2015 13:18:05 +0100
+
 systemd (215-9) unstable; urgency=medium
 
   [ Didier Roche ]
diff --git a/debian/patches/Adjust-systemd-user-pam-config-file-for-Debian.patch b/debian/patches/Adjust-systemd-user-pam-config-file-for-Debian.patch
index 78c5e0c..11336bb 100644
--- a/debian/patches/Adjust-systemd-user-pam-config-file-for-Debian.patch
+++ b/debian/patches/Adjust-systemd-user-pam-config-file-for-Debian.patch
@@ -5,20 +5,21 @@ Subject: Adjust systemd-user pam config file for Debian
 This pam config file is used by libpam-systemd/systemd-logind when
 launching systemd user instances.
 ---
- src/login/systemd-user | 4 ++--
- 1 file changed, 2 insertions(+), 2 deletions(-)
+ src/login/systemd-user | 5 +++--
+ 1 file changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/src/login/systemd-user b/src/login/systemd-user
-index 7b57dbf..f87d560 100644
+index 7b57dbf..cf8d9c8 100644
 --- a/src/login/systemd-user
 +++ b/src/login/systemd-user
-@@ -2,7 +2,7 @@
+@@ -2,7 +2,8 @@
  
  # Used by systemd when launching systemd user instances.
  
 -account include system-auth
 -session include system-auth
 +@include common-account
-+@include common-session
++@include common-session-noninteractive
  auth required pam_deny.so
  password required pam_deny.so
++session optional pam_systemd.so
diff --git a/debian/patches/journal-Fix-syslog-forwarding-without-CAP_SYS_ADMIN.patch b/debian/patches/journal-Fix-syslog-forwarding-without-CAP_SYS_ADMIN.patch
new file mode 100644
index 0000000..a48ce44
--- /dev/null
+++ b/debian/patches/journal-Fix-syslog-forwarding-without-CAP_SYS_ADMIN.patch
@@ -0,0 +1,39 @@
+From: Christian Seiler <christian@iwakd.de>
+Date: Tue, 13 Jan 2015 11:53:25 +0100
+Subject: journal: Fix syslog forwarding without CAP_SYS_ADMIN
+
+In case CAP_SYS_ADMIN is missing (like in containers), one cannot fake pid in
+struct ucred (uid/gid are fine if CAP_SETUID/CAP_SETGID are present).
+
+Ensure that journald will try again to forward the messages to syslog without
+faking the SCM_CREDENTIALS pid (which isn't guaranteed to succeed anyway, since
+it also does the same thing if the process has already exited).
+
+With this patch, journald will no longer silently discard messages
+that are supposed to be sent to syslog in these situations.
+
+https://bugs.debian.org/775067
+---
+ src/journal/journald-syslog.c | 6 +++---
+ 1 file changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/src/journal/journald-syslog.c b/src/journal/journald-syslog.c
+index f97e0d2..01623ec 100644
+--- a/src/journal/journald-syslog.c
++++ b/src/journal/journald-syslog.c
+@@ -85,12 +85,12 @@ static void forward_syslog_iovec(Server *s, const struct iovec *iovec, unsigned
+                 return;
+         }
+ 
+-        if (ucred && errno == ESRCH) {
++        if (ucred && (errno == ESRCH || errno == EPERM)) {
+                 struct ucred u;
+ 
+                 /* Hmm, presumably the sender process vanished
+-                 * by now, so let's fix it as good as we
+-                 * can, and retry */
++                 * by now, or we don't have CAP_SYS_AMDIN, so
++                 * let's fix it as good as we can, and retry */
+ 
+                 u = *ucred;
+                 u.pid = getpid();
diff --git a/debian/patches/series b/debian/patches/series
index 80256e6..4c77439 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -125,6 +125,7 @@ journalctl-correct-help-text-for-until.patch
 Raise-level-of-Found-dependency.-lines.patch
 systemd-tmpfiles-Fix-IGNORE_DIRECTORY_PATH-age-handl.patch
 sysv-generator-handle-Provides-for-non-virtual-facil.patch
+journal-Fix-syslog-forwarding-without-CAP_SYS_ADMIN.patch
 
 ## Debian specific patches:
 Add-back-support-for-Debian-specific-config-files.patch
@@ -177,3 +178,5 @@ Prefer-etc-X11-default-display-manager-if-present.patch
 core-Fix-bind-error-message.patch
 core-Make-binding-notify-private-dbus-socket-more-ro.patch
 Fix-usr-remount-failure-for-split-usr.patch
+sysv-generator-Handle-.sh-suffixes-when-translating-.patch
+sysv-generator-Replace-Provides-symlinks-with-real-u.patch
diff --git a/debian/patches/sysv-generator-Handle-.sh-suffixes-when-translating-.patch b/debian/patches/sysv-generator-Handle-.sh-suffixes-when-translating-.patch
new file mode 100644
index 0000000..3462218
--- /dev/null
+++ b/debian/patches/sysv-generator-Handle-.sh-suffixes-when-translating-.patch
@@ -0,0 +1,65 @@
+From: Martin Pitt <martin.pitt@ubuntu.com>
+Date: Tue, 20 Jan 2015 16:41:31 +0100
+Subject: sysv-generator: Handle .sh suffixes when translating Provides:
+
+When deciding whether the provided name equals the file name in
+sysv_translate_facility(), also consider them equal if the file name has a
+".sh" suffix.
+
+This was uncovered by commit b7e7184 which then created a symlink
+"<name>.service" to itself for ".sh" suffixed init.d scripts.
+
+For additional robustness, refuse to create symlinks to itself in add_alias().
+
+Bug-Debian: https://bugs.debian.org/775889
+---
+ src/sysv-generator/sysv-generator.c | 15 ++++++++++++++-
+ 1 file changed, 14 insertions(+), 1 deletion(-)
+
+diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
+index 2f116fb..b7b62d6 100644
+--- a/src/sysv-generator/sysv-generator.c
++++ b/src/sysv-generator/sysv-generator.c
+@@ -125,6 +125,11 @@ static int add_alias(const char *service, const char *alias) {
+         assert(service);
+         assert(alias);
+ 
++        if (streq(service, alias)) {
++                log_error("Ignoring creation of an alias %s for itself", service);
++                return 0;
++        }
++
+         link = strjoin(arg_dest, "/", alias, NULL);
+         if (!link)
+                 return log_oom();
+@@ -283,6 +288,7 @@ static int sysv_translate_facility(const char *name, const char *filename, char
+         unsigned i;
+         char *r;
+         const char *n;
++        _cleanup_free_ char *filename_no_sh = NULL;
+ 
+         assert(name);
+         assert(_r);
+@@ -304,6 +310,13 @@ static int sysv_translate_facility(const char *name, const char *filename, char
+                 goto finish;
+         }
+ 
++        /* strip ".sh" suffix from file name for comparison */
++        filename_no_sh = strdup(filename);
++        if (!filename_no_sh)
++                return -ENOMEM;
++        if (endswith(filename, ".sh"))
++                filename_no_sh[strlen(filename)-3] = '\0';
++
+         /* If we don't know this name, fallback heuristics to figure
+          * out whether something is a target or a service alias. */
+ 
+@@ -313,7 +326,7 @@ static int sysv_translate_facility(const char *name, const char *filename, char
+ 
+                 /* Facilities starting with $ are most likely targets */
+                 r = unit_name_build(n, NULL, ".target");
+-        } else if (filename && streq(name, filename))
++        } else if (filename && streq(name, filename_no_sh))
+                 /* Names equaling the file name of the services are redundant */
+                 return 0;
+         else
diff --git a/debian/patches/sysv-generator-Replace-Provides-symlinks-with-real-u.patch b/debian/patches/sysv-generator-Replace-Provides-symlinks-with-real-u.patch
new file mode 100644
index 0000000..1bce3cc
--- /dev/null
+++ b/debian/patches/sysv-generator-Replace-Provides-symlinks-with-real-u.patch
@@ -0,0 +1,62 @@
+From: Martin Pitt <martin.pitt@ubuntu.com>
+Date: Wed, 21 Jan 2015 10:25:14 +0100
+Subject: sysv-generator: Replace Provides: symlinks with real units
+
+Since commit b7e7184 the SysV generator creates symlinks for all "Provides:" in
+the LSB header. However, this is too greedy; there are cases where the
+creation of a unit .service file fails because of an already existing
+symlink with the same name:
+
+ - Backup files such as /etc/init.d/foo.bak still have "Provides: foo", and
+   thus get a foo.service -> foo.bak.service link. foo.bak would not be enabled
+   in rcN.d/, but we (deliberately) create units for all executables in init.d/
+   so that a manual "systemctl start" works. If foo.bak is processed before,
+   the symlink already exists.
+
+ - init.d/bar has "Provides: foo", while there also is a real init.d/foo. The
+   former would create a link foo.service -> bar.service, while the latter
+   would fail to create the real foo.service.
+
+Keeping track of which alias symlinks we actually want is error prone, and
+restricting the creation of services for enabled init.d scripts would reduce
+the utility of the generator (for manual starting disabled init.d scripts) as
+well as not cover the second case. So if we encounter an existing symlink, just
+remove it before writing a real unit.
+
+Note that two init.d scripts "foo" and "bar" which both provide the same name
+"common" already work. The first processed init script wins and creates the
+"common.service" symlink, and the second just fails to create the symlink
+again.
+
+Bug-Debian: https://bugs.debian.org/775404
+---
+ src/sysv-generator/sysv-generator.c | 9 +++++++++
+ 1 file changed, 9 insertions(+)
+
+diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
+index b7b62d6..628d579 100644
+--- a/src/sysv-generator/sysv-generator.c
++++ b/src/sysv-generator/sysv-generator.c
+@@ -153,6 +153,7 @@ static int generate_unit_file(SysvStub *s) {
+         _cleanup_free_ char *wants = NULL;
+         _cleanup_free_ char *conflicts = NULL;
+         int r;
++        struct stat st;
+ 
+         before = strv_join(s->before, " ");
+         if (!before)
+@@ -174,6 +175,14 @@ static int generate_unit_file(SysvStub *s) {
+         if (!unit)
+                 return log_oom();
+ 
++        /* We might already have a symlink with the same name from a Provides:,
++         * or from backup files like /etc/init.d/foo.bak. Real scripts always win,
++         * so remove an existing link */
++        if (lstat(unit, &st) == 0 && S_ISLNK(st.st_mode)) {
++                log_warning("Overwriting existing symlink %s with real service", unit);
++                unlink(unit);
++        }
++
+         f = fopen(unit, "wxe");
+         if (!f) {
+                 log_error("Failed to create unit file %s: %m", unit);

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: