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

use PIE (full ASLR) by default on Android too #17448

Closed
wants to merge 1 commit into from
Closed

use PIE (full ASLR) by default on Android too #17448

wants to merge 1 commit into from

Conversation

thestinger
Copy link
Contributor

Android no longer permits non-PIE executables.

Closes #17437

@alexcrichton
Copy link
Member

We can't just instantly drop backtrace support on android, this needs to at least be investigate somewhat to see why backtraces are broken on android.

@thestinger
Copy link
Contributor Author

@alexcrichton: Rust currently has no support for current Android versions at all because they only run executables built with PIE. Lacking support for the platform is worse than lacking symbol names in backtraces.

@thestinger thestinger closed this Sep 24, 2014
@thestinger thestinger reopened this Sep 24, 2014
@thestinger
Copy link
Contributor Author

The Android bot needs to be updated in order for this to have a chance of working. It isn't going to be possible to upgrade it without using PIE, because Android no longer allows non-PIE. Anyway, there's nothing I can do about that so I'll leave this here to rot.

@thestinger thestinger added the O-android Operating system: Android label Sep 24, 2014
@alexcrichton
Copy link
Member

Let's please slow down a bit here.

  • Have you verified by hand that backtrace-related tests do indeed work on an updated version of an android toolchain? (this is a concrete step forward)
  • Do you have evidence to support your claim that it is impossible for backtraces to work with PIE?
  • Precisely what versions of the toolchain and such do you think need to be updated?
  • Do you know if this will break backwards compatibility?

@thestinger
Copy link
Contributor Author

Do you have evidence to support your claim that it is impossible for backtraces to work with PIE?

When did I say this? They work fine with PIE on Linux.

Precisely what versions of the toolchain and such do you think need to be updated?

The NDK and Android version are out-of-date. Android has full support for PIE now and it became mandatory when there were no remaining problems. I know it's out-of-date because it's still working.

Do you know if this will break backwards compatibility?

It doesn't break compatibility.

@alexcrichton
Copy link
Member

So the problem at hand is that enabling PIE breaks backtraces on android, do you know why that happens? It sounds like you're not willing to investigate, why I asked the questions.

@thestinger
Copy link
Contributor Author

It works fine with -C link-args=-pie on my Android, but that uses CyanogenMod. It breaks PIE on the Android bot but I have no idea if it would still break if it was up-to-date.

@thestinger thestinger added the A-security Area: Security (example: address space layout randomization). label Sep 24, 2014
@DanAlbert
Copy link
Contributor

Building without PIE will still work fine for older versions of Android. The patch removing support for non-PIE isn't in any released version of Android yet.

There have recently been a lot of unwinder fixes in AOSP as well. Is it possible that the unwinder tests would be fixed by those?

@thestinger thestinger closed this Sep 28, 2014
@thestinger thestinger reopened this Sep 28, 2014
@alexcrichton
Copy link
Member

@thestinger, have you manually verified that this test passes on an updated android compiler/client? I've tested manually against what I believe is r9d and both the test and the bundled gdb itself fail to get symbols or backtrace correctly.

Android no longer permits non-PIE executables.

Closes #17437
@emberian
Copy link
Member

emberian commented Nov 3, 2014

I think I accidentally did this in my branch, and I can confirm that the tests pass with r10c.

@alexcrichton
Copy link
Member

Closing due to inactivity (and @bors's queue is quite full!). I also tested this on the r10c toolchain, like @cmr said, on the bots and it failed with the same error. I know @cmr and I chatted about this on IRC, but I forgot the outcome, @cmr do you remember?

@thestinger thestinger deleted the pie branch January 28, 2015 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-security Area: Security (example: address space layout randomization). O-android Operating system: Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android now requires PIE executables
4 participants