Re: [GIT PULL] Add trusted_for(2) (was O_MAYEXEC)

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

 




On 04/04/2022 23:28, Linus Torvalds wrote:
On Mon, Apr 4, 2022 at 1:29 PM Mickaël Salaün <mic@xxxxxxxxxxx> wrote:

This initial proposal was using a new faccessat2(2) flag:
AT_INTERPRETED, see
https://lore.kernel.org/all/20200908075956.1069018-2-mic@xxxxxxxxxxx/
What do you think about that? I'm happy to get back to this version if
everyone is OK with it.

I'm certainly happi_er_ with that, but I find that particular patch
odd for other reasons.

In no particular order:

  - what's with the insane non-C syntax? Double parentheses have no
actual meaning in C:

      if ((flags & AT_INTERPRETED)) {
          if ((mode & MAY_EXEC)) {

    so I don't understand why you'd use that strance thing.

I guess it comes from a previous version that ANDed two booleans.



  - why is this an AT_INTERPRETED flag? I don't understand the name, I
don't understand the semantics.

I wasn't sure it was a good idea to add another mode bit, so I ended up using a flag to not break compatibility of other mode checks but extend the semantic to interpreted scripts. But I agree that a new mode makes sense.



    Why would that flag have the same value as AT_SYMLINK_FOLLOW?

It was a bug.



    Why isn't this just a new _mode_ bit, which is what I feel is
sensible? We only use three bits (with no bits set meaning
"existence"), so we have *tons* of bits left in that namespace, and it
would make much more sense to me to have

         #define EXECVE_OK 8

     which is the same as the "group exec" bit, so it actually makes
some kind of sense too.

Looks fine to me. The "EXECVE_" prefix is a bit weird but it will not be defined in the kernel like X_OK and others anyway, and as you said, it matches S_IXGRP.



  - related to that "I don't understand the semantics", the
"documentation" for that thing doesn't make sense either:

     +         The
     +    main usage is for script
     +    interpreters to enforce a policy
     +    consistent with the kernel's one
     +    (through sysctl configuration or LSM
     +    policy).  */

I'll synchronize the documentation with a next series.



Now, what I *think* you mean is
See a following email in reply to Kees.
[...]


And yes, we still need to talk details:

  - no backwards compatibility issues, because we've happily always
checked 'mode' for being valid, so old kernels will always return
-EINVAL.

  - POSIX namespace issues for EXECVE_OK means that the name probably
needs some thinking (maybe we just need to call it __ACCESS_OK_EXECVE
internally or something - the kernel actually doesn't even export the
existing [FRWX]_OK values, because we "know" they map tho the usual
"owner RWX" bits, with F being "no bit set")

Right, I cannot find a better name for now.

See a following email in reply to Kees.
[...]


So to recap: I'm very much ok with some access() extension, but I
think even that very much needs clarification, and the existing patch
is just odd in many many ways.

This v8 was kind of an early version, I'll update everything. Thanks!



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux