Skip to content

Commit 903c0f0

Browse files
committed
Remove unused rpath settings
Prior to this change if a module didn't link to anything under our root we left the rpath from the build un-modified. This lead to rpaths pointint to directories that may not exist. This update will clear any un-used rpath settings, only modules that require an rpath will have one set.
1 parent e8bbdd8 commit 903c0f0

3 files changed

Lines changed: 123 additions & 2 deletions

File tree

relenv/build/common/install.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,11 @@ def finalize(
427427
:type logfp: file
428428
"""
429429
# Run relok8 to make sure the rpaths are relocatable.
430-
relenv.relocate.main(dirs.prefix, log_file_name=str(dirs.logs / "relocate.py.log"))
430+
# Modules that don't link to relenv libs will have their RPATH removed
431+
relenv.relocate.main(
432+
dirs.prefix,
433+
log_file_name=str(dirs.logs / "relocate.py.log"),
434+
)
431435
# Install relenv-sysconfigdata module
432436
libdir = pathlib.Path(dirs.prefix) / "lib"
433437

relenv/relocate.py

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,34 @@ def is_in_dir(
264264
return os.path.realpath(filepath).startswith(os.path.realpath(directory) + os.sep)
265265

266266

267+
def remove_rpath(path: str | os.PathLike[str]) -> bool:
268+
"""
269+
Remove the rpath from a given ELF file.
270+
271+
:param path: The path to an ELF file
272+
:type path: str
273+
274+
:return: True if successful, False otherwise
275+
:rtype: bool
276+
"""
277+
old_rpath = parse_rpath(path)
278+
if not old_rpath:
279+
# No RPATH to remove
280+
return True
281+
282+
log.info("Remove RPATH from %s (was: %s)", path, old_rpath)
283+
proc = subprocess.run(
284+
["patchelf", "--remove-rpath", path],
285+
stderr=subprocess.PIPE,
286+
stdout=subprocess.PIPE,
287+
)
288+
289+
if proc.returncode:
290+
log.error("Failed to remove RPATH from %s: %s", path, proc.stderr.decode())
291+
return False
292+
return True
293+
294+
267295
def patch_rpath(
268296
path: str | os.PathLike[str],
269297
new_rpath: str,
@@ -325,6 +353,7 @@ def handle_elf(
325353
root = libs
326354
proc = subprocess.run(["ldd", path], stderr=subprocess.PIPE, stdout=subprocess.PIPE)
327355
needs_rpath = False
356+
328357
for line in proc.stdout.decode().splitlines():
329358
if line.find("=>") == -1:
330359
log.debug("Skip ldd output line: %s", line)
@@ -371,7 +400,9 @@ def handle_elf(
371400
log.info("Adjust rpath of %s to %s", path, relpath)
372401
patch_rpath(path, relpath)
373402
else:
374-
log.info("Do not adjust rpath of %s", path)
403+
# No relenv libraries are linked, so RPATH is not needed
404+
# Remove any existing RPATH to avoid security/correctness issues
405+
remove_rpath(path)
375406

376407

377408
def main(
@@ -420,6 +451,7 @@ def main(
420451
if path in processed:
421452
continue
422453
log.debug("Checking %s", path)
454+
423455
if is_macho(path):
424456
log.info("Found Mach-O %s", path)
425457
_ = handle_macho(path, libs_dir, rpath_only)

tests/test_relocate.py

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
main,
1717
parse_readelf_d,
1818
patch_rpath,
19+
remove_rpath,
1920
)
2021

2122
pytestmark = [
@@ -276,3 +277,87 @@ def test_handle_elf_rpath_only(tmp_path: pathlib.Path) -> None:
276277
assert not (proj.libs_dir / "fake.so.2").exists()
277278
assert patch_rpath_mock.call_count == 1
278279
patch_rpath_mock.assert_called_with(str(pybin), "$ORIGIN/../lib")
280+
281+
282+
def test_remove_rpath_with_existing_rpath(tmp_path: pathlib.Path) -> None:
283+
"""Test that remove_rpath removes an existing RPATH."""
284+
path = str(tmp_path / "test.so")
285+
with patch("subprocess.run", return_value=MagicMock(returncode=0)):
286+
with patch(
287+
"relenv.relocate.parse_rpath",
288+
return_value=["/some/absolute/path"],
289+
):
290+
assert remove_rpath(path) is True
291+
292+
293+
def test_remove_rpath_no_existing_rpath(tmp_path: pathlib.Path) -> None:
294+
"""Test that remove_rpath succeeds when there's no RPATH to remove."""
295+
path = str(tmp_path / "test.so")
296+
with patch("relenv.relocate.parse_rpath", return_value=[]):
297+
assert remove_rpath(path) is True
298+
299+
300+
def test_remove_rpath_failed(tmp_path: pathlib.Path) -> None:
301+
"""Test that remove_rpath returns False when patchelf fails."""
302+
path = str(tmp_path / "test.so")
303+
with patch("subprocess.run", return_value=MagicMock(returncode=1)):
304+
with patch(
305+
"relenv.relocate.parse_rpath",
306+
return_value=["/some/absolute/path"],
307+
):
308+
assert remove_rpath(path) is False
309+
310+
311+
def test_handle_elf_removes_rpath_when_no_relenv_libs(tmp_path: pathlib.Path) -> None:
312+
"""Test that handle_elf removes RPATH for binaries linking only to system libs."""
313+
proj = LinuxProject(tmp_path / "proj")
314+
module = proj.add_simple_elf("array.so", "lib", "python3.10", "lib-dynload")
315+
316+
# ldd output showing only system libraries
317+
ldd_ret = """
318+
linux-vdso.so.1 => linux-vdso.so.1 (0x0123456789)
319+
libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x0123456789)
320+
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x0123456789)
321+
""".encode()
322+
323+
with proj:
324+
with patch("subprocess.run", return_value=MagicMock(stdout=ldd_ret)):
325+
with patch("relenv.relocate.remove_rpath") as remove_rpath_mock:
326+
with patch("relenv.relocate.patch_rpath") as patch_rpath_mock:
327+
handle_elf(
328+
str(module), str(proj.libs_dir), True, str(proj.root_dir)
329+
)
330+
# Should remove RPATH, not patch it
331+
assert remove_rpath_mock.call_count == 1
332+
assert patch_rpath_mock.call_count == 0
333+
remove_rpath_mock.assert_called_with(str(module))
334+
335+
336+
def test_handle_elf_sets_rpath_when_relenv_libs_present(tmp_path: pathlib.Path) -> None:
337+
"""Test that handle_elf sets RPATH for binaries linking to relenv libs."""
338+
proj = LinuxProject(tmp_path / "proj")
339+
module = proj.add_simple_elf("_ssl.so", "lib", "python3.10", "lib-dynload")
340+
libssl = proj.libs_dir / "libssl.so.3"
341+
libssl.touch()
342+
343+
# ldd output showing relenv-built library
344+
ldd_ret = """
345+
linux-vdso.so.1 => linux-vdso.so.1 (0x0123456789)
346+
libssl.so.3 => {libssl} (0x0123456789)
347+
libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x0123456789)
348+
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x0123456789)
349+
""".format(
350+
libssl=libssl
351+
).encode()
352+
353+
with proj:
354+
with patch("subprocess.run", return_value=MagicMock(stdout=ldd_ret)):
355+
with patch("relenv.relocate.remove_rpath") as remove_rpath_mock:
356+
with patch("relenv.relocate.patch_rpath") as patch_rpath_mock:
357+
handle_elf(
358+
str(module), str(proj.libs_dir), True, str(proj.root_dir)
359+
)
360+
# Should patch RPATH, not remove it
361+
assert patch_rpath_mock.call_count == 1
362+
assert remove_rpath_mock.call_count == 0
363+
patch_rpath_mock.assert_called_with(str(module), "$ORIGIN/../..")

0 commit comments

Comments
 (0)