patchelf --set-rpath adds external unresolved symbol XXX to binary

[tomaszrostanskithales]

I'm building strongswan on Openwrt 21.02 for newport platform.
I noticed that the swanctl binary when executed on target board crashes with:

root@OpenWrt:~# swanctl
Error relocating /usr/lib/ipsec/libstrongswan.so.0: XXX: symbol not found
Error relocating /usr/sbin/swanctl: lib: symbol not found

The binaries are compiled with gcc 8.4.0, binutils2.34, musl 1.1.24.
The unstripped binaries are ok and does not contain XXX symbol. When executed on target platform works fine.

However when I run patchelf --set-rpath '' libvici.so.0.0.0 or any other swanctl binary, then the unresolved XXX symbol appears in the library.

[Mic92]

When patchelf replaces sections it uses X invalidate them:

patchelf/src/patchelf.cc

Line 702 in c62988a

memset(contents + rdi(shdr.sh_offset), 'X', rdi(shdr.sh_size));

Can you reproduce this bug on x86 or at least aarch64?
Can you provide the binaries and libraries in question?

My current theory is what happens is that there was no rpath set before and now by doing patchelf --set-rpath you are introducing a new section - If there was an rpath before it would have been truncated instead. So the question is, what are you trying to do here?

[tomaszrostanskithales]

The patchelf (from git master, commit c62988a) was initially built and executed on x86_64, the binaries built are for aarch64.
I have also cross-compiled it for aarch64 and executed on target platform. The result is the same.

patchelf --print-rpath executed on each binary prints:
/mnt/workspace/acc5g/openwrt-builder/dl/openwrt-21.02/staging_dir/toolchain-aarch64_generic_gcc-8.4.0_musl/bin//../../lib:/mnt/workspace/acc5g/openwrt-builder/dl/openwrt-21.02/staging_dir/toolchain-aarch64_generic_gcc-8.4.0_musl/bin//../../usr/lib:/usr/lib/ipsec

prebuilt swanctl and libraries are here:
https://github.com/tomaszrostanskithales/patchelf-issue-315

[Mic92]

Thanks. That is helpful.
patchelf writes partially to the .dynstr section or it writes the wrong size for the .dynstr section when updating the binary:

Here you can see your XXXX

$  objdump -s -j .dynstr libvici.so.0.0.0 
libvici.so.0.0.0:     file format elf64-little

