From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x735.google.com (mail-qk1-x735.google.com [IPv6:2607:f8b0:4864:20::735]) by sourceware.org (Postfix) with ESMTPS id D0E893858D35 for ; Tue, 16 Nov 2021 13:14:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D0E893858D35 Received: by mail-qk1-x735.google.com with SMTP id t6so13240720qkg.1 for ; Tue, 16 Nov 2021 05:14: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=LSR3LAvW+RywVP2AZwZjEFI8lD6q7SIJHHahNaQfZac=; b=jOLZjkPST9Gz3JEjAK7mmWoFLUGnDfHwsNqwgyRcrf8/vXlMS0Yb+EsBVAy25d29Xs 2V6Xw/9X8Itp2hrJZN3w8VEQ1icaJXBY7RnStYvu/2MlWHYpMk7sy8gW8o7W2Kl5atZk FM9VMesYPjEJ2eTtTl9tXmNwlB8zuNcTGQ23gv2aU3tDw/MhgMcngDeLx2U2/CWafh7b I1kCUx08cfgrSkmNKfyUwnYTMMaMGosg3CuRe5q78vSJztzHe0rpheTz6V7z8P/oWsvF 84FofS1gmHazCmNIEFb38n3FFpmRsJvkznAuQrCJwserIX8tbgMfasFK+NJgu0hNd//Z EuxQ== X-Gm-Message-State: AOAM5338I5nR/0BK6oFivFOOJZhdb6stKl7oYWB8zifZrUuKN1OrD4Wk jMUY39cA6Tmd3LjoYDYBmfAyUA== X-Google-Smtp-Source: ABdhPJzkQPLcZ0NkflZWPMbFzpFAquGe7isDWHGqB+c2Hz1gUanEX/GO/M+m+nSyMZXnKM2aXIQXgg== X-Received: by 2002:a05:620a:1192:: with SMTP id b18mr6000381qkk.393.1637068443401; Tue, 16 Nov 2021 05:14:03 -0800 (PST) Received: from ?IPV6:2804:431:c7ca:66dc:190:a0a5:4184:e499? ([2804:431:c7ca:66dc:190:a0a5:4184:e499]) by smtp.gmail.com with ESMTPSA id h68sm8391531qkf.126.2021.11.16.05.14.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 16 Nov 2021 05:14:03 -0800 (PST) Message-ID: <37adfe77-f066-8046-2fe4-5026d3bdac1c@linaro.org> Date: Tue, 16 Nov 2021 10:14:00 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 Subject: Re: [PATCH v6 01/20] elf: Suppress audit calls when a (new) namespace is empty (BZ #28062) Content-Language: en-US To: Florian Weimer Cc: libc-alpha@sourceware.org, John Mellor-Crummey , Ben Woodard References: <20211115183734.531155-1-adhemerval.zanella@linaro.org> <20211115183734.531155-2-adhemerval.zanella@linaro.org> <87ee7hfdk7.fsf@oldenburg.str.redhat.com> From: Adhemerval Zanella In-Reply-To: <87ee7hfdk7.fsf@oldenburg.str.redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP 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, 16 Nov 2021 13:14:06 -0000 On 15/11/2021 16:01, Florian Weimer wrote: > * Adhemerval Zanella: > >> For a new Lmid_t the namespace link_map list are empty, so it requires >> to check if before using it. This can happen for when audit module >> is used along with dlmopen. > > Looks like you forgot to update the commit message. Indeed, the title is also misleading. I have changed to: elf: Move la_activity (LA_ACT_ADD) after _dl_add_to_namespace_list() (BZ #28062) It ensures that the the namespace is guaranteed to not be empty. > > My patch linter also flags these: > > + pid_t (*s)(void) = xdlsym (h, "getpid"); > + int (*foo)(void) = xdlsym (h, "foo"); > > Missing space before “(void)”. Ack. > >> diff --git a/elf/dl-load.c b/elf/dl-load.c >> index 9f4fa9617d..9538bcb7dc 100644 >> --- a/elf/dl-load.c >> +++ b/elf/dl-load.c > >> @@ -1515,6 +1479,42 @@ cannot enable executable stack as shared object requires"); >> /* Now that the object is fully initialized add it to the object list. */ >> _dl_add_to_namespace_list (l, nsid); >> >> + /* Signal that we are going to add new objects. */ >> + if (r->r_state == RT_CONSISTENT) >> + { >> +#ifdef SHARED >> + /* Auditing checkpoint: we are going to add new objects. */ >> + if ((mode & __RTLD_AUDIT) == 0 >> + && __glibc_unlikely (GLRO(dl_naudit) > 0)) >> + { >> + struct link_map *head = GL(dl_ns)[nsid]._ns_loaded; >> + /* Do not call the functions for any auditing object. */ >> + if (head->l_auditing == 0) >> + { >> + struct audit_ifaces *afct = GLRO(dl_audit); >> + for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt) >> + { >> + if (afct->activity != NULL) >> + afct->activity (&link_map_audit_state (head, cnt)->cookie, >> + LA_ACT_ADD); >> + >> + afct = afct->next; >> + } >> + } >> + } >> +#endif > > Please add a comment that this happens after _dl_add_to_namespace_list, > so that the namespace is guaranteed not to be empty. I have changed to: diff --git a/elf/dl-load.c b/elf/dl-load.c index 9538bcb7dc..240b5efb45 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -1483,7 +1483,9 @@ cannot enable executable stack as shared object requires"); if (r->r_state == RT_CONSISTENT) { #ifdef SHARED - /* Auditing checkpoint: we are going to add new objects. */ + /* Auditing checkpoint: we are going to add new objects. Since this + is called after _dl_add_to_namespace_list() the namespace is + guaranteed to not be empty. */ if ((mode & __RTLD_AUDIT) == 0 && __glibc_unlikely (GLRO(dl_naudit) > 0)) {