[PATCH] elf: Fix handling of symbol versions which hash to zero (bug 29190)

[Florian Weimer]

* 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.

Should I send a v2 with these changes?

Thanks,
Florian