Skip to content

Allow multiple modifications in same call #361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 22, 2021
Merged

Allow multiple modifications in same call #361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d306abe
Allow multiple modifications in same call
fzakaria Dec 21, 2021
e268662
Add a unit test
fzakaria Dec 21, 2021
dc375bc
Remove result and add it to gitignore
fzakaria Dec 21, 2021
d02a3ec
Code clarity and test working with musl
fzakaria Dec 21, 2021
e589fe3
Merge branch 'master' into faridzakaria/fix-add-replace-for-real
fzakaria Dec 21, 2021
b92a6e8
Fix replace-add-needed.sh test for musl
fzakaria Dec 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ Makefile
.direnv/
.vscode/
.idea/
result
13 changes: 12 additions & 1 deletion src/patchelf.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,7 @@ void ElfFile<ElfFileParamNames>::modifySoname(sonameMode op, const std::string &
}

changed = true;
this->rewriteSections();
}

template<ElfFileParams>
Expand All @@ -1158,6 +1159,7 @@ void ElfFile<ElfFileParamNames>::setInterpreter(const std::string & newInterpret
std::string & section = replaceSection(".interp", newInterpreter.size() + 1);
setSubstr(section, 0, newInterpreter + '\0');
changed = true;
this->rewriteSections();
}


Expand Down Expand Up @@ -1234,6 +1236,7 @@ void ElfFile<ElfFileParamNames>::removeRPath(Elf_Shdr & shdrDynamic) {
}
}
memset(last, 0, sizeof(Elf_Dyn) * (dyn - last));
this->rewriteSections();
}

template<ElfFileParams>
Expand Down Expand Up @@ -1380,6 +1383,7 @@ void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op,
newDyn.d_un.d_val = shdrDynStr.sh_size;
setSubstr(newDynamic, 0, std::string((char *) &newDyn, sizeof(Elf_Dyn)));
}
this->rewriteSections();
}


Expand Down Expand Up @@ -1409,6 +1413,8 @@ void ElfFile<ElfFileParamNames>::removeNeeded(const std::set<std::string> & libs
}

memset(last, 0, sizeof(Elf_Dyn) * (dyn - last));

this->rewriteSections();
}

template<ElfFileParams>
Expand Down Expand Up @@ -1532,6 +1538,8 @@ void ElfFile<ElfFileParamNames>::replaceNeeded(const std::map<std::string, std::
--verNeedNum;
}
}

this->rewriteSections();
}

template<ElfFileParams>
Expand Down Expand Up @@ -1582,6 +1590,8 @@ void ElfFile<ElfFileParamNames>::addNeeded(const std::set<std::string> & libs)
}

changed = true;

this->rewriteSections();
}

template<ElfFileParams>
Expand Down Expand Up @@ -1638,6 +1648,7 @@ void ElfFile<ElfFileParamNames>::noDefaultLib()
setSubstr(newDynamic, 0, std::string((char *) &newDyn, sizeof(Elf_Dyn)));
}

this->rewriteSections();
changed = true;
}

Expand Down Expand Up @@ -1667,6 +1678,7 @@ void ElfFile<ElfFileParamNames>::clearSymbolVersions(const std::set<std::string>
}
}
changed = true;
this->rewriteSections();
}

static bool printInterpreter = false;
Expand Down Expand Up @@ -1726,7 +1738,6 @@ static void patchElf2(ElfFile && elfFile, const FileContents & fileContents, con
elfFile.noDefaultLib();

if (elfFile.isChanged()){
elfFile.rewriteSections();
writeFile(fileName, elfFile.fileContents);
} else if (alwaysWrite) {
debug("not modified, but alwaysWrite=true\n");
Expand Down
3 changes: 2 additions & 1 deletion tests/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ src_TESTS = \
basic-flags.sh \
set-empty-rpath.sh \
phdr-corruption.sh \
replace-needed.sh
replace-needed.sh \
replace-add-needed.sh

build_TESTS = \
$(no_rpath_arch_TESTS)
Expand Down
38 changes: 38 additions & 0 deletions tests/replace-add-needed.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#! /bin/sh -e
SCRATCH=scratch/$(basename $0 .sh)
PATCHELF=$(readlink -f "../src/patchelf")

rm -rf ${SCRATCH}
mkdir -p ${SCRATCH}

cp simple ${SCRATCH}/
cp libfoo.so ${SCRATCH}/
cp libbar.so ${SCRATCH}/

cd ${SCRATCH}

libcldd=$(ldd ./simple | awk '/ => / { print $3 }' | grep -E "(libc.so|ld-musl)")

# We have to set the soname on these libraries
${PATCHELF} --set-soname libbar.so ./libbar.so
Comment on lines +16 to +17
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly discussed with @trws that if the shared object doesn't have a DT_SONAME, it won't make use of the previously discovered loaded shared object.

This was discovered while writing this test. You can see below that libbar.so was not found, even though its set with an absolute path.

ldd ~/code/github.com/NixOS/patchelf/tests/scratch/replace-add-needed/simple
	linux-vdso.so.1 (0x00007ffc90565000)
	/home/fmzakari/code/github.com/NixOS/patchelf/tests/scratch/replace-add-needed/libbar.so (0x00007f532a4c4000)
	/home/fmzakari/code/github.com/NixOS/patchelf/tests/scratch/replace-add-needed/libfoo.so (0x00007f532a4bf000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f532a2da000)
	/nix/store/jsp3h3wpzc842j0rz61m5ly71ak6qgdn-glibc-2.32-54/lib/ld-linux-x86-64.so.2 => /lib64/ld-linux-x86-64.so.2 (0x00007f532a4cb000)
	libbar.so => not found


# Add a libbar.so so we can rewrite it later
${PATCHELF} --add-needed libbar.so ./simple

# Make the NEEDED in libfoo the same as simple
# This is a current "bug" in musl
# https://www.openwall.com/lists/musl/2021/12/21/1
${PATCHELF} --replace-needed libbar.so $(readlink -f ./libbar.so) ./libfoo.so
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mic92 please read the linked mailing issue I opened for why we have to do this for musl
https://www.openwall.com/lists/musl/2021/12/21/1

This should now succeed on musl and glibc proving the fix of the bug.


${PATCHELF} --replace-needed libc.so.6 ${libcldd} \
--replace-needed libbar.so $(readlink -f ./libbar.so) \
--add-needed $(readlink -f ./libfoo.so) \
./simple

exitCode=0
./simple || exitCode=$?

if test "$exitCode" != 0; then
ldd ./simple
exit 1
fi