Skip to content
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.

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

Cabal-HEAD/ghc-HEAD injects insecure RUNPATHs to a temp build directory in dynamic binaries using library of the same package #4025

Closed
trofi opened this issue Oct 22, 2016 · 9 comments · Fixed by #4033
Assignees
Milestone

Comments

@trofi
Copy link
Contributor

trofi commented Oct 22, 2016

Let's look at small-ish systemwide-installed idiii package. I'm installing it the same way on 2 systems as:

$ mkdir /tmp/foo
$ cd /tmp/foo
$ cabal sandbox init
$ cabal install idiii --reinstall --enable-executable-dynamic

Good system (ghc-8.0.1/Cabal-1.24):

readelf -a ./.cabal-sandbox/bin/read-idiii | grep idiii
 0x0000000000000001 (NEEDED)             Shared library: [libHSidiii-0.1.3.3-2mhwbeokSof8SZHBkN2jio-ghc8.0.1.so]
 0x000000000000001d (RUNPATH)            Library runpath: [/tmp/foo/.cabal-sandbox/lib/x86_64-linux-ghc-8.0.1/idiii-0.1.3.3-2mhwbeokSof8SZHBkN2jio:...]

Bad system (ghc-8.1.20161022/Cabal-HEAD):

sf foo # readelf -a ./.cabal-sandbox/bin/read-idiii | grep idiii
 0x0000000000000001 (NEEDED)             Shared library: [libHSidiii-0.1.3.3-AFJIiuU3Qs1GUtsFo9lbmP-ghc8.1.20161022.so]
 0x000000000000001d (RUNPATH)            Library runpath: [/tmp/foo/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161022:...:/tmp/cabal-tmp-31011/idiii-0.1.3.3/dist/dist-sandbox-6b4809b5/build:...]

Note additional RUNPATH to a temp build directory. Gentoo package manager flags that entry as insecure:

 * QA Notice: The following files contain insecure RUNPATHs
 *  Please file a bug about this at https://bugs.gentoo.org/
 *  with the maintainer of the package.
 *   /dev/shm/portage/dev-haskell/idiii-0.1.3.3/image/usr/bin/read-idiii
 *     RPATH: /usr/lib64/hunit-1.3.1.2/ghc-8.1.20161022:/usr/lib64/missingh-1.4.0.1/ghc-8.1.20161022:/usr/lib64/ghc-8.1.20161022/array-0.5.1.1:/usr/lib64/ghc-8.1.20161022/base-4.9.0.0:/usr/lib64/ghc-8.1.20161022/binary-0.8.3.0:/usr/lib64/ghc-8.1.20161022/bytestring-0.10.8.1:/usr/lib64/ghc-8.1.20161022/containers-0.5.7.1:/usr/lib64/data-accessor-0.2.2.7/ghc-8.1.20161022:/usr/lib64/ghc-8.1.20161022/deepseq-1.4.2.0:/usr/lib64/ghc-8.1.20161022/directory-1.2.6.2:/usr/lib64/ghc-8.1.20161022/filepath-1.4.1.0:/usr/lib64/ghc-8.1.20161022/ghc-prim-0.5.0.0:/usr/lib64/hslogger-1.2.10/ghc-8.1.20161022:/dev/shm/portage/dev-haskell/idiii-0.1.3.3/work/idiii-0.1.3.3/dist/build:/usr/lib64/ghc-8.1.20161022/integer-gmp-1.0.0.1:/usr/lib64/mtl-2.2.1/ghc-8.1.20161022:/usr/lib64/network-2.6.3.1/ghc-8.1.20161022:/usr/lib64/old-locale-1.0.0.7/ghc-8.1.20161022:/usr/lib64/old-time-1.1.0.3/ghc-8.1.20161022:/usr/lib64/parsec-3.1.11/ghc-8.1.20161022:/usr/lib64/polyparse-1.12/ghc-8.1.20161022:/usr/lib64/ghc-8.1.20161022/process-1.4.2.0:/usr/lib64/random-1.1/ghc-8.1.20161022:/usr/lib64/regex-base-0.93.2/ghc-8.1.20161022:/usr/lib64/regex-compat-0.95.1/ghc-8.1.20161022:/usr/lib64/regex-posix-0.95.2/ghc-8.1.20161022:/usr/lib64/ghc-8.1.20161022/rts:/usr/lib64/text-1.2.2.1/ghc-8.1.20161022:/usr/lib64/ghc-8.1.20161022/time-1.6.0.1:/usr/lib64/ghc-8.1.20161022/transformers-0.5.2.0:/usr/lib64/ghc-8.1.20161022/unix-2.7.2.0:/usr/lib64/utf8-string-1.0.1.1/ghc-8.1.20161022:/usr/lib64/x86_64-linux-ghc-8.1.20161022

