Re: [systemd-devel] [PATCH] tmpfiles: only change device permissions if mknod succeeded
Tom Gundersen t...@jklm.no writes: On Mon, Oct 27, 2014 at 4:53 PM, Tom Gundersen t...@jklm.no wrote: On Mon, Oct 27, 2014 at 4:48 PM, Lennart Poettering mzerq...@0pointer.de wrote: On Sat, 25.10.14 01:36, Tom Gundersen (t...@jklm.no) wrote: On Mon, Oct 20, 2014 at 9:32 PM, Lennart Poettering lenn...@poettering.net wrote: On Tue, 14.10.14 16:19, Jan Synacek (jsyna...@redhat.com) wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1147248 Hmm, so far tmpfiles always adjust access modes, for all types of lines, if that's possible. I think this makes sense. The bug referenced above seems to suggest though that the access mode of the /dev/fuse file node is specified differently in two places though. This sounds like something to fix first? Well, the /run/tmpfiles.d/kmod.conf one is what the kernel exposes, and then the udev rules overrides this. We could surely fix this case, but in general I think we should expect that these may differ. To me it seems that we should not create devices nodes at all, except in systemd-tmpfiles-setup-dev.service, the reason being that udev rules are only applied to static nodes at udev startup, so any device nodes created (or changed) after that may end up with the wrong permissions (as seen here). Hmm, so does this mean that the kmod tmpfiles converter really should suffixits lines with the exclamation mark? That way, only invocation of tmpfiles with --boot would honour those files, which are the ones we start at boot. Does that make sense? Yes, indeed, this is precisely what we want. I had missed that feature. I'll do a patch. And done: http://permalink.gmane.org/gmane.linux.kernel.modules/1402. Jan, does this look like it solves the original problem? Cheers, Tom On my current rawhide (updated today, systemd-216-11.fc22.x86_64), with kmod patched using the patch you've provided, /dev/fuse is not created, not even on boot. However, invoking systemd-tmpfiles.d --create --boot correctly creates the node. # cat /run/tmpfiles.d/kmod.conf c! /dev/fuse 0600 - - - 10:229 c! /dev/btrfs-control 0600 - - - 10:234 c! /dev/loop-control 0600 - - - 10:237 d /dev/net 0755 - - - c! /dev/net/tun 0600 - - - 10:200 c! /dev/ppp 0600 - - - 108:0 c! /dev/uinput 0600 - - - 10:223 c! /dev/uhid 0600 - - - 10:239 d /dev/vfio 0755 - - - c! /dev/vfio/vfio 0600 - - - 10:196 c! /dev/vhci 0600 - - - 10:137 c! /dev/vhost-net 0600 - - - 10:238 d /dev/snd 0755 - - - c! /dev/snd/timer 0600 - - - 116:33 d /dev/snd 0755 - - - c! /dev/snd/seq 0600 - - - 116:1 Is that how it should work? Cheers, -- Jan Synacek Software Engineer, Red Hat signature.asc Description: PGP signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] tmpfiles: only change device permissions if mknod succeeded
On Wed, Oct 29, 2014 at 10:37 AM, Jan Synacek jsyna...@redhat.com wrote: Tom Gundersen t...@jklm.no writes: On Mon, Oct 27, 2014 at 4:53 PM, Tom Gundersen t...@jklm.no wrote: On Mon, Oct 27, 2014 at 4:48 PM, Lennart Poettering mzerq...@0pointer.de wrote: On Sat, 25.10.14 01:36, Tom Gundersen (t...@jklm.no) wrote: On Mon, Oct 20, 2014 at 9:32 PM, Lennart Poettering lenn...@poettering.net wrote: On Tue, 14.10.14 16:19, Jan Synacek (jsyna...@redhat.com) wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1147248 Hmm, so far tmpfiles always adjust access modes, for all types of lines, if that's possible. I think this makes sense. The bug referenced above seems to suggest though that the access mode of the /dev/fuse file node is specified differently in two places though. This sounds like something to fix first? Well, the /run/tmpfiles.d/kmod.conf one is what the kernel exposes, and then the udev rules overrides this. We could surely fix this case, but in general I think we should expect that these may differ. To me it seems that we should not create devices nodes at all, except in systemd-tmpfiles-setup-dev.service, the reason being that udev rules are only applied to static nodes at udev startup, so any device nodes created (or changed) after that may end up with the wrong permissions (as seen here). Hmm, so does this mean that the kmod tmpfiles converter really should suffixits lines with the exclamation mark? That way, only invocation of tmpfiles with --boot would honour those files, which are the ones we start at boot. Does that make sense? Yes, indeed, this is precisely what we want. I had missed that feature. I'll do a patch. And done: http://permalink.gmane.org/gmane.linux.kernel.modules/1402. Jan, does this look like it solves the original problem? Cheers, Tom On my current rawhide (updated today, systemd-216-11.fc22.x86_64), with kmod patched using the patch you've provided, /dev/fuse is not created, not even on boot. However, invoking systemd-tmpfiles.d --create --boot correctly creates the node. # cat /run/tmpfiles.d/kmod.conf c! /dev/fuse 0600 - - - 10:229 c! /dev/btrfs-control 0600 - - - 10:234 c! /dev/loop-control 0600 - - - 10:237 d /dev/net 0755 - - - c! /dev/net/tun 0600 - - - 10:200 c! /dev/ppp 0600 - - - 108:0 c! /dev/uinput 0600 - - - 10:223 c! /dev/uhid 0600 - - - 10:239 d /dev/vfio 0755 - - - c! /dev/vfio/vfio 0600 - - - 10:196 c! /dev/vhci 0600 - - - 10:137 c! /dev/vhost-net 0600 - - - 10:238 d /dev/snd 0755 - - - c! /dev/snd/timer 0600 - - - 116:33 d /dev/snd 0755 - - - c! /dev/snd/seq 0600 - - - 116:1 Is that how it should work? Yes, you also need systemd v217, as that adds the --boot argument to systemd-tmpfiles-setup-dev.service. Cheers, Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] tmpfiles: only change device permissions if mknod succeeded
On Sat, 25.10.14 01:36, Tom Gundersen (t...@jklm.no) wrote: On Mon, Oct 20, 2014 at 9:32 PM, Lennart Poettering lenn...@poettering.net wrote: On Tue, 14.10.14 16:19, Jan Synacek (jsyna...@redhat.com) wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1147248 Hmm, so far tmpfiles always adjust access modes, for all types of lines, if that's possible. I think this makes sense. The bug referenced above seems to suggest though that the access mode of the /dev/fuse file node is specified differently in two places though. This sounds like something to fix first? Well, the /run/tmpfiles.d/kmod.conf one is what the kernel exposes, and then the udev rules overrides this. We could surely fix this case, but in general I think we should expect that these may differ. To me it seems that we should not create devices nodes at all, except in systemd-tmpfiles-setup-dev.service, the reason being that udev rules are only applied to static nodes at udev startup, so any device nodes created (or changed) after that may end up with the wrong permissions (as seen here). Hmm, so does this mean that the kmod tmpfiles converter really should suffixits lines with the exclamation mark? That way, only invocation of tmpfiles with --boot would honour those files, which are the ones we start at boot. Does that make sense? 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] tmpfiles: only change device permissions if mknod succeeded
On Mon, Oct 27, 2014 at 4:48 PM, Lennart Poettering mzerq...@0pointer.de wrote: On Sat, 25.10.14 01:36, Tom Gundersen (t...@jklm.no) wrote: On Mon, Oct 20, 2014 at 9:32 PM, Lennart Poettering lenn...@poettering.net wrote: On Tue, 14.10.14 16:19, Jan Synacek (jsyna...@redhat.com) wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1147248 Hmm, so far tmpfiles always adjust access modes, for all types of lines, if that's possible. I think this makes sense. The bug referenced above seems to suggest though that the access mode of the /dev/fuse file node is specified differently in two places though. This sounds like something to fix first? Well, the /run/tmpfiles.d/kmod.conf one is what the kernel exposes, and then the udev rules overrides this. We could surely fix this case, but in general I think we should expect that these may differ. To me it seems that we should not create devices nodes at all, except in systemd-tmpfiles-setup-dev.service, the reason being that udev rules are only applied to static nodes at udev startup, so any device nodes created (or changed) after that may end up with the wrong permissions (as seen here). Hmm, so does this mean that the kmod tmpfiles converter really should suffixits lines with the exclamation mark? That way, only invocation of tmpfiles with --boot would honour those files, which are the ones we start at boot. Does that make sense? Yes, indeed, this is precisely what we want. I had missed that feature. I'll do a patch. Thanks! Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] tmpfiles: only change device permissions if mknod succeeded
On Mon, Oct 27, 2014 at 4:53 PM, Tom Gundersen t...@jklm.no wrote: On Mon, Oct 27, 2014 at 4:48 PM, Lennart Poettering mzerq...@0pointer.de wrote: On Sat, 25.10.14 01:36, Tom Gundersen (t...@jklm.no) wrote: On Mon, Oct 20, 2014 at 9:32 PM, Lennart Poettering lenn...@poettering.net wrote: On Tue, 14.10.14 16:19, Jan Synacek (jsyna...@redhat.com) wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1147248 Hmm, so far tmpfiles always adjust access modes, for all types of lines, if that's possible. I think this makes sense. The bug referenced above seems to suggest though that the access mode of the /dev/fuse file node is specified differently in two places though. This sounds like something to fix first? Well, the /run/tmpfiles.d/kmod.conf one is what the kernel exposes, and then the udev rules overrides this. We could surely fix this case, but in general I think we should expect that these may differ. To me it seems that we should not create devices nodes at all, except in systemd-tmpfiles-setup-dev.service, the reason being that udev rules are only applied to static nodes at udev startup, so any device nodes created (or changed) after that may end up with the wrong permissions (as seen here). Hmm, so does this mean that the kmod tmpfiles converter really should suffixits lines with the exclamation mark? That way, only invocation of tmpfiles with --boot would honour those files, which are the ones we start at boot. Does that make sense? Yes, indeed, this is precisely what we want. I had missed that feature. I'll do a patch. And done: http://permalink.gmane.org/gmane.linux.kernel.modules/1402. Jan, does this look like it solves the original problem? Cheers, Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] tmpfiles: only change device permissions if mknod succeeded
On Mon, Oct 20, 2014 at 9:32 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Tue, 14.10.14 16:19, Jan Synacek (jsyna...@redhat.com) wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1147248
Hmm, so far tmpfiles always adjust access modes, for all types of
lines, if that's possible. I think this makes sense. The bug
referenced above seems to suggest though that the access mode of the
/dev/fuse file node is specified differently in two places
though. This sounds like something to fix first?
Well, the /run/tmpfiles.d/kmod.conf one is what the kernel exposes,
and then the udev rules overrides this. We could surely fix this case,
but in general I think we should expect that these may differ.
To me it seems that we should not create devices nodes at all, except
in systemd-tmpfiles-setup-dev.service, the reason being that udev
rules are only applied to static nodes at udev startup, so any device
nodes created (or changed) after that may end up with the wrong
permissions (as seen here).
---
src/tmpfiles/tmpfiles.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 8108b43..ae0289d 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -824,6 +824,7 @@ static int create_item(Item *i) {
case CREATE_BLOCK_DEVICE:
case CREATE_CHAR_DEVICE: {
mode_t file_type;
+bool mknod_succeeded;
if (have_effective_cap(CAP_MKNOD) == 0) {
/* In a container we lack CAP_MKNOD. We
@@ -842,6 +843,7 @@ static int create_item(Item *i) {
r = mknod(i-path, i-mode | file_type,
i-major_minor);
label_context_clear();
}
+mknod_succeeded = (r == 0);
if (r 0) {
if (errno == EPERM) {
@@ -881,10 +883,11 @@ static int create_item(Item *i) {
}
}
-r = item_set_perms(i, i-path);
-if (r 0)
-return r;
-
+if (mknod_succeeded) {
+r = item_set_perms(i, i-path);
+if (r 0)
+return r;
+}
break;
}
--
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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] tmpfiles: only change device permissions if mknod succeeded
On Tue, 14.10.14 16:19, Jan Synacek (jsyna...@redhat.com) wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1147248
Hmm, so far tmpfiles always adjust access modes, for all types of
lines, if that's possible. I think this makes sense. The bug
referenced above seems to suggest though that the access mode of the
/dev/fuse file node is specified differently in two places
though. This sounds like something to fix first?
---
src/tmpfiles/tmpfiles.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 8108b43..ae0289d 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -824,6 +824,7 @@ static int create_item(Item *i) {
case CREATE_BLOCK_DEVICE:
case CREATE_CHAR_DEVICE: {
mode_t file_type;
+bool mknod_succeeded;
if (have_effective_cap(CAP_MKNOD) == 0) {
/* In a container we lack CAP_MKNOD. We
@@ -842,6 +843,7 @@ static int create_item(Item *i) {
r = mknod(i-path, i-mode | file_type,
i-major_minor);
label_context_clear();
}
+mknod_succeeded = (r == 0);
if (r 0) {
if (errno == EPERM) {
@@ -881,10 +883,11 @@ static int create_item(Item *i) {
}
}
-r = item_set_perms(i, i-path);
-if (r 0)
-return r;
-
+if (mknod_succeeded) {
+r = item_set_perms(i, i-path);
+if (r 0)
+return r;
+}
break;
}
--
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
[systemd-devel] [PATCH] tmpfiles: only change device permissions if mknod succeeded
https://bugzilla.redhat.com/show_bug.cgi?id=1147248
---
src/tmpfiles/tmpfiles.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 8108b43..ae0289d 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -824,6 +824,7 @@ static int create_item(Item *i) {
case CREATE_BLOCK_DEVICE:
case CREATE_CHAR_DEVICE: {
mode_t file_type;
+bool mknod_succeeded;
if (have_effective_cap(CAP_MKNOD) == 0) {
/* In a container we lack CAP_MKNOD. We
@@ -842,6 +843,7 @@ static int create_item(Item *i) {
r = mknod(i-path, i-mode | file_type,
i-major_minor);
label_context_clear();
}
+mknod_succeeded = (r == 0);
if (r 0) {
if (errno == EPERM) {
@@ -881,10 +883,11 @@ static int create_item(Item *i) {
}
}
-r = item_set_perms(i, i-path);
-if (r 0)
-return r;
-
+if (mknod_succeeded) {
+r = item_set_perms(i, i-path);
+if (r 0)
+return r;
+}
break;
}
--
1.9.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
