Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200605142300.14591-1-linux@rasmusvillemoes.dk>
 <CAHk-=wgz68f2u7bFPZCWgbsbEJw+2HWTJFXSg_TguY+xJ8WrNw@mail.gmail.com> <dcd7516b-0a1f-320d-018d-f3990e771f37@rasmusvillemoes.dk>
In-Reply-To: <dcd7516b-0a1f-320d-018d-f3990e771f37@rasmusvillemoes.dk>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sun, 7 Jun 2020 09:37:21 -0700
Message-ID: <CAHk-=wixdSUWFf6BoT7rJUVRmjUv+Lir_Rnh81xx7e2wnzgKbg@mail.gmail.com>
Subject: Re: [PATCH resend] fs/namei.c: micro-optimize acl_permission_check
To:     Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc:     Alexander Viro <viro@zeniv.linux.org.uk>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sun, Jun 7, 2020 at 6:22 AM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> Yes, I did think about that, but I thought this was the more obviously
> correct approach, and that in practice one only sees the 0X44 and 0X55
> cases.

I'm not sure about that - it probably depends on your umask.

Because I see a lot of -rw-rw-r--. in my home directory, and it looks
like I have a umask of 0002.

That's just the Fedora default, I think. Looking at /etc/bashrc, it does

    if [ $UID -gt 199 ] && [ "`/usr/bin/id -gn`" = "`/usr/bin/id -un`" ]; then
       umask 002
    else
       umask 022
    fi

iow, if you have the same user-name and group name, then umask is 002
by default for regular users.

Honestly, I'm not sure why Fedora has that "each user has its own
group" thing, but it's at least one common setup.

So I think that the system you are looking at just happens to have
umask 0022, which is traditional when you have just a 'user' group.

> That will kinda work, except you do that mask &= MAY_RWX before
> check_acl(), which cares about MAY_NOT_BLOCK and who knows what other bits.

Good catch.

> Perhaps this? As a whole function, I think that's a bit easier for
> brain-storming. It's your patch, just with that rwx thing used instead
> of mask, except for the call to check_acl().

Looks fine to me. Once we have to have rwx/mask separate, I'm not sure
it's worth having that early masking at all (didn't check what the
register pressure is over that "check_acl()" call, but at least it is
fairly easy to follow along.

Send me a patch with commit message etc, and I'll apply it.

               Linus