I use /dev/shm/ as a temp directory in that system.

Adding @ezyang and @dcoutts for visibility.

@ezyang
Copy link
Contributor

ezyang commented Oct 22, 2016

That definitely looks bad. If the regression is recent, it's likely to be related to #3979. Also adding @christiaanb

@23Skidoo 23Skidoo added this to the 1.24.0.1 milestone Oct 22, 2016
@christiaanb
Copy link
Collaborator

@trofi What does the combination of GHC-HEAD/Cabal-1.24-git-branch give you?

@23Skidoo
Copy link
Member

Attn. @bgamari - we probably don't want to ship GHC 8.0.2 with this bug.

@christiaanb
Copy link
Collaborator

@23Skidoo @bgamari I don't think this bug exists in GHC 8.0.2. That is, given:

$ ghc --info
 [("Project name","The Glorious Glasgow Haskell Compilation System")
 ,("GCC extra via C opts"," -fwrapv -fno-builtin")
 ,("C compiler command","/usr/bin/gcc")
 ,("C compiler flags"," -fno-stack-protector")
 ,("C compiler link flags","")
 ,("Haskell CPP command","/usr/bin/gcc")
 ,("Haskell CPP flags","-E -undef -traditional")
 ,("ld command","/usr/bin/ld")
 ,("ld flags","")
 ,("ld supports compact unwind","YES")
 ,("ld supports build-id","YES")
 ,("ld supports filelist","NO")
 ,("ld is GNU ld","YES")
 ,("ar command","/usr/bin/ar")
 ,("ar flags","q")
 ,("ar supports at file","YES")
 ,("touch command","touch")
 ,("dllwrap command","/bin/false")
 ,("windres command","/bin/false")
 ,("libtool command","libtool")
 ,("perl command","/usr/bin/perl")
 ,("cross compiling","NO")
 ,("target os","OSLinux")
 ,("target arch","ArchX86_64")
 ,("target word size","8")
 ,("target has GNU nonexec stack","True")
 ,("target has .ident directive","True")
 ,("target has subsections via symbols","False")
 ,("Unregisterised","NO")
 ,("LLVM llc command","llc")
 ,("LLVM opt command","opt")
 ,("Project version","8.0.1.20161022")
 ,("Project Git commit id","9448e62740ca03aeb915bf0ecf8b16e54a52798a")
 ,("Booter version","8.0.1")
 ,("Stage","2")
 ,("Build platform","x86_64-unknown-linux")
 ,("Host platform","x86_64-unknown-linux")
 ,("Target platform","x86_64-unknown-linux")
 ,("Have interpreter","YES")
 ,("Object splitting supported","YES")
 ,("Have native code generator","YES")
 ,("Support SMP","YES")
 ,("Tables next to code","YES")
 ,("RTS ways","l debug thr thr_debug thr_l  dyn debug_dyn thr_dyn thr_debug_dyn l_dyn thr_l_dyn")
 ,("RTS expects libdw","NO")
 ,("Support dynamic-too","YES")
 ,("Support parallel --make","YES")
 ,("Support reexported-modules","YES")
 ,("Support thinning and renaming package flags","YES")
 ,("Requires unified installed package IDs","YES")
 ,("Uses package keys","YES")
 ,("Uses unit IDs","YES")
 ,("Dynamic by default","NO")
 ,("GHC Dynamic","YES")
 ,("GHC Profiled","NO")
 ,("Leading underscore","NO")
 ,("Debug on","False")
 ,("LibDir","/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022")
 ,("Global Package DB","/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/package.conf.d")
 ]

and,

$ cabal --version
cabal-install version 1.24.0.1
compiled using version 1.24.1.0 of the Cabal library 

I get:

