lkml.org 
[lkml]   [2022]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [GIT PULL] Add trusted_for(2) (was O_MAYEXEC)

On 04/04/2022 23:28, Linus Torvalds wrote:
> On Mon, Apr 4, 2022 at 1:29 PM Mickaël Salaün <mic@digikod.net> wrote:
>>
>> This initial proposal was using a new faccessat2(2) flag:
>> AT_INTERPRETED, see
>> https://lore.kernel.org/all/20200908075956.1069018-2-mic@digikod.net/
>> 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!

\
 
 \ /
  Last update: 2022-04-05 22:12    [W:0.125 / U:0.280 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site