From patchwork Mon Apr 10 08:20:46 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maninder Singh X-Patchwork-Id: 19932 Received: (qmail 36083 invoked by alias); 10 Apr 2017 08:22:17 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 36055 invoked by uid 89); 10 Apr 2017 08:22:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=H*r:sk:2017041, lazy, acquire, Hx-spam-relays-external:ESMTPA X-HELO: epoutp01.samsung.com From: Maninder Singh To: libc-alpha@sourceware.org, triegel@redhat.com, szabolcs.nagy@arm.com Cc: pankaj.m@samsung.com, ajeet.y@samsung.com, a.sahrawat@samsung.com, lalit.mohan@samsung.com, akhilesh.k@samsung.com, hakbong5.lee@samsung.com, Maninder Singh , Vaneet Narang Subject: [PATCH v4] dl-load: add memory barrier before updating the next Date: Mon, 10 Apr 2017 13:50:46 +0530 Message-Id: <1491812446-7350-1-git-send-email-maninder1.s@samsung.com> X-CMS-MailID: 20170410082212epcas5p4cb83ce1c13504510c6458ccd0eb81c8d X-Msg-Generator: CA X-Sender-IP: 182.195.40.14 X-Local-Sender: =?UTF-8?B?TWFuaW5kZXIgU2luZ2gbU1JJLURlbGhpLVBsYXRmb3JtIFMv?= =?UTF-8?B?VyAxIFRlYW0b7IK87ISx7KCE7J6QG0xlYWQgRW5naW5lZXI=?= X-Global-Sender: =?UTF-8?B?TWFuaW5kZXIgU2luZ2gbU1JJLURlbGhpLVBsYXRmb3JtIFMv?= =?UTF-8?B?VyAxIFRlYW0bU2Ftc3VuZ8KgRWxlY3Ryb25pY3PCoBtMZWFkIEVuZ2luZWVy?= X-Sender-Code: =?UTF-8?B?QzEwG1NXQUhRG0MxMElEMDJJRDAyODExNQ==?= X-MTR: 20170410082212epcas5p4cb83ce1c13504510c6458ccd0eb81c8d X-EPHeader: CA CMS-TYPE: 105P DLP-Filter: Pass X-CFilter-Loop: Reflected X-CMS-RootMailID: 20170410082212epcas5p4cb83ce1c13504510c6458ccd0eb81c8d X-RootMTR: 20170410082212epcas5p4cb83ce1c13504510c6458ccd0eb81c8d References: [BZ #21349]: race condition between dl_open and rtld lazy symbol resolve. Issue Fix: race condition between add_name_to_object & _dl_name_match_p. One threads calling dlopen which further calls add_name_to_object & other thread trying to resolve RTLD_LAZY symbols through _dl_runtime_resolve which further calls. _dl_name_match_p checks if libname->next is valid, then it assumes libname->next->name to be valid. Also add_name_to_object initialized name first and then sets valid next pointer. This patch avoids any reorder of instruction when next is set before name to avoid any race. so as suggested by Torvald Riegel : "Add C11 memory barrier before updating the liblist next and add comments for barriers." Signed-off-by: Vaneet Narang Signed-off-by: Maninder Singh --- v1 -> v2: use C11 atomics rather than direct memory barriers. v2 -> v3: use comments for barriers and enter Bugzilla ID. v3 -> v4: remove extra parens. elf/dl-load.c | 5 ++++- elf/dl-misc.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/elf/dl-load.c b/elf/dl-load.c index a5318f9..03c6afb 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -418,7 +418,10 @@ add_name_to_object (struct link_map *l, const char *name) newname->name = memcpy (newname + 1, name, name_len); newname->next = NULL; newname->dont_free = 0; - lastp->next = newname; + /* We need release memory order here because we need to synchronize + with other thread doing _dl_runtime_resolve which calls _dl_name_match_p + to traverse all names added to libname_list */ + atomic_store_release (&lastp->next, newname); } /* Standard search directories. */ diff --git a/elf/dl-misc.c b/elf/dl-misc.c index 1e9a6ee..a26d6f6 100644 --- a/elf/dl-misc.c +++ b/elf/dl-misc.c @@ -295,7 +295,10 @@ _dl_name_match_p (const char *name, const struct link_map *map) if (strcmp (name, runp->name) == 0) return 1; else - runp = runp->next; + /* We need to acquire memory order here because we need to synchronize + with other thread calling dlopen and adding new name to libname_list + through add_name_to_object */ + runp = atomic_load_acquire (&runp->next); return 0; }