Re: [systemd-devel] [PATCH] dbus-manager: don't allow enabling if unit is masked
On Tue, 07.10.14 08:23, Jan Synacek (jsyna...@redhat.com) wrote: Lennart Poettering lenn...@poettering.net writes: On Mon, 06.10.14 13:21, Jan Synacek (jsyna...@redhat.com) wrote: Hmm with this change in place we'd have different behaviour for the cases where systemctl executes the operation client-side, and when it goes via the bus. We really should keep those differences in behaviour to a minimum. I figure the verification for this really needs to be moved a few levels down, somewhere into unit_file_enable() and friends, so that all code paths behave the same. But that wouldn't fix a scenario where one uses just dbus to call the method, would it? Maybe I'm missing something, but that's how I understood the code so far. However, I agree that the fix is incomplete and I'll try to fix that. unit_file_enable() is what is used by both codepaths: the one via dbus, and the one executed directly in systemctl, from the client side. If you change unit_file_enable() you hence did the work for both sides. While I'm at it, what about disable? Should it behave in the same way, i.e. return error when the unit is masked? My guess is that yes, but I'm not sure. disable, reenable, mask, unmask, should not be changed. Only enable, link and preset should get this check. 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] dbus-manager: don't allow enabling if unit is masked
Lennart Poettering lenn...@poettering.net writes:
On Mon, 06.10.14 13:21, Jan Synacek (jsyna...@redhat.com) wrote:
Hmm with this change in place we'd have different behaviour for the
cases where systemctl executes the operation client-side, and when it
goes via the bus. We really should keep those differences in behaviour
to a minimum.
I figure the verification for this really needs to be moved a few
levels down, somewhere into unit_file_enable() and friends, so that
all code paths behave the same.
But that wouldn't fix a scenario where one uses just dbus to call the
method, would it? Maybe I'm missing something, but that's how I
understood the code so far. However, I agree that the fix is incomplete
and I'll try to fix that.
While I'm at it, what about disable? Should it behave in the same way,
i.e. return error when the unit is masked? My guess is that yes, but I'm
not sure.
https://bugzilla.redhat.com/show_bug.cgi?id=1149069
---
src/core/dbus-manager.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 533ce43..c2d52b2 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1588,18 +1588,23 @@ static int method_enable_unit_files_generic(
if (r 0)
return r;
-#ifdef HAVE_SELINUX
STRV_FOREACH(i, l) {
Unit *u;
u = manager_get_unit(m, *i);
if (u) {
+#ifdef HAVE_SELINUX
r = selinux_unit_access_check(u, message, verb,
error);
if (r 0)
return r;
+#endif
+if (u-load_state == UNIT_MASKED) {
+sd_bus_error_setf(error,
BUS_ERROR_UNIT_MASKED,
+ Unit %s is masked.,
u-id);
+return -EADDRNOTAVAIL;
+}
}
}
-#endif
scope = m-running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM :
UNIT_FILE_USER;
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
--
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] dbus-manager: don't allow enabling if unit is masked
On Tue, Oct 07, 2014 at 08:23:32AM +0200, Jan Synacek wrote: While I'm at it, what about disable? Should it behave in the same way, i.e. return error when the unit is masked? My guess is that yes, but I'm not sure. I don't see a reason why disabling should be disallowed. I think we even allow (or should allow) disabling of uninstalled units. Disabling is only about removing links. E.g. in the situation where somebody masked a file by adding a link to /dev/null, and wants to remove the links also, this should be totally OK. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] dbus-manager: don't allow enabling if unit is masked
https://bugzilla.redhat.com/show_bug.cgi?id=1149069
---
src/core/dbus-manager.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 533ce43..c2d52b2 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1588,18 +1588,23 @@ static int method_enable_unit_files_generic(
if (r 0)
return r;
-#ifdef HAVE_SELINUX
STRV_FOREACH(i, l) {
Unit *u;
u = manager_get_unit(m, *i);
if (u) {
+#ifdef HAVE_SELINUX
r = selinux_unit_access_check(u, message, verb, error);
if (r 0)
return r;
+#endif
+if (u-load_state == UNIT_MASKED) {
+sd_bus_error_setf(error, BUS_ERROR_UNIT_MASKED,
+ Unit %s is masked., u-id);
+return -EADDRNOTAVAIL;
+}
}
}
-#endif
scope = m-running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM :
UNIT_FILE_USER;
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] dbus-manager: don't allow enabling if unit is masked
On Mon, 06.10.14 13:21, Jan Synacek (jsyna...@redhat.com) wrote:
Hmm with this change in place we'd have different behaviour for the
cases where systemctl executes the operation client-side, and when it
goes via the bus. We really should keep those differences in behaviour
to a minimum.
I figure the verification for this really needs to be moved a few
levels down, somewhere into unit_file_enable() and friends, so that
all code paths behave the same.
https://bugzilla.redhat.com/show_bug.cgi?id=1149069
---
src/core/dbus-manager.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 533ce43..c2d52b2 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1588,18 +1588,23 @@ static int method_enable_unit_files_generic(
if (r 0)
return r;
-#ifdef HAVE_SELINUX
STRV_FOREACH(i, l) {
Unit *u;
u = manager_get_unit(m, *i);
if (u) {
+#ifdef HAVE_SELINUX
r = selinux_unit_access_check(u, message, verb,
error);
if (r 0)
return r;
+#endif
+if (u-load_state == UNIT_MASKED) {
+sd_bus_error_setf(error,
BUS_ERROR_UNIT_MASKED,
+ Unit %s is masked.,
u-id);
+return -EADDRNOTAVAIL;
+}
}
}
-#endif
scope = m-running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM :
UNIT_FILE_USER;
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
