From: Siddhesh Poyarekar <siddhesh@sourceware.org>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>,
libc-alpha@sourceware.org
Subject: Re: [PATCH v3 00/19] Improve loader environment variable handling
Date: Mon, 20 Nov 2023 18:12:39 -0500 [thread overview]
Message-ID: <fe9599ff-0858-4aa9-abf9-52faf0396617@sourceware.org> (raw)
In-Reply-To: <20231106202552.3404059-1-adhemerval.zanella@linaro.org>
On 2023-11-06 15:25, Adhemerval Zanella wrote:
> The recent CVE-2023-4911 fix [1] and tunable change to SXID_ERASE
> discussion [2] brought some issues with the current environment handling
> by the loader. Besides the bugs in tuning parsing, some other questions
> are:
Overall I think 1-10 are ready to be committed since they handle a full
block of work, i.e. tunables validation. I know 11 kinda belongs in
that block, but it would be nice to reduce this set to 10 from 19 for
the next version :)
Also maybe push the independent patches in 11-19 that have R-b already.
Thanks,
Sid
>
> * What should be the security boundaries for tunable and other tuning
> environment variables?
>
> * Should tunables be filtered out or be disabled altogether in setuid
> binaries [3]?
>
> * How should ld.so handle security-sensitive tunable (like malloc
> options)?
>
> * How to handle ill-formatted tunable definition [4]?
>
> * Is tunable copy/parsing (through tunable_strdup) required [5]?
>
> * Which other environment variables the loader should ignore and/or
> filter in security-sensitive context.
>
> On this patchset, I followed the idea laid out in the discussion on
> whether to apply SXID_ERASE to all tunables [6]:
>
> 1. ignore any tunable on AT_SECURE binaries (as some Linux distributions
> do already [7]);
>
> 2. Add malloc tunables along with GLIBC_TUNABLES to unsecvars;
>
> 3. Do not parse ill-formatted GLIBC_TUNABLES strings;
>
> 4. Remove the requirement of duplicating the GLIBC_TUNABLES string for
> parsing.
>
> 5. Ignore most of the environment variables on security-sensitive
> mode (AT_SECURE/setuid/setgid).
>
> Patch #1 removes '/etc/suid-debug', which has not been working since
> malloc debugging supported moved to libc_malloc_debug.so. It is one
> thing less that might change AT_SECURE binaries' behavior
> due to environment configurations.
>
> Patch #2 removed tunables parsing and applying for setuid/setgid
> binaries (similar to Alt Linux patch).
>
> Patch #3 and #4 add all malloc tunable and GLIBC_TUNABLES to unsecvars
> and improve tst-env-setuid.c to test all possible environment variables.
>
> Patch #5 and #6 improved the GLIBC_TUNABLES handling to avoid handling
> ill-formatted inputs.
>
> Patch #7 makes _dl_debug_vdprintf usable before self-relocation so patch
> #8 can add a loader warning that ill-formatted GLIBC_TUNABLES inputs are
> ignored (it also fixes the issue where the GLIBC_TUNABLE allocation
> failure will trigger a SEGFAULT on some architecture for PIE).
>
> Patch #9, #10, and #11 remove the tunable_strdup and make the
> GLIBC_TUNABLE parsing in place (no more possible allocation failure).
> The parsing now tracks the tunable start and its size. The
> dl-tunable-parse.h adds helper functions to help to parse, like an
> strcmp that also checks for size and an iterator for suboptions that are
> comma-separated (used on hwcap parsing by x86, powerpc, and s390x).
>
> Patch #12, #13, #14, #16, and #18 make loader ignore all but just
> LD_PRELOAD and LD_AUDIT for setuid binaries. For both options, loader
> ensures that pathnames containing slashes are ignored and shared
> libraries are loaded only from the standard search directories and only
> if they have set-user-ID mode bit enabled.
>
> [1] https://sourceware.org/pipermail/libc-alpha/2023-October/151921.html
> [2] https://sourceware.org/pipermail/libc-alpha/2023-October/151936.html
> [3] https://www.openwall.com/lists/oss-security/2023/10/03/3
> [4] https://sourceware.org/pipermail/libc-alpha/2023-October/151927.html
> [5] https://sourceware.org/pipermail/libc-alpha/2023-October/151959.html
> [6] https://sourceware.org/pipermail/libc-alpha/2023-October/152011.html
> [7] https://git.altlinux.org/gears/g/glibc.git?p=glibc.git;a=commitdiff;h=5d1686416ab766f3dd0780ab730650c4c0f76ca9
>
> Changes from v2:
> * Extend tst-tunables with tunables aliases tests.
> * Use warning instead of an error to indicate invalid tunables.
> * Fixed tunable_initialize for string aliases.
> Changes from v1:
> * Ignore most of the environment variables on security-sensitive mode.
> * Extend tests.
>
> Adhemerval Zanella (19):
> elf: Remove /etc/suid-debug support
> elf: Add GLIBC_TUNABLES to unsecvars
> elf: Ignore GLIBC_TUNABLES for setuid/setgid binaries
> elf: Add all malloc tunable to unsecvars
> elf: Do not process invalid tunable format
> elf: Do not parse ill-formatted strings
> elf: Fix _dl_debug_vdprintf to work before self-relocation
> elf: Emit warning if tunable is ill-formatted
> x86: Use dl-symbol-redir-ifunc.h on cpu-tunables
> s390: Use dl-symbol-redir-ifunc.h on cpu-tunables
> elf: Do not duplicate the GLIBC_TUNABLES string
> elf: Ignore LD_PROFILE for setuid binaries
> elf: Remove LD_PROFILE for static binaries
> elf: Ignore loader debug env vars for setuid
> elf: Remove any_debug from dl_main_state
> elf: Ignore LD_LIBRARY_PATH and debug env var for setuid for static
> elf: Add comments on how LD_AUDIT and LD_PRELOAD handle
> __libc_enable_secure
> elf: Ignore LD_BIND_NOW and LD_BIND_NOT for setuid binaries
> elf: Refactor process_envvars
>
> elf/Makefile | 26 +-
> elf/dl-load.c | 10 +-
> elf/dl-main.h | 3 -
> elf/dl-printf.c | 16 +-
> elf/dl-runtime.c | 12 +-
> elf/dl-support.c | 41 +--
> elf/dl-tunable-types.h | 10 -
> elf/dl-tunables.c | 219 ++++--------
> elf/dl-tunables.h | 6 +-
> elf/dl-tunables.list | 17 -
> elf/{dl-profstub.c => libc-dl-profstub.c} | 0
> elf/rtld.c | 122 +++++--
> elf/tst-env-setuid-static.c | 1 +
> elf/tst-env-setuid-tunables.c | 59 ++--
> elf/tst-env-setuid.c | 140 +++++---
> elf/tst-tunables.c | 321 ++++++++++++++++++
> include/dlfcn.h | 5 +
> manual/README.tunables | 9 -
> manual/memory.texi | 4 +-
> manual/tunables.texi | 4 +-
> scripts/gen-tunables.awk | 18 +-
> stdio-common/Makefile | 5 +
> stdio-common/_itoa.c | 5 +
> sysdeps/aarch64/dl-machine.h | 4 +-
> sysdeps/aarch64/dl-trampoline.S | 2 +-
> sysdeps/alpha/dl-machine.h | 6 +-
> sysdeps/alpha/dl-trampoline.S | 4 +
> sysdeps/arm/dl-machine.h | 4 +-
> sysdeps/arm/dl-trampoline.S | 2 +-
> sysdeps/generic/dl-tunables-parse.h | 129 +++++++
> sysdeps/generic/unsecvars.h | 10 +
> sysdeps/hppa/dl-machine.h | 36 +-
> sysdeps/hppa/dl-trampoline.S | 2 +
> sysdeps/i386/dl-machine.h | 2 +
> sysdeps/i386/dl-trampoline.S | 2 +-
> .../i686/multiarch/dl-symbol-redir-ifunc.h | 5 +
> sysdeps/ia64/dl-machine.h | 10 +-
> sysdeps/ia64/dl-trampoline.S | 2 +-
> sysdeps/loongarch/dl-machine.h | 6 +-
> sysdeps/loongarch/dl-trampoline.h | 2 +
> sysdeps/m68k/dl-machine.h | 4 +-
> sysdeps/m68k/dl-trampoline.S | 2 +
> sysdeps/powerpc/powerpc32/dl-machine.c | 2 +-
> sysdeps/powerpc/powerpc32/dl-machine.h | 10 +-
> sysdeps/powerpc/powerpc32/dl-trampoline.S | 2 +-
> sysdeps/powerpc/powerpc64/dl-machine.h | 20 +-
> sysdeps/powerpc/powerpc64/dl-trampoline.S | 2 +-
> sysdeps/s390/cpu-features.c | 169 ++++-----
> .../s390/multiarch/dl-symbol-redir-ifunc.h | 2 +
> sysdeps/s390/s390-32/dl-machine.h | 8 +-
> sysdeps/s390/s390-32/dl-trampoline.h | 2 +-
> sysdeps/s390/s390-64/dl-machine.h | 8 +-
> sysdeps/s390/s390-64/dl-trampoline.h | 2 +-
> sysdeps/sh/dl-machine.h | 2 +
> sysdeps/sh/dl-trampoline.S | 2 +
> sysdeps/sparc/sparc32/dl-machine.h | 4 +-
> sysdeps/sparc/sparc32/dl-trampoline.S | 2 +
> sysdeps/sparc/sparc64/dl-machine.h | 4 +-
> sysdeps/sparc/sparc64/dl-trampoline.S | 2 +
> .../unix/sysv/linux/aarch64/cpu-features.c | 38 ++-
> .../sysv/linux/i386/dl-writev.h} | 18 +-
> .../unix/sysv/linux/powerpc/cpu-features.c | 45 +--
> .../sysv/linux/powerpc/tst-hwcap-tunables.c | 6 +-
> sysdeps/x86/Makefile | 4 +-
> sysdeps/x86/cpu-tunables.c | 135 +++-----
> sysdeps/x86/tst-hwcap-tunables.c | 148 ++++++++
> sysdeps/x86_64/64/dl-tunables.list | 1 -
> sysdeps/x86_64/dl-machine.h | 2 +
> sysdeps/x86_64/dl-trampoline.S | 64 ++--
> .../x86_64/multiarch/dl-symbol-redir-ifunc.h | 15 +
> 70 files changed, 1281 insertions(+), 725 deletions(-)
> rename elf/{dl-profstub.c => libc-dl-profstub.c} (100%)
> create mode 100644 elf/tst-env-setuid-static.c
> create mode 100644 elf/tst-tunables.c
> create mode 100644 sysdeps/generic/dl-tunables-parse.h
> rename sysdeps/{x86_64/memcmp-isa-default-impl.h => unix/sysv/linux/i386/dl-writev.h} (62%)
> create mode 100644 sysdeps/x86/tst-hwcap-tunables.c
>
next prev parent reply other threads:[~2023-11-20 23:12 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-06 20:25 Adhemerval Zanella
2023-11-06 20:25 ` [PATCH v3 01/19] elf: Remove /etc/suid-debug support Adhemerval Zanella
2023-11-06 20:25 ` [PATCH v3 02/19] elf: Add GLIBC_TUNABLES to unsecvars Adhemerval Zanella
2023-11-06 20:25 ` [PATCH v3 03/19] elf: Ignore GLIBC_TUNABLES for setuid/setgid binaries Adhemerval Zanella
2023-11-06 20:25 ` [PATCH v3 04/19] elf: Add all malloc tunable to unsecvars Adhemerval Zanella
2023-11-06 20:25 ` [PATCH v3 05/19] elf: Do not process invalid tunable format Adhemerval Zanella
2023-11-06 20:25 ` [PATCH v3 06/19] elf: Do not parse ill-formatted strings Adhemerval Zanella
2023-11-20 21:48 ` Siddhesh Poyarekar
2023-11-06 20:25 ` [PATCH v3 07/19] elf: Fix _dl_debug_vdprintf to work before self-relocation Adhemerval Zanella
2023-11-06 20:25 ` [PATCH v3 08/19] elf: Emit warning if tunable is ill-formatted Adhemerval Zanella
2023-11-20 21:50 ` Siddhesh Poyarekar
2023-11-06 20:25 ` [PATCH v3 09/19] x86: Use dl-symbol-redir-ifunc.h on cpu-tunables Adhemerval Zanella
2023-11-06 20:25 ` [PATCH v3 10/19] s390: " Adhemerval Zanella
2023-11-06 20:25 ` [PATCH v3 11/19] elf: Do not duplicate the GLIBC_TUNABLES string Adhemerval Zanella
2023-11-20 22:44 ` Siddhesh Poyarekar
2023-11-21 18:12 ` Adhemerval Zanella Netto
2023-11-22 11:39 ` Adhemerval Zanella Netto
2023-11-22 12:23 ` Siddhesh Poyarekar
2023-11-22 13:03 ` Adhemerval Zanella Netto
2023-11-22 13:24 ` Siddhesh Poyarekar
2023-11-22 14:13 ` Adhemerval Zanella Netto
2023-11-06 20:25 ` [PATCH v3 12/19] elf: Ignore LD_PROFILE for setuid binaries Adhemerval Zanella
2023-11-20 22:47 ` Siddhesh Poyarekar
2023-11-06 20:25 ` [PATCH v3 13/19] elf: Remove LD_PROFILE for static binaries Adhemerval Zanella
2023-11-20 22:55 ` Siddhesh Poyarekar
2023-11-06 20:25 ` [PATCH v3 14/19] elf: Ignore loader debug env vars for setuid Adhemerval Zanella
2023-11-20 22:57 ` Siddhesh Poyarekar
2023-11-21 18:24 ` Adhemerval Zanella Netto
2023-11-06 20:25 ` [PATCH v3 15/19] elf: Remove any_debug from dl_main_state Adhemerval Zanella
2023-11-20 22:58 ` Siddhesh Poyarekar
2023-11-06 20:25 ` [PATCH v3 16/19] elf: Ignore LD_LIBRARY_PATH and debug env var for setuid for static Adhemerval Zanella
2023-11-20 22:59 ` Siddhesh Poyarekar
2023-11-06 20:25 ` [PATCH v3 17/19] elf: Add comments on how LD_AUDIT and LD_PRELOAD handle __libc_enable_secure Adhemerval Zanella
2023-11-06 20:25 ` [PATCH v3 18/19] elf: Ignore LD_BIND_NOW and LD_BIND_NOT for setuid binaries Adhemerval Zanella
2023-11-20 23:02 ` Siddhesh Poyarekar
2023-11-06 20:25 ` [PATCH v3 19/19] elf: Refactor process_envvars Adhemerval Zanella
2023-11-20 23:09 ` Siddhesh Poyarekar
2023-11-21 19:00 ` Adhemerval Zanella Netto
2023-11-20 23:12 ` Siddhesh Poyarekar [this message]
2023-11-21 19:37 ` [PATCH v3 00/19] Improve loader environment variable handling Adhemerval Zanella Netto
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=fe9599ff-0858-4aa9-abf9-52faf0396617@sourceware.org \
--to=siddhesh@sourceware.org \
--cc=adhemerval.zanella@linaro.org \
--cc=libc-alpha@sourceware.org \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).