From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb36.google.com (mail-yb1-xb36.google.com [IPv6:2607:f8b0:4864:20::b36]) by sourceware.org (Postfix) with ESMTPS id 2837B3857428 for ; Thu, 2 Jun 2022 05:34:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2837B3857428 Received: by mail-yb1-xb36.google.com with SMTP id g4so6486796ybf.12 for ; Wed, 01 Jun 2022 22:34:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=jyKCQsrgbJe0zLOsBAKx2YDAcr9FL0i4nN0Cg9C1sfY=; b=qFlBxwyogTkKmgI7SxT1xqA46R4iLG8qUw5RmSu5iK1g34HicV8ycVq/j/PF+SZC+1 kuEYDhbJMD4vyxluUlEd/YaFoFAkfxd4PM8vl1oRukTW7bhleG/QO9aw7gUWVhyR/oUr YX5Goqu9XzexMQbfF60nsmJ94p3FxvfxYzvTGn3dkifslmhW47trVM3/8vqie7C+oPaS UYaQ+YrDrH9F/93+31hEV9qN7FUSinvQdBqozYKFaRiC+UrGzPmAjBfJo9NMwled5nxV klLrQMJRfYSaopYawx/bsy+HpKPm5jf9SdAq+aTAfSBBGX/UFpYQgOWMjOlq9Y4dJlx+ 8i6w== X-Gm-Message-State: AOAM532fzaCiDfMxhg0N0moPJva4/OnPoBH9aF+UIkM7B8Qb1Ra+Z8RZ soHRsJZp50ZWFI47pnDVqpmg3bifs4ac1gg3s8k+eRX2ko2d+w== X-Google-Smtp-Source: ABdhPJwFZ9zWcswCxNRvtW+sgbxuBVpTDKCoBs0yRkA2/XFxutwqH14eD7ugVhV0S/JmwdA3QjPbBqmI9ECQBDq0BzY= X-Received: by 2002:a05:6902:348:b0:64a:d339:a75f with SMTP id e8-20020a056902034800b0064ad339a75fmr3435021ybs.63.1654148062488; Wed, 01 Jun 2022 22:34:22 -0700 (PDT) MIME-Version: 1.0 References: <87fskvfcpj.fsf@oldenburg.str.redhat.com> <20220601043048.gb2ioqgbfkr45kj5@google.com> <87r148rgfg.fsf@oldenburg.str.redhat.com> In-Reply-To: <87r148rgfg.fsf@oldenburg.str.redhat.com> From: =?UTF-8?B?RsSBbmctcnXDrCBTw7JuZw==?= Date: Wed, 1 Jun 2022 22:34:11 -0700 Message-ID: Subject: Re: [PATCH] elf: Fix handling of symbol versions which hash to zero (bug 29190) To: Florian Weimer Cc: libc-alpha@sourceware.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-23.6 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Thu, 02 Jun 2022 05:34:24 -0000 On Wed, Jun 1, 2022 at 12:28 AM Florian Weimer wrote: > > * Fangrui Song: > > >>diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c > >>index a42f6d5390..9abe96fd92 100644 > >>--- a/elf/dl-lookup.c > >>+++ b/elf/dl-lookup.c > >>@@ -114,12 +114,22 @@ check_match (const char *const undef_name, > >> /* We can match the version information or use the > >> default one if it is not hidden. */ > >> ElfW(Half) ndx = verstab[symidx] & 0x7fff; > >>- if ((map->l_versions[ndx].hash != version->hash > >>- || strcmp (map->l_versions[ndx].name, version->name)) > >>- && (version->hidden || map->l_versions[ndx].hash > >>- || (verstab[symidx] & 0x8000))) > >>- /* It's not the version we want. */ > >>- return NULL; > >>+ if (map->l_versions[ndx].hash == version->hash > >>+ && strcmp (map->l_versions[ndx].name, version->name) == 0) > >>+ /* This is an exact version match. Return the symbol below. */ > >>+ ; > >>+ else > >>+ { > >>+ if (!version->hidden > >>+ && map->l_versions[ndx].name[0] == '\0' > >>+ && (verstab[symidx] & 0x8000) == 0 > >>+ && (*num_versions)++ == 0) > > > > (*num_versions)++ == 0 can be removed. > > That's not quite clear to me. I kept it for consistency with the code > below. But I can remove it. > > > map->l_versions[ndx].hash => map->l_versions[ndx].name[0] == '\0' > > looks good to me. > > > > I think version->hidden is to differentiate relocation resolving and > > dlvsym. Since dlsym and dlvsym are differentiated by `version`, > > we could introduce a flag to differentiate relocation resolving from > > dlsym/dlvsym. Then version->hidden can be made clearer. > > Given this is a versioned lookup, I'm not sure we want to different > behavior for dlvsym and dlsym. So we should probably drop the condition > altogether. See my reply on https://sourceware.org/pipermail/libc-alpha/2022-June/139283.html . I think we should keep distinct behaviors for dlvsym and dlsym. > Should I send a v2 with these changes? If you drop (*num_versions)++ == 0 , this can be committed as-is. Reviewed-by: Fangrui Song > Thanks, > Florian >