[systemd-devel] [PATCH] tmpfiles: only change device permissions if mknod succeeded

[Tom Gundersen]

On Mon, Oct 20, 2014 at 9:32 PM, Lennart Poettering
<lennart at poettering.net> wrote:
> On Tue, 14.10.14 16:19, Jan Synacek (jsynacek at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>>
>
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel