linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
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>
Subject: Re: [PATCH resend] fs/namei.c: micro-optimize acl_permission_check
Date: Sun, 7 Jun 2020 09:37:21 -0700	[thread overview]
Message-ID: <CAHk-=wixdSUWFf6BoT7rJUVRmjUv+Lir_Rnh81xx7e2wnzgKbg@mail.gmail.com> (raw)
In-Reply-To: <dcd7516b-0a1f-320d-018d-f3990e771f37@rasmusvillemoes.dk>

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

  reply	other threads:[~2020-06-07 16:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-05 14:23 [PATCH resend] fs/namei.c: micro-optimize acl_permission_check Rasmus Villemoes
2020-06-05 20:18 ` Linus Torvalds
2020-06-07 13:22   ` Rasmus Villemoes
2020-06-07 16:37     ` Linus Torvalds [this message]
2020-06-07 19:48       ` Linus Torvalds
2020-06-08  2:05         ` Al Viro
2020-06-08 19:18           ` Linus Torvalds
2020-06-08 10:08         ` Rasmus Villemoes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wixdSUWFf6BoT7rJUVRmjUv+Lir_Rnh81xx7e2wnzgKbg@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).