From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x330.google.com (mail-ot1-x330.google.com [IPv6:2607:f8b0:4864:20::330]) by sourceware.org (Postfix) with ESMTPS id 7A5393858C74 for ; Tue, 1 Feb 2022 13:31:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7A5393858C74 Received: by mail-ot1-x330.google.com with SMTP id j38-20020a9d1926000000b0059fa6de6c71so16192992ota.10 for ; Tue, 01 Feb 2022 05:31:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=+OFSX5r8SMlLYm7VMSsaeDBjXtpPZKDZBFm4VbIaaUU=; b=K9arwEGWZrtWpM0UuMCusvCFLvwe1MO7P+aPfZOmFRkTINAk6UgEKROY1paKuiwKR5 fQNrYl6MOL6Yfv8k/DJXv1OhA+HzktxXoqkGouRhllX/W6jPChPX1mZxQcPKk0oJndR8 ozpWGfY7sIEXe6KcGsju02E9EZmaj96EKkR0fgtzan/ZYURcBNrws33rdsI/t3ctJxv7 XLOY4+6sqUWwZ0IO6Te4dCqmYfoedmvmuKFT44y0jyxM5bDk3wANRdXesi0O9++xiAdT iUQmLel4fbpbjcJVHF6sn5o6QOTdLgLb/kRCcBkL0wp8W6eHGhlwAV0Njg8/UTPRh4oR x2Jw== X-Gm-Message-State: AOAM5316RmXxyzlqdTg0GrkHjqJFWSyJMILC0wxjAPTHlXXYlHeGv+J0 6CRQnwHOkg651qSQLCTw28/M2w== X-Google-Smtp-Source: ABdhPJzqX/k9cmiYwR/FhcuqTv71ZVMN0ylZ8rLlrHufsnbGCCHnfqqwv2v9RioPASG8meqQlMqvgg== X-Received: by 2002:a05:6830:12c1:: with SMTP id a1mr1680006otq.374.1643722262152; Tue, 01 Feb 2022 05:31:02 -0800 (PST) Received: from ?IPV6:2804:431:c7ca:709a:f3cb:a92a:e1ce:d27d? ([2804:431:c7ca:709a:f3cb:a92a:e1ce:d27d]) by smtp.gmail.com with ESMTPSA id h9sm7433451otk.42.2022.02.01.05.31.00 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Feb 2022 05:31:01 -0800 (PST) Message-ID: Date: Tue, 1 Feb 2022 10:30:59 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.1 Subject: Re: [PATCH v12 1/4] elf: Add la_activity during application exit Content-Language: en-US To: Carlos O'Donell , libc-alpha@sourceware.org, jma14 Cc: John Mellor-Crummey , Ben Woodard References: <20220125183700.1280931-1-adhemerval.zanella@linaro.org> <20220125183700.1280931-2-adhemerval.zanella@linaro.org> <4d957740-c220-8437-e201-cbf6162093be@redhat.com> From: Adhemerval Zanella In-Reply-To: <4d957740-c220-8437-e201-cbf6162093be@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Feb 2022 13:31:08 -0000 On 01/02/2022 01:21, Carlos O'Donell wrote: > On 1/25/22 13:36, Adhemerval Zanella wrote: >> diff --git a/elf/tst-audit23.c b/elf/tst-audit23.c >> new file mode 100644 >> index 0000000000..3eea322051 >> --- /dev/null >> +++ b/elf/tst-audit23.c >> @@ -0,0 +1,239 @@ >> +/* Check DT_AUDIT la_objopen and la_objclose for all objects. > > This does not check DT_AUDIT, which is set only by the static linker for --audit? > > Suggest: > Check for expected la_objopen and la_objeclose for all objects. > Ack. >> + Copyright (C) 2022 Free Software Foundation, Inc. >> + This file is part of the GNU C Library. >> + >> + The GNU C Library is free software; you can redistribute it and/or >> + modify it under the terms of the GNU Lesser General Public >> + License as published by the Free Software Foundation; either >> + version 2.1 of the License, or (at your option) any later version. >> + >> + The GNU C Library is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + Lesser General Public License for more details. >> + >> + You should have received a copy of the GNU Lesser General Public >> + License along with the GNU C Library; if not, see >> + . */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +static int restart; >> +#define CMDLINE_OPTIONS \ >> + { "restart", no_argument, &restart, 1 }, >> + >> +static int >> +handle_restart (void) >> +{ >> + xdlopen ("tst-audit23mod.so", RTLD_NOW); >> + xdlmopen (LM_ID_NEWLM, LIBC_SO, RTLD_NOW); >> + >> + return 0; >> +} >> + >> +static inline bool >> +startswith (const char *str, const char *pre) >> +{ >> + size_t lenpre = strlen (pre); >> + size_t lenstr = strlen (str); >> + return lenstr >= lenpre && memcmp (pre, str, lenpre) == 0; >> +} >> + >> +static inline bool >> +is_vdso (const char *str) >> +{ >> + return startswith (str, "linux-gate") >> + || startswith (str, "linux-vdso"); >> +} >> + >> +static int >> +do_test (int argc, char *argv[]) >> +{ >> + /* We must have either: >> + - One our fource parameters left if called initially: > > s/One our fource/One or four/g. > Ack. >> + + path to ld.so optional >> + + "--library-path" optional >> + + the library path optional >> + + the application name */ >> + if (restart) >> + return handle_restart (); >> + >> + char *spargv[9]; >> + int i = 0; >> + for (; i < argc - 1; i++) >> + spargv[i] = argv[i + 1]; >> + spargv[i++] = (char *) "--direct"; >> + spargv[i++] = (char *) "--restart"; >> + spargv[i] = NULL; >> + TEST_VERIFY_EXIT (i < array_length (spargv)); >> + > > OK. Prepare to re-run ourselves with auditor. > >> + setenv ("LD_AUDIT", "tst-auditmod23.so", 0); >> + struct support_capture_subprocess result >> + = support_capture_subprogram (spargv[0], spargv); >> + support_capture_subprocess_check (&result, "tst-audit22", 0, sc_allow_stderr); >> + >> + /* The expected la_objopen/la_objclose: >> + 1. executable >> + 2. loader >> + 3. libc.so >> + 4. tst-audit23mod.so >> + 5. libc.so (LM_ID_NEWLM). >> + 6. vdso (optional and ignored). */ >> + enum { max_objs = 6 }; >> + struct la_obj_t >> + { >> + char *lname; >> + uintptr_t laddr; >> + Lmid_t lmid; >> + bool closed; >> + } objs[max_objs] = { [0 ... max_objs-1] = { .closed = false } }; >> + size_t nobjs = 0; >> + >> + /* The expected namespaces are one for the audit module, one for the >> + application, and another for the dlmopen on handle_restart. */ >> + enum { max_ns = 3 }; >> + uintptr_t acts[max_ns] = { 0 }; >> + size_t nacts = 0; >> + int last_act = -1; >> + uintptr_t last_act_cookie = -1; >> + bool seen_first_objclose = false; >> + >> + FILE *out = fmemopen (result.err.buffer, result.err.length, "r"); >> + TEST_VERIFY (out != NULL); >> + char *buffer = NULL; >> + size_t buffer_length = 0; >> + while (xgetline (&buffer, &buffer_length, out)) >> + { >> + if (startswith (buffer, "la_activity: ")) >> + { >> + uintptr_t cookie; >> + int this_act; >> + int r = sscanf (buffer, "la_activity: %d %"SCNxPTR"", &this_act, >> + &cookie); >> + TEST_COMPARE (r, 2); >> + >> + /* The cookie identifies the object at the head of the link map, >> + so we only add a new namespace if it changes from previous > > s/from/from the/g > Ack. >> + one. This work since dlmopen is the last in the test body. */ > > s/work/works/g Ack. > >> + if (cookie != last_act_cookie && last_act_cookie != -1) >> + TEST_COMPARE (last_act, LA_ACT_CONSISTENT); >> + >> + if (this_act == LA_ACT_ADD && acts[nacts] != cookie) >> + { >> + acts[nacts++] = cookie; >> + last_act_cookie = cookie; >> + } >> + /* The LA_ACT_DELETE is called in the reverse order of LA_ACT_ADD >> + at program termination (if the tests adds a dlclose or a library >> + with extra dependencies this require to be adapted). */ > > s/require/will need/g Ack. > > Florian's fix to make dlclose order consistent will make this consistent too! > >> + else if (this_act == LA_ACT_DELETE) >> + { >> + last_act_cookie = acts[--nacts]; >> + TEST_COMPARE (acts[nacts], cookie); >> + acts[nacts] = 0; >> + } >> + else if (this_act == LA_ACT_CONSISTENT) >> + { >> + TEST_COMPARE (cookie, last_act_cookie); >> + >> + /* LA_ACT_DELETE must always be followed by an la_objclose. */ >> + if (last_act == LA_ACT_DELETE) >> + TEST_COMPARE (seen_first_objclose, true); >> + else >> + TEST_COMPARE (last_act, LA_ACT_ADD); >> + } >> + >> + last_act = this_act; >> + seen_first_objclose = false; >> + } >> + else if (startswith (buffer, "la_objopen: ")) >> + { >> + char *lname; >> + uintptr_t laddr; >> + Lmid_t lmid; >> + uintptr_t cookie; >> + int r = sscanf (buffer, "la_objopen: %"SCNxPTR" %ms %"SCNxPTR" %ld", >> + &cookie, &lname, &laddr, &lmid); >> + TEST_COMPARE (r, 4); >> + >> + /* la_objclose is not triggered by vDSO because glibc does not >> + unload it. */ > > OK. > >> + if (is_vdso (lname)) >> + continue; >> + if (nobjs == max_objs) >> + FAIL_EXIT1 ("non expected la_objopen: %s %"PRIxPTR" %ld", >> + lname, laddr, lmid); >> + objs[nobjs].lname = lname; >> + objs[nobjs].laddr = laddr; >> + objs[nobjs].lmid = lmid; >> + objs[nobjs].closed = false; >> + nobjs++; >> + >> + /* This indirectly checks that la_objopen always come before > > s/come/comes/g Ack. > >> + la_objclose btween la_activity calls. */ >> + seen_first_objclose = false; >> + } >> + else if (startswith (buffer, "la_objclose: ")) >> + { >> + char *lname; >> + uintptr_t laddr; >> + Lmid_t lmid; >> + uintptr_t cookie; >> + int r = sscanf (buffer, "la_objclose: %"SCNxPTR" %ms %"SCNxPTR" %ld", >> + &cookie, &lname, &laddr, &lmid); >> + TEST_COMPARE (r, 4); >> + >> + for (size_t i = 0; i < nobjs; i++) >> + { >> + if (strcmp (lname, objs[i].lname) == 0 && lmid == objs[i].lmid) >> + { >> + TEST_COMPARE (objs[i].closed, false); >> + objs[i].closed = true; >> + break; >> + } >> + } >> + >> + /* la_objclose should be called after la_activity(LA_ACT_DELETE) for >> + the closed object's namespace. */ > > OK. Agreed. > >> + TEST_COMPARE (last_act, LA_ACT_DELETE); >> + if (!seen_first_objclose) >> + { >> + TEST_COMPARE (last_act_cookie, cookie); >> + seen_first_objclose = true; >> + } >> + } >> + } >> + >> + for (size_t i = 0; i < nobjs; i++) >> + { >> + TEST_COMPARE (objs[i].closed, true); >> + free (objs[i].lname); >> + } >> + >> + /* la_activity(LA_ACT_CONSISTENT) should be the last callback received. >> + Since only one link map may be not-CONSISTENT at a time, this also >> + ensures la_activity(LA_ACT_CONSISTENT) is the last callback received >> + for every namespace. */ >> + TEST_COMPARE (last_act, LA_ACT_CONSISTENT); >> + >> + free (buffer); >> + xfclose (out); >> + >> + return 0; >> +} >> + >> +#define TEST_FUNCTION_ARGV do_test >> +#include >> diff --git a/elf/tst-audit23mod.c b/elf/tst-audit23mod.c >> new file mode 100644 >> index 0000000000..9a116ff682 >> --- /dev/null >> +++ b/elf/tst-audit23mod.c >> @@ -0,0 +1,23 @@ >> +/* Extra modules for tst-audit23 > > s/modules/module/g Ack. > >> + Copyright (C) 2022 Free Software Foundation, Inc. >> + This file is part of the GNU C Library. >> + >> + The GNU C Library is free software; you can redistribute it and/or >> + modify it under the terms of the GNU Lesser General Public >> + License as published by the Free Software Foundation; either >> + version 2.1 of the License, or (at your option) any later version. >> + >> + The GNU C Library is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + Lesser General Public License for more details. >> + >> + You should have received a copy of the GNU Lesser General Public >> + License along with the GNU C Library; if not, see >> + . */ >> + >> +int >> +foo (void) >> +{ >> + return 0; >> +} > > OK. > >> diff --git a/elf/tst-auditmod23.c b/elf/tst-auditmod23.c >> new file mode 100644 >> index 0000000000..42eccae11b >> --- /dev/null >> +++ b/elf/tst-auditmod23.c >> @@ -0,0 +1,74 @@ >> +/* Audit modules loaded by tst-audit23. > > s/modules/module/g. Ack. > >> + Copyright (C) 2022 Free Software Foundation, Inc. >> + This file is part of the GNU C Library. >> + >> + The GNU C Library is free software; you can redistribute it and/or >> + modify it under the terms of the GNU Lesser General Public >> + License as published by the Free Software Foundation; either >> + version 2.1 of the License, or (at your option) any later version. >> + >> + The GNU C Library is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + Lesser General Public License for more details. >> + >> + You should have received a copy of the GNU Lesser General Public >> + License along with the GNU C Library; if not, see >> + . */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +unsigned int >> +la_version (unsigned int version) >> +{ >> + return LAV_CURRENT; >> +} >> + >> +struct map_desc_t >> +{ >> + char *lname; >> + uintptr_t laddr; >> + Lmid_t lmid; >> +}; >> + >> +void >> +la_activity (uintptr_t *cookie, unsigned int flag) >> +{ >> + fprintf (stderr, "%s: %d %"PRIxPTR"\n", __func__, flag, (uintptr_t) cookie); >> +} >> + >> +unsigned int >> +la_objopen (struct link_map *map, Lmid_t lmid, uintptr_t *cookie) >> +{ >> + const char *l_name = map->l_name[0] == '\0' ? "mainapp" : map->l_name; >> + fprintf (stderr, "%s: %"PRIxPTR" %s %"PRIxPTR" %ld\n", __func__, >> + (uintptr_t) cookie, l_name, map->l_addr, lmid); >> + >> + struct map_desc_t *map_desc = malloc (sizeof (struct map_desc_t)); >> + if (map_desc == NULL) >> + abort (); >> + >> + map_desc->lname = strdup (l_name); >> + map_desc->laddr = map->l_addr; >> + map_desc->lmid = lmid; >> + >> + *cookie = (uintptr_t) map_desc; > > OK. > >> + >> + return 0; >> +} >> + >> +unsigned int >> +la_objclose (uintptr_t *cookie) >> +{ >> + struct map_desc_t *map_desc = (struct map_desc_t *) *cookie; >> + fprintf (stderr, "%s: %"PRIxPTR" %s %"PRIxPTR" %ld\n", __func__, >> + (uintptr_t) cookie, map_desc->lname, map_desc->laddr, >> + map_desc->lmid); >> + >> + return 0; >> +} > >