linux-kernel - Re: [PATCH] security/commoncap: don't assume "setid" if all ids are identical

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <87h61t7siv.fsf@email.froward.int.ebiederm.org>
Date: Fri, 09 May 2025 09:44:40 -0500
From: "Eric W. Biederman" <ebiederm@...ssion.com>
To: Max Kellermann <max.kellermann@...os.com>
Cc: sergeh@...nel.org,  "Serge E. Hallyn" <serge@...lyn.com>,  Andy
 Lutomirski <luto@...nel.org>,  paul@...l-moore.com,  jmorris@...ei.org,
  kees@...nel.org,  morgan@...nel.org,
  linux-security-module@...r.kernel.org,  linux-kernel@...r.kernel.org
Subject: Re: [PATCH] security/commoncap: don't assume "setid" if all ids are
 identical

Max Kellermann <max.kellermann@...os.com> writes:

> On Fri, May 9, 2025 at 12:12 AM <sergeh@...nel.org> wrote:
>> ABI stability is about the most important thing to Linus, so yes, if
>> documentation and code disagree, then we should fix the documentation,
>> except in the case where the current behavior just really is wrong
>> or insecure.
>
> It is insecure indeed (can be abused for LD_PRELOAD
> attacks):https://lore.kernel.org/lkml/CAKPOu+8+1uVrDJHwmHJd2d46-N6AwjR4_bbtoSJS+sx6J=rkjg@mail.gmail.com/

I don't understand what you are trying to solve,
but the patch at the top of the thread introduces a
has_identical_uids_gids and is pure nonsense.

In particular __is_setuid or __is_setgid being true guarantees
that has_identical_uids_gids will be false.

Which means has_identical_uids_gids adds nothing, and the patch is
pointless.

If your concern is LD_PRELOAD and the like please don't play with
the uids/gids and instead just make certain bprm->secureexec gets
set.

At this point I am pretty certain that changing the logic and leaving
extra uids/gids set will result in security vulnerabilities for someone
who actually depends upon how the code works today.  I see no evidence
in this conversation that anyone has surveyed the users of NO_NEW_PRIVS
and verified how anyone actually uses it.  Without such evidence we
have to assume that userspace depends upon the current behavior.

Eric