$ mkdir /tmp/foo
$ cd /tmp/foo
$ cabal sandbox init
$ cabal install idiii --reinstall --enable-executable-dynamic
$ readelf -a ./.cabal-sandbox/bin/read-idiii | grep RPATH
 0x000000000000000f (RPATH)              Library rpath: [/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/array-0.5.1.1:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/base-4.9.0.0:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/binary-0.8.3.0:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/bytestring-0.10.8.1:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/containers-0.5.7.1:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/deepseq-1.4.2.0:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/directory-1.2.6.2:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/filepath-1.4.1.0:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/ghc-prim-0.5.0.0:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/integer-gmp-1.0.0.1:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/process-1.4.2.0:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/rts:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/time-1.6.0.1:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/transformers-0.5.2.0:/opt/ghc/8.0.2/lib/ghc-8.0.1.20161022/unix-2.7.2.0:/tmp/foo/.cabal-sandbox/lib/x86_64-linux-ghc-8.0.1.20161022]

That is, on the ghc-8.0 branch of GHC (i.e. ghc-8.0.2, which includes ghc/ghc@9448e62), and using a Cabal-1.24.1.0-based cabal-install (which includes #4011), the RPATH does not mention the temporary build directory.

@23Skidoo 23Skidoo modified the milestones: 2.0, 1.24.0.1 Oct 24, 2016
@23Skidoo
Copy link
Member

@christiaanb Have you been able to reproduce the issue with Cabal HEAD?

@christiaanb
Copy link
Collaborator

@23Skidoo Yes I can reproduce using ghc-HEAD and Cabal-HEAD, using:

$ ghc --info
 [("Project name","The Glorious Glasgow Haskell Compilation System")
 ,("GCC extra via C opts"," -fwrapv -fno-builtin")
 ,("C compiler command","gcc")
 ,("C compiler flags"," -fno-stack-protector")
 ,("C compiler link flags","")
 ,("Haskell CPP command","gcc")
 ,("Haskell CPP flags","-E -undef -traditional")
 ,("ld command","/usr/bin/ld")
 ,("ld flags","")
 ,("ld supports compact unwind","YES")
 ,("ld supports build-id","YES")
 ,("ld supports filelist","NO")
 ,("ld is GNU ld","YES")
 ,("ar command","/usr/bin/ar")
 ,("ar flags","q")
 ,("ar supports at file","YES")
 ,("touch command","touch")
 ,("dllwrap command","/bin/false")
 ,("windres command","/bin/false")
 ,("libtool command","libtool")
 ,("perl command","/usr/bin/perl")
 ,("cross compiling","NO")
 ,("target os","OSLinux")
 ,("target arch","ArchX86_64")
 ,("target word size","8")
 ,("target has GNU nonexec stack","True")
 ,("target has .ident directive","True")
 ,("target has subsections via symbols","False")
 ,("Unregisterised","NO")
 ,("LLVM llc command","llc")
 ,("LLVM opt command","opt")
 ,("Project version","8.1.20161023")
 ,("Project Git commit id","f084e6845515fbfb774a09ae5d2af1eea8fdc3f0")
 ,("Booter version","7.10.3")
 ,("Stage","2")
 ,("Build platform","x86_64-unknown-linux")
 ,("Host platform","x86_64-unknown-linux")
 ,("Target platform","x86_64-unknown-linux")
 ,("Have interpreter","YES")
 ,("Object splitting supported","YES")
 ,("Have native code generator","YES")
 ,("Support SMP","YES")
 ,("Tables next to code","YES")
 ,("RTS ways","l debug thr thr_debug thr_l thr_p dyn debug_dyn thr_dyn thr_debug_dyn l_dyn thr_l_dyn")
 ,("RTS expects libdw","NO")
 ,("Support dynamic-too","YES")
 ,("Support parallel --make","YES")
 ,("Support reexported-modules","YES")
 ,("Support thinning and renaming package flags","YES")
 ,("Support Backpack","YES")
 ,("Requires unified installed package IDs","YES")
 ,("Uses package keys","YES")
 ,("Uses unit IDs","YES")
 ,("Dynamic by default","NO")
 ,("GHC Dynamic","YES")
 ,("GHC Profiled","NO")
 ,("Leading underscore","NO")
 ,("Debug on","False")
 ,("LibDir","/opt/ghc/head/lib/ghc-8.1.20161023")
 ,("Global Package DB","/opt/ghc/head/lib/ghc-8.1.20161023/package.conf.d")
 ]

and

$ cabal --version
cabal-install version 1.25.0.0
compiled using version 1.25.0.0 of the Cabal library

Which is a cabal-install based on the Cabal and cabal-install sources of 266c5aa

I get:

$ mkdir /tmp/bar
$ cd /tmp/bar
$ cabal sandbox init
$ cabal install idiii --reinstall --enable-executable-dynamic
$ readelf -a ./.cabal-sandbox/bin/read-idiii | grep dist
 0x000000000000000f (RPATH)              Library rpath: [/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/HUnit-1.5.0.0-LmQy44w6E24IQ5C7bKO5Pw:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/MissingH-1.4.0.1-KVpfHgs4VtEHraATNStfAj:/opt/ghc/head/lib/ghc-8.1.20161023/array-0.5.1.1:/opt/ghc/head/lib/ghc-8.1.20161023/base-4.9.0.0:/opt/ghc/head/lib/ghc-8.1.20161023/binary-0.8.3.0:/opt/ghc/head/lib/ghc-8.1.20161023/bytestring-0.10.8.1:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/call-stack-0.1.0-izRGIczcaH64KMRs1Wg8q:/opt/ghc/head/lib/ghc-8.1.20161023/containers-0.5.7.1:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/data-accessor-0.2.2.7-8YIdFyGw7Iv38wc1N9wMCB:/opt/ghc/head/lib/ghc-8.1.20161023/deepseq-1.4.2.0:/opt/ghc/head/lib/ghc-8.1.20161023/directory-1.2.6.2:/opt/ghc/head/lib/ghc-8.1.20161023/filepath-1.4.1.0:/opt/ghc/head/lib/ghc-8.1.20161023/ghc-prim-0.5.0.0:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/hslogger-1.2.10-2MqNtqteqzxLp4s2hmbEFL:/tmp/cabal-tmp-31386/idiii-0.1.3.3/dist/dist-sandbox-45689551/build:/opt/ghc/head/lib/ghc-8.1.20161023/integer-gmp-1.0.0.1:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/mtl-2.2.1-6qsR1PHUy5lL47Hpoa4jCM:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/network-2.6.3.1-G4Up1CPKbp7DeFsnywOnGG:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/old-locale-1.0.0.7-6glXNhHF891B41ZfuI8hU8:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/old-time-1.1.0.3-IcvdkJUsE9M8t3io8peAEp:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/parsec-3.1.11-37j7M1YEHqtEooY7BpJdri:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/polyparse-1.12-FkXEOA8o2Io9i7TJcmDT44:/opt/ghc/head/lib/ghc-8.1.20161023/process-1.4.2.0:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/random-1.1-54KmMHXjttlERYcr1mvsAe:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/regex-base-0.93.2-71isvdwnRNrGKtKYo9rpQd:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/regex-compat-0.95.1-4nLo9klb6Pk7Tun1BAC3he:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/regex-posix-0.95.2-BbcuFE4RwTG3oQUmzAVjm:/opt/ghc/head/lib/ghc-8.1.20161023/rts:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/text-1.2.2.1-9Yh8rJoh8fO2JMLWffT3Qs:/opt/ghc/head/lib/ghc-8.1.20161023/time-1.6.0.1:/opt/ghc/head/lib/ghc-8.1.20161023/transformers-0.5.2.0:/opt/ghc/head/lib/ghc-8.1.20161023/unix-2.7.2.0:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023/utf8-string-1.0.1.1-2T8mBCuEDlXDo8zed8Onw4:/tmp/bar/.cabal-sandbox/lib/x86_64-linux-ghc-8.1.20161023]

@christiaanb
Copy link
Collaborator

@ezyang
Copy link
Contributor

ezyang commented Oct 24, 2016

First off, a clarifying note: it is NOT an error for /tmp to be mentioned in the RPATH. Those path's are fine; they show up because @trofi's sandbox was placed in tmp, and it is expected behavior for rpaths to consequently point to the sandbox directory. The error is specifically the directory /tmp/cabal-tmp-31011/idiii-0.1.3.3/dist/dist-sandbox-6b4809b5/build, which is a completely temporary build directory for idiii.

Next, I think @christiaanb is right and it has nothing to do with dynlibdir changes; there's a decent shot that 0d15ede tickled an existing bug in depLibraryPaths.

Here's my theory:

  • In 1.24, installedPkgs contained only references to the external package database, not any internal libraries. In particular, when we built read-idiii executable, installedPkgs did NOT have a reference to the internal library; instead, depLibraryPaths has a special case (hasInternalDeps) to add the final libdir to the RPATH.
  • In HEAD after 0d15ede we now add the internal libraries (with their INPLACE data) to installedPkgs, which means it gets picked up when we look at ipkgs. Additionally, I forgot to remove the special case for internal libraries.

If this is correct, then 8.0.2 shouldn't have this bug. Would be a good thing to check when we cut the RC.

I guess the fix is, we should vary the "inplace" registrations based on whether or not we are building for inplace or not.

@ezyang
Copy link
Contributor

ezyang commented Oct 25, 2016

I am quite a bit annoyed by this bug, because it is caused by the inplace package database. With a per-component build, the problem goes away, because you're always building w.r.t. the library-dirs from the externally provided package database, which has the correct path. No fix needed.

But given that we can't get rid of the inplace db (yet), we must find some suitable solution. If we really do believe the inplace database will go away soon, I suppose we could just bash our way to a solution: for example, we could add some completely alternate codepath for computing rpaths of internal libraries (there'd need to be a way of filtering out the bad inplace internal entries from installedPkgs), which would just not get exercised if you did a per-component build.

