public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: John Mellor-Crummey <johnmc@rice.edu>
To: Carlos O'Donell <carlos@redhat.com>
Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>,
	libc-alpha@sourceware.org, jma14 <jma14@rice.edu>,
	Ben Woodard <woodard@redhat.com>
Subject: Re: [PATCH v12 0/4] Multiple rtld-audit fixes
Date: Tue, 1 Feb 2022 01:53:07 -0600	[thread overview]
Message-ID: <5B3A03FD-FBAD-4AC1-89B5-71D7007BB7E1@rice.edu> (raw)
In-Reply-To: <2490cb9e-fd04-316a-4642-d1b52d69d919@redhat.com>

Thanks to Adhemerval, Florian, Szabolcs, and Carlos for all of the work it took to develop, test, and land these changes. My team really appreciates it! 

These fixes will provide the firm foundation we need for reliable tools.

John Mellor-Crummey

(sent from my phone)

> On Feb 1, 2022, at 12:45 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> 
> On 1/25/22 13:36, Adhemerval Zanella wrote:
>> This patches fixes some of remaining issues brought by John
>> Mellor-Crummey [1] while trying to use it along with the HPCToolkit.
>> This should cover all the issues listed as 'Tier 1' [2] (the aarch64
>> SVE requires additional work, so it is postpone to 2.36) and also
>> most of the 'Tier2' issue (BZ#28096 inclusive) which prevents the use
>> of some glibc function that uses TLS internally on the audit module.
> 
> These changes look good to me. I've reviewed all 4 patches and I don't have any
> logical changes, just minor cleanups. Please post v13 and I'll ACK that for inclusion
> in glibc 2.35.
> 
>> I have checked the patches on x86_64, i686, aarch64, armv7, powerpc64,
>> powerpc64le, and powerpc.
> 
> I have tested on x86_64 and i686. Thanks for working on this. I would like to include
> this in glibc 2.35. The only external public data change is the LAV_VERSION bump,
> otherwise we can change the internal implementation at will. The LAV_VERSION bump
> will mean pre-glibc-2.35 audit modules need to be recompiled.
> 
> Thank you for working through these issues for the users who presented their
> requirements to the community.
> 
> I tested these changes in a Fedora Rawhide desktop environment to catch any potential
> issues like the DTV gap reuse problem. I didn't see any problems and ran GNOME,
> the full desktop, Firefox, and sandboxed content (youtube), all without problems.
> 
> I also tested the downgrade errors:
> LD_AUDIT=./audit.so ./main 
>      4124:    ERROR: audit interface './audit.so' requires version 2 (maximum supported version 1); ignored.
> 
> Which shows a new audit module trying to be loaded by an old glibc.
> 
> The test passes with your changes, but segfaults without them.
> 
>> [1] https://sourceware.org/pipermail/libc-alpha/2021-June/127636.html
>> [2] https://docs.google.com/document/d/1dVaDBdzySecxQqD6hLLzDrEF18M1UtjDna9gL5BWWI0/edit#
>> [3] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/ld-audit-fixes
>> 
>> Changes from v11:
>>  - Fixed tst-audit23.
>> 
>> Changes from v10:
>>  - Removed l_auditing usage on initial-exec TLS access fix.
>>  - Fixed aarch64 comments about the _dl_runtime_profile stack layout.
>>  - Added some more argumet checking on some tests.
>>  - Fixed copyright years.
>> 
>> Changes from v9:
>>  - Fixed aarch64 comments about the _dl_runtime_profile stack layout.
>>  - Rebased against master.
>> 
>> Changes from v8:
>>  - Improved la_activity test coverage.
>>  - Fixed BZ#28096 regression.
>> 
>> Changes from v7:
>>  - Added la_activity tests during application exit.
>>  - Refactor _dl_allocate_tls_init to not initialize static TLS.
>>  - Changed sotruss to warn instead of error for bind-now.
>>  - Added NEWS and changed commit message for aarch64.
>> 
>> Changes from v6:
>>  - Dropped SVE, main application on main_map l_name, and Run
>>    constructors if executable has a soname of a dependency patches.
>>  - Bumped LAV_VERSION to 2 on la_symbind bind-now support.
>>  - Added extension pointer on aarch64 fix.
>>  - Moved the refactor patch at the start of the set.
>>  - Changed _dl_audit_objsearch interface.
>> 
>> Changes from v5:
>>  - Fixed build with --enable-profiling=yes.
>>  - Moved la_activity (LA_ACT_ADD) *after* _dl_add_to_namespace_list()
>>    for BZ#28062 fix.
>>  - Fixed powerpc64 ELFv1 OPD toc setup for bind-now.
>>  - Fixed testsuite issues for ia64.
>>  - Removed LA_SYMB_BINDNOW now that LA_SYMB_NOPLTENTER and
>>    LA_SYMB_NOPLTEXIT is passed for bind-now.
>> 
>> Changes from v4:
>>  - Added a fix for constructors if executable has a soname of a
>>    dependency
>>  - Rebased against master.
>> 
>> Changes from v3
>>  - Added a aarch64 SVE RFC patch.
>>  - Fixed an issue with bind-now fix on powerpc64 ELFv1.
>>  - Rebased against master.
>> 
>> Changes from v2
>>  - Refactored rtld-audit code to move common come to dl-audit.c.
>>  - Issue audit la_objopen() for vDSO.
>>  - Isseu la_activity during application exit.
>>  - Issue la_symbind() for bind-now (BZ #23734).
>>  - Fix runtime linker auditing on aarch64 (BZ #26643)
>> 
>> Changes from v1
>>  - Fixed -fstack-protector-all tst-auditmod17.
>>  - Simplify the _dl_call_libc_early_init call the 'Fix audit
>>    regression' patch.
>>  - Remove symbind check fr BZ#15333.
>>  - Added the BZ#28096 fix.
>> 
>> --
>> 
>> Adhemerval Zanella (3):
>>  elf: Add la_activity during application exit
>>  elf: Fix initial-exec TLS access on audit modules (BZ #28096)
>>  elf: Issue la_symbind for bind-now (BZ #23734)
>> 
>> Ben Woodard (1):
>>  elf: Fix runtime linker auditing on aarch64 (BZ #26643)
>> 
>> NEWS                             |   8 ++
>> bits/link_lavcurrent.h           |   2 +-
>> elf/Makefile                     | 104 +++++++++++++-
>> elf/dl-audit.c                   |  58 +++++---
>> elf/dl-fini.c                    |   8 ++
>> elf/dl-tls.c                     |  13 +-
>> elf/do-rel.h                     |  57 ++++++--
>> elf/rtld.c                       |   5 +-
>> elf/sotruss-lib.c                |   7 +
>> elf/tst-audit21.c                |  42 ++++++
>> elf/tst-audit23.c                | 239 +++++++++++++++++++++++++++++++
>> elf/tst-audit23mod.c             |  23 +++
>> elf/tst-audit24a.c               |  36 +++++
>> elf/tst-audit24amod1.c           |  31 ++++
>> elf/tst-audit24amod2.c           |  25 ++++
>> elf/tst-audit24b.c               |  37 +++++
>> elf/tst-audit24bmod1.c           |  31 ++++
>> elf/tst-audit24bmod2.c           |  23 +++
>> elf/tst-audit24c.c               |   2 +
>> elf/tst-audit24d.c               |  36 +++++
>> elf/tst-audit24dmod1.c           |  33 +++++
>> elf/tst-audit24dmod2.c           |  28 ++++
>> elf/tst-audit24dmod3.c           |  31 ++++
>> elf/tst-audit24dmod4.c           |  25 ++++
>> elf/tst-audit25a.c               | 129 +++++++++++++++++
>> elf/tst-audit25b.c               | 128 +++++++++++++++++
>> elf/tst-audit25mod1.c            |  30 ++++
>> elf/tst-audit25mod2.c            |  30 ++++
>> elf/tst-audit25mod3.c            |  22 +++
>> elf/tst-audit25mod4.c            |  22 +++
>> elf/tst-auditmod21a.c            |  80 +++++++++++
>> elf/tst-auditmod21b.c            |  22 +++
>> elf/tst-auditmod23.c             |  74 ++++++++++
>> elf/tst-auditmod24.h             |  29 ++++
>> elf/tst-auditmod24a.c            | 114 +++++++++++++++
>> elf/tst-auditmod24b.c            | 104 ++++++++++++++
>> elf/tst-auditmod24c.c            |   3 +
>> elf/tst-auditmod24d.c            | 120 ++++++++++++++++
>> elf/tst-auditmod25.c             |  79 ++++++++++
>> nptl/allocatestack.c             |   2 +-
>> sysdeps/aarch64/Makefile         |  20 +++
>> sysdeps/aarch64/bits/link.h      |  26 ++--
>> sysdeps/aarch64/dl-audit-check.h |  28 ++++
>> sysdeps/aarch64/dl-link.sym      |   6 +-
>> sysdeps/aarch64/dl-trampoline.S  |  81 +++++++----
>> sysdeps/aarch64/tst-audit26.c    |  37 +++++
>> sysdeps/aarch64/tst-audit26mod.c |  33 +++++
>> sysdeps/aarch64/tst-audit26mod.h |  50 +++++++
>> sysdeps/aarch64/tst-audit27.c    |  64 +++++++++
>> sysdeps/aarch64/tst-audit27mod.c |  95 ++++++++++++
>> sysdeps/aarch64/tst-audit27mod.h |  67 +++++++++
>> sysdeps/aarch64/tst-auditmod26.c | 103 +++++++++++++
>> sysdeps/aarch64/tst-auditmod27.c | 180 +++++++++++++++++++++++
>> sysdeps/generic/dl-audit-check.h |  23 +++
>> sysdeps/generic/dl-lookupcfg.h   |   3 +
>> sysdeps/generic/ldsodefs.h       |   7 +-
>> sysdeps/hppa/dl-lookupcfg.h      |   3 +
>> sysdeps/ia64/dl-lookupcfg.h      |   3 +
>> sysdeps/powerpc/dl-lookupcfg.h   |  39 +++++
>> 59 files changed, 2674 insertions(+), 86 deletions(-)
>> create mode 100644 elf/tst-audit21.c
>> create mode 100644 elf/tst-audit23.c
>> create mode 100644 elf/tst-audit23mod.c
>> create mode 100644 elf/tst-audit24a.c
>> create mode 100644 elf/tst-audit24amod1.c
>> create mode 100644 elf/tst-audit24amod2.c
>> create mode 100644 elf/tst-audit24b.c
>> create mode 100644 elf/tst-audit24bmod1.c
>> create mode 100644 elf/tst-audit24bmod2.c
>> create mode 100644 elf/tst-audit24c.c
>> create mode 100644 elf/tst-audit24d.c
>> create mode 100644 elf/tst-audit24dmod1.c
>> create mode 100644 elf/tst-audit24dmod2.c
>> create mode 100644 elf/tst-audit24dmod3.c
>> create mode 100644 elf/tst-audit24dmod4.c
>> create mode 100644 elf/tst-audit25a.c
>> create mode 100644 elf/tst-audit25b.c
>> create mode 100644 elf/tst-audit25mod1.c
>> create mode 100644 elf/tst-audit25mod2.c
>> create mode 100644 elf/tst-audit25mod3.c
>> create mode 100644 elf/tst-audit25mod4.c
>> create mode 100644 elf/tst-auditmod21a.c
>> create mode 100644 elf/tst-auditmod21b.c
>> create mode 100644 elf/tst-auditmod23.c
>> create mode 100644 elf/tst-auditmod24.h
>> create mode 100644 elf/tst-auditmod24a.c
>> create mode 100644 elf/tst-auditmod24b.c
>> create mode 100644 elf/tst-auditmod24c.c
>> create mode 100644 elf/tst-auditmod24d.c
>> create mode 100644 elf/tst-auditmod25.c
>> create mode 100644 sysdeps/aarch64/dl-audit-check.h
>> create mode 100644 sysdeps/aarch64/tst-audit26.c
>> create mode 100644 sysdeps/aarch64/tst-audit26mod.c
>> create mode 100644 sysdeps/aarch64/tst-audit26mod.h
>> create mode 100644 sysdeps/aarch64/tst-audit27.c
>> create mode 100644 sysdeps/aarch64/tst-audit27mod.c
>> create mode 100644 sysdeps/aarch64/tst-audit27mod.h
>> create mode 100644 sysdeps/aarch64/tst-auditmod26.c
>> create mode 100644 sysdeps/aarch64/tst-auditmod27.c
>> create mode 100644 sysdeps/generic/dl-audit-check.h
>> create mode 100644 sysdeps/powerpc/dl-lookupcfg.h
>> 
> 
> 
> -- 
> Cheers,
> Carlos.
> 

      reply	other threads:[~2022-02-01  7:53 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-25 18:36 Adhemerval Zanella
2022-01-25 18:36 ` [PATCH v12 1/4] elf: Add la_activity during application exit Adhemerval Zanella
2022-01-26 11:42   ` Florian Weimer
2022-01-26 12:18     ` Adhemerval Zanella
2022-01-26 12:25       ` Florian Weimer
2022-02-01  4:21   ` Carlos O'Donell
2022-02-01 13:30     ` Adhemerval Zanella
2022-01-25 18:36 ` [PATCH v12 2/4] elf: Fix initial-exec TLS access on audit modules (BZ #28096) Adhemerval Zanella
2022-02-01  5:29   ` Carlos O'Donell
2022-02-01 13:35     ` Adhemerval Zanella
2022-01-25 18:36 ` [PATCH v12 3/4] elf: Issue la_symbind for bind-now (BZ #23734) Adhemerval Zanella
2022-02-01  6:06   ` Carlos O'Donell
2022-02-01 13:47     ` Adhemerval Zanella
2022-01-25 18:37 ` [PATCH v12 4/4] elf: Fix runtime linker auditing on aarch64 (BZ #26643) Adhemerval Zanella
2022-02-01  6:19   ` Carlos O'Donell
2022-02-01 14:11     ` Adhemerval Zanella
2022-02-01  6:45 ` [PATCH v12 0/4] Multiple rtld-audit fixes Carlos O'Donell
2022-02-01  7:53   ` John Mellor-Crummey [this message]

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=5B3A03FD-FBAD-4AC1-89B5-71D7007BB7E1@rice.edu \
    --to=johnmc@rice.edu \
    --cc=adhemerval.zanella@linaro.org \
    --cc=carlos@redhat.com \
    --cc=jma14@rice.edu \
    --cc=libc-alpha@sourceware.org \
    --cc=woodard@redhat.com \
    /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).