Contents of section .dynstr:
 0ca0 005f6669 6e69005f 5f637861 5f66696e  ._fini.__cxa_fin
 0cb0 616c697a 65005f5f 64657265 67697374  alize.__deregist
 0cc0 65725f66 72616d65 5f696e66 6f005f5f  er_frame_info.__
 0cd0 72656769 73746572 5f667261 6d655f69  register_frame_i
 0ce0 6e666f00 66726565 00657870 6c696369  nfo.free.explici
 0cf0 745f627a 65726f00 6368756e 6b5f656d  t_bzero.chunk_em
 0d00 70747900 6d616c6c 6f630062 696f5f72  pty.malloc.bio_r
 0d10 65616465 725f6372 65617465 006d656d  eader_create.mem
 0d20 73657400 656e756d 65726174 6f725f65  set.enumerator_e
 0d30 6e756d65 72617465 5f646566 61756c74  numerate_default
 0d40 00627569 6c74696e 5f667072 696e7466  .builtin_fprintf
 0d50 00636875 6e6b5f70 72696e74 61626c65  .chunk_printable
 0d60 00737472 64757000 73747263 6d700062  .strdup.strcmp.b
 0d70 75696c74 696e5f76 736e7072 696e7466  uiltin_vsnprintf
 0d80 005f5f73 7461636b 5f63686b 5f677561  .__stack_chk_gua
 0d90 72640073 74726368 72005f5f 73746163  rd.strchr.__stac
 0da0 6b5f6368 6b5f6661 696c0062 75696c74  k_chk_fail.built
 0db0 696e5f73 6e707269 6e746600 73657474  in_snprintf.sett
 0dc0 696e6773 5f76616c 75655f61 735f626f  ings_value_as_bo
 0dd0 6f6c005f 5f657272 6e6f5f6c 6f636174  ol.__errno_locat
 0de0 696f6e00 73747274 6f6c0073 74726c65  ion.strtol.strle
 0df0 6e007374 726e6475 70007669 63695f73  n.strndup.vici_s
 0e00 7472696e 67696679 00766963 695f7665  tringify.vici_ve
 0e10 72696679 5f747970 65006462 67007669  rify_type.dbg.vi
 0e20 63695f74 7970655f 6e616d65 73007669  ci_type_names.vi
 0e30 63695f6d 65737361 67655f63 72656174  ci_message_creat
 0e40 655f6672 6f6d5f64 61746100 6c696e6b  e_from_data.link
 0e50 65645f6c 6973745f 63726561 74650076  ed_list_create.v
 0e60 6963695f 6d657373 6167655f 63726561  ici_message_crea
 0e70 74655f66 726f6d5f 656e756d 65726174  te_from_enumerat
 0e80 6f720076 6963695f 6275696c 6465725f  or.vici_builder_
 0e90 63726561 74650076 6963695f 6d657373  create.vici_mess
 0ea0 6167655f 63726561 74655f66 726f6d5f  age_create_from_
 0eb0 61726773 0062696f 5f777269 7465725f  args.bio_writer_
 0ec0 63726561 74650076 6963695f 63657274  create.vici_cert
 0ed0 5f696e66 6f5f6672 6f6d5f73 74720073  _info_from_str.s
 0ee0 74726361 7365636d 70006e74 6f686c00  trcasecmp.ntohl.
 0ef0 76696369 5f636f6e 6e656374 006c6962  vici_connect.lib
 0f00 00686173 68746162 6c655f65 7175616c  .hashtable_equal
 0f10 735f7374 72006861 73687461 626c655f  s_str.hashtable_
 0f20 68617368 5f737472 00686173 68746162  hash_str.hashtab
 0f30 6c655f63 72656174 65006d75 7465785f  le_create.mutex_
 0f40 63726561 74650063 6f6e6476 61725f63  create.condvar_c
 0f50 72656174 65007669 63695f64 6973636f  reate.vici_disco
 0f60 6e6e6563 74007669 63695f62 6567696e  nnect.vici_begin
 0f70 00766963 695f6265 67696e5f 73656374  .vici_begin_sect
 0f80 696f6e00 76696369 5f656e64 5f736563  ion.vici_end_sec
 0f90 74696f6e 00766963 695f6164 645f6b65  tion.vici_add_ke
 0fa0 795f7661 6c756500 76696369 5f616464  y_value.vici_add
 0fb0 5f6b6579 5f76616c 75656600 76696369  _key_valuef.vici
 0fc0 5f626567 696e5f6c 69737400 76696369  _begin_list.vici
 0fd0 5f616464 5f6c6973 745f6974 656d0076  _add_list_item.v
 0fe0 6963695f 6164645f 6c697374 5f697465  ici_add_list_ite
 0ff0 6d660076 6963695f 656e645f 6c697374  mf.vici_end_list
 1000 00766963 695f7375 626d6974 0068746f  .vici_submit.hto
 1010 6e6c0076 6963695f 66726565 5f726571  nl.vici_free_req
 1020 00766963 695f6475 6d700076 6963695f  .vici_dump.vici_
 1030 70617273 65007669 63695f70 61727365  parse.vici_parse
 1040 5f6e616d 65007669 63695f70 61727365  _name.vici_parse
 1050 5f6e616d 655f6571 00766963 695f7061  _name_eq.vici_pa
 1060 7273655f 76616c75 65007669 63695f70  rse_value.vici_p
 1070 61727365 5f76616c 75655f73 74720076  arse_value_str.v
 1080 6963695f 70617273 655f6362 00766963  ici_parse_cb.vic
 1090 695f6669 6e640076 6963695f 66696e64  i_find.vici_find
 10a0 5f737472 00766963 695f6669 6e645f69  _str.vici_find_i
 10b0 6e740076 6963695f 66726565 5f726573  nt.vici_free_res
 10c0 00766963 695f7265 67697374 65720076  .vici_register.v
 10d0 6963695f 696e6974 006c6962 72617279  ici_init.library
 10e0 5f696e69 74006462 675f6465 6661756c  _init.dbg_defaul
 10f0 745f7365 745f6c65 76656c00 76696369  t_set_level.vici
 1100 5f646569 6e697400 6c696272 6172795f  _deinit.library_
 1110 6465696e 6974006c 69627374 726f6e67  deinit.libstrong
 1120 7377616e 2e736f2e 30006c69 62676363  swan.so.0.libgcc
 1130 5f732e73 6f2e3100 6c696263 2e736f00  _s.so.1.libc.so.
 1140 6c696276 6963692e 736f2e30 00474c49  libvici.so.0.GLI
 1150 42435f32 2e300000 58585858 58585858  BC_2.0..XXXXXXXX
 1160 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 1170 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 1180 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 1190 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 11a0 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 11b0 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 11c0 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 11d0 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 11e0 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 11f0 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 1200 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 1210 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 1220 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 1230 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 1240 58585858 58585858 58585858 58585858  XXXXXXXXXXXXXXXX
 1250 58585858 58585858 58585858 5800      XXXXXXXXXXXXX.  