We shouldn't just revert 0d15ede because that would regress #2971 (well, I guess didn't actually affect anyone, so maybe we could...) installedPkgs is used by very few pieces of code: depLibraryPaths, Haddock haddockPackageFlags (there is probably a bug here), checkForeignDeps and checkRelocatable (never gets to see inplace things), and ppHsc2hs) so we could also just eliminate it all together in favor of some more special types.

This bug raises a concern: if GHC ever builds any of the file paths from the inplace package db into the object files it generates, then GHC builds are nondeterministic. (It doesn't seem to do so at the moment, but it is a bit worrisome); a bug of this nature would be analogous to this one.

@23Skidoo 23Skidoo changed the title Cabal-HEAD/ghc-EHAD injects insecure RUNPATHs to a temp build directory in dynamic binaries using library of the same package Cabal-HEAD/ghc-HEAD injects insecure RUNPATHs to a temp build directory in dynamic binaries using library of the same package Oct 25, 2016
ezyang added a commit to ezyang/cabal that referenced this issue Oct 25, 2016
In 1.24, installedPkgs contained only references to the external package
database, not any internal libraries. In particular, when we built a
dynamically linked executable, installedPkgs did NOT have a reference to
the internal library; instead, depLibraryPaths has a special case
(hasInternalDeps) to add the final libdir to the RPATH.

In HEAD, after 0d15ede, we started adding internal libraries (with the
INPLACE registrations) to installedPkgs to fix haskell#2971.  But depLibraryPaths
was not updated, which means that the inplace registrations got picked
up and added to the RPATH, resulting in bad temporary directories
showing up in RPATHs.

In this commit, we just filter out internal entries from installedPkgs
and compute the library dirs for them from scratch (this code already
existed, so no loss!)

Fixes haskell#4025.

Signed-off-by: Edward Z. Yang <[email protected]>
ezyang added a commit to ezyang/cabal that referenced this issue Oct 25, 2016
In 1.24, installedPkgs contained only references to the external package
database, not any internal libraries. In particular, when we built a
dynamically linked executable, installedPkgs did NOT have a reference to
the internal library; instead, depLibraryPaths has a special case
(hasInternalDeps) to add the final libdir to the RPATH.

In HEAD, after 0d15ede, we started adding internal libraries (with the
INPLACE registrations) to installedPkgs to fix haskell#2971.  But depLibraryPaths
was not updated, which means that the inplace registrations got picked
up and added to the RPATH, resulting in bad temporary directories
showing up in RPATHs.

In this commit, we just filter out internal entries from installedPkgs
and compute the library dirs for them from scratch (this code already
existed, so no loss!)

Fixes haskell#4025.

Signed-off-by: Edward Z. Yang <[email protected]>
@ezyang ezyang self-assigned this Oct 25, 2016
erikd pushed a commit to erikd/cabal that referenced this issue Jan 28, 2017
In 1.24, installedPkgs contained only references to the external package
database, not any internal libraries. In particular, when we built a
dynamically linked executable, installedPkgs did NOT have a reference to
the internal library; instead, depLibraryPaths has a special case
(hasInternalDeps) to add the final libdir to the RPATH.

In HEAD, after 0d15ede, we started adding internal libraries (with the
INPLACE registrations) to installedPkgs to fix haskell#2971.  But depLibraryPaths
was not updated, which means that the inplace registrations got picked
up and added to the RPATH, resulting in bad temporary directories
showing up in RPATHs.

In this commit, we just filter out internal entries from installedPkgs
and compute the library dirs for them from scratch (this code already
existed, so no loss!)

Fixes haskell#4025.

Signed-off-by: Edward Z. Yang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants