linux-kernel - Re: [GIT PULL] SELinux patches for v6.2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 13 Dec 2022 09:44:20 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Paul Moore <paul@...l-moore.com>
Cc:     selinux@...r.kernel.org, linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] SELinux patches for v6.2

On Mon, Dec 12, 2022 at 7:05 PM Paul Moore <paul@...l-moore.com> wrote:
>
>   Unfortunately, this pull request does
> conflict with fixes that were merged during the v6.1-rcX cycle so you
> will either need to do some manual fixup or you can pull the tag below
> which has the necessary fixes and has been sanity tested today.

I did the merge manually, but compared to your version. They were
identical except that you hadn't added the documentation entry for the
gfp_flags parameter.

That said, I'm not super-happy with that merge - it was the trivial
straightforward one, but when I looked at the code it struck me that
the only thing that actually seems to *use* that gfp_flags argument is
that

        if (oldc->str) {
                s = kstrdup(oldc->str, gfp_flags);
                if (!s)
                        return -ENOMEM;

sequence. And it strikes me that this is not the only place where
selinux ends up doing that whole

                str = kstrdup(ctx->str, GFP_xyz);

dance.

It feels to me like that thing shouldn't be an allocation at all, but
that selinux should use ref-counted strings instead (and just increase
the refcount). It's in other places like context_cpy(), but having it
be a refcounted string would also potentially help with
"context_cmp()" in that the string compare could be a "is it the same
ref-counted pointer" and maybe hit that case most of the time before
it even needs to do an actual strcmp.

Hmm?

Anyway, that was just my reaction to resolving that conflict, and
obviously *not* for this merge window. I'm just saying that if you
agree, maybe that could be a future improvement, making the whole
allocation - and the whole need for that gfp_flag - go away?

               Linus