[tobiasbrunner]

This seems to be a duplicate of #45.

[Mic92]

Right. But we have a data here that reproduces the issue. So let's keep it open.

[tobiasbrunner]

Sure, I just wanted to point to it because a possible solution was proposed by @dezgeg there.

[tobiasbrunner]

Just to clarify what the problem is: libstrongswan defines a global variable that's called lib. If RPATH ends with lib as well, the linker will not add a separate string for the symbol to .dynstr but just lets the symbol's st_name point to the end of the string added for RPATH. You can see that here with dumpelf -v libstrongswan.so.0.0.0:

...
/* Section Header #3 '.dynsym' 0x7FD90 */
{
         ...
         * Elf64_Sym sym380 = {
         *      .st_name  = 11128,
         *      .st_value = 0x7C888,
         *      .st_size  = 8, (bytes)
         *      .st_info  = 17,
         *      .st_other = 0,
         *      .st_shndx = 20
         * };
         ...
},
...
/* Section Header #4 '.dynstr' 0x7FDD0 */
{
        .sh_name      = 45         ,
        .sh_type      = 3          , /* [SHT_STRTAB] */
        .sh_flags     = 2          ,
        .sh_addr      = 0x5B68     ,
        .sh_offset    = 23400      , /* (bytes) */
        .sh_size      = 11132      , /* (bytes) */
        ...
        /* section dump:
         ...
         * /mnt/workspace/acc5g/openwrt-builder/dl/openwrt-21.02/staging_dir/toolchain-aarch64_generic_gcc-8.4.0_musl/bin//../../lib:/mnt/workspace/acc5g/openwrt-builder/dl/openwrt-21.02/staging_dir/toolchain-aarch64_generic_gcc-8.4.0_musl/bin//../../usr/lib
         */
},

The symbol points to 11128, which is exactly the lib with which the RPATH content and the .dynstr section end (the latter's size is 11132 bytes, 11132 - 1 - 3 = 11128, -1 for the null-terminator).

So when patchelf removes RPATH and overwrites the contents to which it pointed, that also affects the symbol, which now points to the last three X.

I guess patchelf could overwrite stuff more selectively (as suggested in the other issue), or it could check if any symbols point into the area that's to be overwritten and either leave those parts (for RPATH it would just be at the end of the overwritten area as it's a single null-terminated string, so the length in the memset call could just be reduced), or re-add the overwritten parts that are used by any symbols to .dynstr after the overwriting step and modify the st_name of the symbols.

[Mic92]

Checking for symbols that point to RPATH seems like the cleanest option also it requires a bit more code. The alternative would be to not override the old RPATH at all.

[tomaszrostanskithales]

Any progress on that topic? The problem still exists on 0.17.0 release.

I have patched strongswan renaming global "lib" variable to "swlibrary" but the problem still exists.
The corresponding binaries are here for reference:
https://github.com/tomaszrostanskithales/patchelf-issue-315/blob/main/patched-strongswan.tar

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/bit-for-bit-reproducible-images-for-raspberry-pi/26333/1

[brenoguim]

Created a minimal reproducer:
good.cpp

extern "C" int lib;
int good() { return lib; }

$ g++ -fPIC -shared -o libgood.so good.cpp -Wl,-rpath=lib
$ patchelf --set-rpath other --output libbad.so libgood.so

Running nm -D on libgood.so and libbad.so will show that libgood.so that a symbol named lib and libbad.so has a symbol named XXX

[brenoguim]

We have two bugs in this scenario where the rpath shares the string with a symbol:

1. If the new rpath is larger than the current, patchelf will mark the old string as X, tainting the symbol name
2. If the new rpath is smaller or equal the current, patchelf will overwrite it, also tainting the symbol name

Some options that we can choose from:

1. Short term: Add a flag to disable this behavior such that modifyRPath never changes the old string.
2. Medium term: Merge --clean-strtab and tell the user to use --set-rpath together with --clean-strtab and make the modifyRPath function check if --clean-strtab was passed and disable the tainting behavior
3. Longer term: Run 2 by default.

[brenoguim]

I'll try a different approach. I'll extract a method to iterate on all string references from the --clean-strtab branch and use it to check if modifyRPATH can add the Xs.
It will be safer than enabling --clean-strtab by default.

[brenoguim]

@tomaszrostanskithales Can you test the build from https://github.com/brenoguim/patchelf/tree/breno.315 ?