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

Building git-core's ICS branch... #9

Open
erik360 opened this Issue Feb 17, 2012 · 38 comments

Comments

Projects
None yet
4 participants
@erik360

erik360 commented Feb 17, 2012

So I got this built last night to fix the ICS segmentation fault error and I had a few questions, if you don't mind.

  1. When compiled, this binary is about 150K, versus about 20K for the master branch. Any idea why? I built both and the make files look the same.

  2. It seems your gc branch is the most up to date, but does not include the segmentation fault / environment fix. Is there a plan to merge that or release a fixed su binary into the wild soon?

Thanks!

@git-core

This comment has been minimized.

Show comment
Hide comment
@git-core

git-core Feb 18, 2012

Collaborator
  1. When compiled, this binary is about 150K, versus about 20K for the master
    branch. Any idea why? I built both and the make files look the same.
    git-core:ics (or chainsdd:gc-ics) was dirty experimental fix. Its
    whole purpose was to check whether it could help somebody. The code
    allocates a huge array statically, thus increased size of the
    executable.

  2. It seems your gc branch is the most up to date, but does not include the
    segmentation fault / environment fix. Is there a plan to merge that or
    release a fixed su binary into the wild soon?
    When I asked ChainsDD last time, he though the patch isn't needed
    anymore. I don't have an ICS device, so my opinion doesn't count here.
    If you think the patch indeed is useful, please, explain why.

Collaborator

git-core commented Feb 18, 2012

  1. When compiled, this binary is about 150K, versus about 20K for the master
    branch. Any idea why? I built both and the make files look the same.
    git-core:ics (or chainsdd:gc-ics) was dirty experimental fix. Its
    whole purpose was to check whether it could help somebody. The code
    allocates a huge array statically, thus increased size of the
    executable.

  2. It seems your gc branch is the most up to date, but does not include the
    segmentation fault / environment fix. Is there a plan to merge that or
    release a fixed su binary into the wild soon?
    When I asked ChainsDD last time, he though the patch isn't needed
    anymore. I don't have an ICS device, so my opinion doesn't count here.
    If you think the patch indeed is useful, please, explain why.

@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Feb 18, 2012

Thank you for the reply.

I have a Verizon Galaxy Nexus running AOKP b23 (4.0.3). The segmentation fault is definitely still an issue on ICS and ICS ports, failing on commands such as 'pm'. I follow zeppelinrox's SuperCharger thread on XDA ( http://forum.xda-developers.com/showthread.php?t=991276 ) and there have been other reports of the same errors including by zep himself. In fact he found the other open issue #6 discussing the problem on this git, which brought me here.

Both the mentioned workaround of passing LD_LIBRARY_PATH and your patch have fixed the issue on my phone and his (I believe he runs an ICS port on a Milestone). So yes, I think it's worth looking at this issue again.

A few more followup question, if you would indulge me. I'm new to AOSP and compiling against it. I followed your build instructions in your git wiki, so I believe I have the current 4.0.3 branch.

  1. Is there any reason not to use your patch (in its current form) if it fixes the issue on ICS ROMs?

  2. Against which AOSP target should I be building su? The one we've been testing I built against 'lunch full-user'.

  3. Is there any reason the patched compile shouldn't work on pre-ICS ROMs such as CM7? I had one report of someone who said it 'didn't work' on CM7 (no more details yet), but I've yet to test it on my other phone, a Thunderbolt running a CM7 port (Thundershed 1.2). Does it matter which AOSP branch it's compiled against, or even which target?

Thank you again!

erik360 commented Feb 18, 2012

Thank you for the reply.

I have a Verizon Galaxy Nexus running AOKP b23 (4.0.3). The segmentation fault is definitely still an issue on ICS and ICS ports, failing on commands such as 'pm'. I follow zeppelinrox's SuperCharger thread on XDA ( http://forum.xda-developers.com/showthread.php?t=991276 ) and there have been other reports of the same errors including by zep himself. In fact he found the other open issue #6 discussing the problem on this git, which brought me here.

Both the mentioned workaround of passing LD_LIBRARY_PATH and your patch have fixed the issue on my phone and his (I believe he runs an ICS port on a Milestone). So yes, I think it's worth looking at this issue again.

A few more followup question, if you would indulge me. I'm new to AOSP and compiling against it. I followed your build instructions in your git wiki, so I believe I have the current 4.0.3 branch.

  1. Is there any reason not to use your patch (in its current form) if it fixes the issue on ICS ROMs?

  2. Against which AOSP target should I be building su? The one we've been testing I built against 'lunch full-user'.

  3. Is there any reason the patched compile shouldn't work on pre-ICS ROMs such as CM7? I had one report of someone who said it 'didn't work' on CM7 (no more details yet), but I've yet to test it on my other phone, a Thunderbolt running a CM7 port (Thundershed 1.2). Does it matter which AOSP branch it's compiled against, or even which target?

Thank you again!

@git-core

This comment has been minimized.

Show comment
Hide comment
@git-core

git-core Feb 18, 2012

Collaborator

I've merged gc into gc-ics, so you may use the version w/o known bugs and security issues now.
Note, the merge is completely untested. (Well, it compiles at least.)

  1. Is there any reason not to use your patch (in its current form) if it fixes the issue on ICS ROMs?
    As far as I understand the situation, ROM developers chose to modify the source of the bug/left it to App developers. Perhaps, I'm wrong.
    The patch in its current from is too heavy, I guess. It blindly copies all environment from the caller to the callee. The kernel allows up to 0x7FFFFFFF variables, 32 pages each (but not greater than 1/4 of stack size though) in the environment.
    If somebody would like to test, I could re-implement the patch by inheriting LD_LIBRARY_PATH only.
  2. Against which AOSP target should I be building su?
    Doesn't matter for native binaries, I guess. I use to build against full-eng, for example. Official su-binary is compiled for armv6, but irrelevant of TARGET_PRODUCT/TARGET_BUILD_VARIANT.
  3. Is there any reason the patched compile shouldn't work on pre-ICS ROMs
    I myself tested it on a 2.3 device.
    Does it matter which AOSP branch it's compiled against, or even which target?
    It doesn't until you're going to compile for Donut. Do you mean target in sense of the cpu arch like arm, x86, mips or in sense of the build variant of Android? If the former, su-binary is portable, I hope.
Collaborator

git-core commented Feb 18, 2012

I've merged gc into gc-ics, so you may use the version w/o known bugs and security issues now.
Note, the merge is completely untested. (Well, it compiles at least.)

  1. Is there any reason not to use your patch (in its current form) if it fixes the issue on ICS ROMs?
    As far as I understand the situation, ROM developers chose to modify the source of the bug/left it to App developers. Perhaps, I'm wrong.
    The patch in its current from is too heavy, I guess. It blindly copies all environment from the caller to the callee. The kernel allows up to 0x7FFFFFFF variables, 32 pages each (but not greater than 1/4 of stack size though) in the environment.
    If somebody would like to test, I could re-implement the patch by inheriting LD_LIBRARY_PATH only.
  2. Against which AOSP target should I be building su?
    Doesn't matter for native binaries, I guess. I use to build against full-eng, for example. Official su-binary is compiled for armv6, but irrelevant of TARGET_PRODUCT/TARGET_BUILD_VARIANT.
  3. Is there any reason the patched compile shouldn't work on pre-ICS ROMs
    I myself tested it on a 2.3 device.
    Does it matter which AOSP branch it's compiled against, or even which target?
    It doesn't until you're going to compile for Donut. Do you mean target in sense of the cpu arch like arm, x86, mips or in sense of the build variant of Android? If the former, su-binary is portable, I hope.
@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Feb 20, 2012

I'd like to test a version of your patch that inherits LD_LIBRARY_PATH only. It seems that is the only one that matters, so it would be nice if that's all it took to fix the problem.

Another question then... I've done a bit of searching and it seems that Android 4 dropped support for armv6. I'm guessing then it is not possible to compile su as armv6 with the current source tree? I see a post on androidsu.com from Oct. 23rd, 2011 that seems to hint at the same problem. It looks like ChainsDD found a way to compile for ARM generic. Was this with an old source tree, some other cross-compile method maybe?

erik360 commented Feb 20, 2012

I'd like to test a version of your patch that inherits LD_LIBRARY_PATH only. It seems that is the only one that matters, so it would be nice if that's all it took to fix the problem.

Another question then... I've done a bit of searching and it seems that Android 4 dropped support for armv6. I'm guessing then it is not possible to compile su as armv6 with the current source tree? I see a post on androidsu.com from Oct. 23rd, 2011 that seems to hint at the same problem. It looks like ChainsDD found a way to compile for ARM generic. Was this with an old source tree, some other cross-compile method maybe?

@git-core

This comment has been minimized.

Show comment
Hide comment
@git-core

git-core Feb 26, 2012

Collaborator

I've pushed the patch into both chainsdd:gc-ics and git-core:ics repos.

It looks like ChainsDD found a way to compile for ARM generic

I don't know details but what I see in AOSP tree it is controlled by TARGET_PRODUCT, full sets generic as PRODUCT_DEVICE and, hence, chooses cpu arch and abis for armv7, generic_armv5 sets generic_armv5 (what a coincidence!) as PRODUCT_DEVICE and chooses default abis and armv5 as cpu arch.

Collaborator

git-core commented Feb 26, 2012

I've pushed the patch into both chainsdd:gc-ics and git-core:ics repos.

It looks like ChainsDD found a way to compile for ARM generic

I don't know details but what I see in AOSP tree it is controlled by TARGET_PRODUCT, full sets generic as PRODUCT_DEVICE and, hence, chooses cpu arch and abis for armv7, generic_armv5 sets generic_armv5 (what a coincidence!) as PRODUCT_DEVICE and chooses default abis and armv5 as cpu arch.

@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Feb 26, 2012

I found the target setting and compiled for armv5, thanks for that!

However the patch doesn't seem to be working. After running su there is still no LD_LIBRARY_PATH environment string, and I get the seg faults.

Relevant logcat entries:

E/su (22700): sudb - Opening database
E/su (22700): sudb - Database opened
E/su (22700): sudb - Database closed
E/su (22700): Opening environment failed with 13: Permission denied

erik360 commented Feb 26, 2012

I found the target setting and compiled for armv5, thanks for that!

However the patch doesn't seem to be working. After running su there is still no LD_LIBRARY_PATH environment string, and I get the seg faults.

Relevant logcat entries:

E/su (22700): sudb - Opening database
E/su (22700): sudb - Database opened
E/su (22700): sudb - Database closed
E/su (22700): Opening environment failed with 13: Permission denied

@git-core

This comment has been minimized.

Show comment
Hide comment
@git-core

git-core Feb 27, 2012

Collaborator

Got it. I forgot to test w/ non-root credentials. Could you test the following:
move call to populate_environment(ctx) from line 378 to line 535 where it shall look like
populate_environment(&ctx)
Thus, access to /proc//environ will be performed w/ root credentials.
(I assume seg fault occurs in the execed process, not in su itself, doesn't it?)

Collaborator

git-core commented Feb 27, 2012

Got it. I forgot to test w/ non-root credentials. Could you test the following:
move call to populate_environment(ctx) from line 378 to line 535 where it shall look like
populate_environment(&ctx)
Thus, access to /proc//environ will be performed w/ root credentials.
(I assume seg fault occurs in the execed process, not in su itself, doesn't it?)

@perettigiuliano

This comment has been minimized.

Show comment
Hide comment
@perettigiuliano

perettigiuliano Feb 27, 2012

Sorry guys for intrusion i am following thread #6 because i am developing an app which send coomands to a su console but after upgrading my tablet to an ICS ROM this has stopped to work exactly with logcat repoted in thread #6.
What is the trick?

Sorry guys for intrusion i am following thread #6 because i am developing an app which send coomands to a su console but after upgrading my tablet to an ICS ROM this has stopped to work exactly with logcat repoted in thread #6.
What is the trick?

@git-core

This comment has been minimized.

Show comment
Hide comment
@git-core

git-core Feb 27, 2012

Collaborator

What is the trick?

The reason is explained there. The workaround is to pass LD_LIBRARY_PATH over su.

Collaborator

git-core commented Feb 27, 2012

What is the trick?

The reason is explained there. The workaround is to pass LD_LIBRARY_PATH over su.

@perettigiuliano

This comment has been minimized.

Show comment
Hide comment
@perettigiuliano

perettigiuliano Feb 27, 2012

Odd... passing LD_LIBRARY_PATH as suggested there there is no errors in logcat but nothing happens...

Odd... passing LD_LIBRARY_PATH as suggested there there is no errors in logcat but nothing happens...

@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Feb 28, 2012

move call to populate_environment(ctx) from line 378 to line 535 where it shall look like

I think our line numbers are a bit off. In the current su.c, the call to populate_environment is on line 377, and line 535 is in the middle a

if (setgroups(0, NULL)) {

block. Can you give me a line before / after marker to move that call? I'm thinking you may have meant move to line 545 after the three set* calls.

erik360 commented Feb 28, 2012

move call to populate_environment(ctx) from line 378 to line 535 where it shall look like

I think our line numbers are a bit off. In the current su.c, the call to populate_environment is on line 377, and line 535 is in the middle a

if (setgroups(0, NULL)) {

block. Can you give me a line before / after marker to move that call? I'm thinking you may have meant move to line 545 after the three set* calls.

@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Feb 28, 2012

Ok, I fixed it. I moved the call to populate_environment(ctx) down about 9 lines, just above #define PARG(arg), and left it as is, not (&ctx). Tested with no errors or seg faults. I also verified LD_LIBRARY_PATH is set for the root shell. I just want to verify that's the proper patch?

erik360 commented Feb 28, 2012

Ok, I fixed it. I moved the call to populate_environment(ctx) down about 9 lines, just above #define PARG(arg), and left it as is, not (&ctx). Tested with no errors or seg faults. I also verified LD_LIBRARY_PATH is set for the root shell. I just want to verify that's the proper patch?

@perettigiuliano

This comment has been minimized.

Show comment
Hide comment
@perettigiuliano

perettigiuliano Feb 28, 2012

Wraithdu nice done!
If you want a beta tester i am here.

Wraithdu nice done!
If you want a beta tester i am here.

@git-core

This comment has been minimized.

Show comment
Hide comment
@git-core

git-core Feb 28, 2012

Collaborator

I think our line numbers are a bit off.

I checked ChainsDD:gc-ics:su.c in the github source browser again. It shows populate_environment is on line 378. Oops, wait, this stupid piece of crap starts counting from line 5 actually. At least, for me. OK. You shall move populate code in main just before "if (setgroups(0, NULL)) {"
(after end of block of "if (chown(REQUESTOR_CACHE_PATH, st.st_uid, st.st_gid)) {")
The inserted line shall be
populate_environment(&ctx);
ctx is of type struct su_context in main() while it's pointer to struct su_context in allow(). Thus, the need of &.
Sorry, I still haven't enough time to fix the code myself.

I moved the call to populate_environment(ctx) down about 9 lines, just above #define PARG(arg)

It works only for cases when you switch to root. It doesn't work at all if you switch to a non-privileged user. If you may outlive this restriction you may keep your approach. The final patch will fix the bug differently, anyway. I'll rather add temporary identity change to populate_environment or near it and keep it exactly in the place it's being located now. But I'm not sure still.

Collaborator

git-core commented Feb 28, 2012

I think our line numbers are a bit off.

I checked ChainsDD:gc-ics:su.c in the github source browser again. It shows populate_environment is on line 378. Oops, wait, this stupid piece of crap starts counting from line 5 actually. At least, for me. OK. You shall move populate code in main just before "if (setgroups(0, NULL)) {"
(after end of block of "if (chown(REQUESTOR_CACHE_PATH, st.st_uid, st.st_gid)) {")
The inserted line shall be
populate_environment(&ctx);
ctx is of type struct su_context in main() while it's pointer to struct su_context in allow(). Thus, the need of &.
Sorry, I still haven't enough time to fix the code myself.

I moved the call to populate_environment(ctx) down about 9 lines, just above #define PARG(arg)

It works only for cases when you switch to root. It doesn't work at all if you switch to a non-privileged user. If you may outlive this restriction you may keep your approach. The final patch will fix the bug differently, anyway. I'll rather add temporary identity change to populate_environment or near it and keep it exactly in the place it's being located now. But I'm not sure still.

@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Feb 28, 2012

Thanks for the clarification. I got your patch to work, so it's good for now until you finalize it.

Just a question... should you be able to 'su 1000' from a non-root shell? Or must you be root first, ie 'su; su 1000' ?

erik360 commented Feb 28, 2012

Thanks for the clarification. I got your patch to work, so it's good for now until you finalize it.

Just a question... should you be able to 'su 1000' from a non-root shell? Or must you be root first, ie 'su; su 1000' ?

@git-core

This comment has been minimized.

Show comment
Hide comment
@git-core

git-core Feb 28, 2012

Collaborator

It's quite legally to change identity from one uid to another. Superuser still asks a user for permissions though.
It has been shown in built-in usage test, by the way.

Collaborator

git-core commented Feb 28, 2012

It's quite legally to change identity from one uid to another. Superuser still asks a user for permissions though.
It has been shown in built-in usage test, by the way.

@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Feb 28, 2012

Well the reason I ask that, is that 'su 1000' from a non-root shell fails.

E/su      (10805): sudb - Opening database
E/su      (10805): sudb - Database opened
E/su      (10805): sudb - Database closed
I/ActivityManager(  185): START {flg=0x10000000 cmp=com.noshufou.android.su/.SuRequestActivity (has extras)} from pid 10596
I/ActivityManager(  185): Displayed com.noshufou.android.su/.SuRequestActivity: +239ms
I/Su.SuRequestActivity(10596): Sending result: ALLOW for UID: 10075
W/su      (10805): SECURITY RISK: Requestor still receives credentials in intent
E/su      (10805): setresgid (1000) failed with 1: Operation not permitted

But issuing 'su' first, so the shell is root, allows 'su 1000' to succeed.

E/su      (11029): sudb - Opening database
E/su      (11029): sudb - Database opened
E/su      (11029): sudb - Database closed
D/su      (11029): 10075 /system/bin/mksh executing 0 /system/bin/sh using shell /system/bin/sh : sh
D/su      (11041): 0 /system/bin/mksh executing 10076 /system/bin/sh using shell /system/bin/sh : sh

erik360 commented Feb 28, 2012

Well the reason I ask that, is that 'su 1000' from a non-root shell fails.

E/su      (10805): sudb - Opening database
E/su      (10805): sudb - Database opened
E/su      (10805): sudb - Database closed
I/ActivityManager(  185): START {flg=0x10000000 cmp=com.noshufou.android.su/.SuRequestActivity (has extras)} from pid 10596
I/ActivityManager(  185): Displayed com.noshufou.android.su/.SuRequestActivity: +239ms
I/Su.SuRequestActivity(10596): Sending result: ALLOW for UID: 10075
W/su      (10805): SECURITY RISK: Requestor still receives credentials in intent
E/su      (10805): setresgid (1000) failed with 1: Operation not permitted

But issuing 'su' first, so the shell is root, allows 'su 1000' to succeed.

E/su      (11029): sudb - Opening database
E/su      (11029): sudb - Database opened
E/su      (11029): sudb - Database closed
D/su      (11029): 10075 /system/bin/mksh executing 0 /system/bin/sh using shell /system/bin/sh : sh
D/su      (11041): 0 /system/bin/mksh executing 10076 /system/bin/sh using shell /system/bin/sh : sh
@git-core

This comment has been minimized.

Show comment
Hide comment
@git-core

git-core Feb 28, 2012

Collaborator

Well, you've caught a bug in su. My congratulations if it's your first su bug.
Will try fix this soon.
(BTW, I guess it might be already fixed in current gc-ics HEAD. For smooth pull you have to drop your HEAD by doing git-reset --hard HEAD^)

Collaborator

git-core commented Feb 28, 2012

Well, you've caught a bug in su. My congratulations if it's your first su bug.
Will try fix this soon.
(BTW, I guess it might be already fixed in current gc-ics HEAD. For smooth pull you have to drop your HEAD by doing git-reset --hard HEAD^)

@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Feb 28, 2012

Heh, thanks :)

I think it's a real bug though, I did a fresh git clone when you merged master into ics. But I can do a fresh clone when I get home to test.

BTW, should I be pulling from ChainsDD's gc-ics branch, or your repo / ics branch?

erik360 commented Feb 28, 2012

Heh, thanks :)

I think it's a real bug though, I did a fresh git clone when you merged master into ics. But I can do a fresh clone when I get home to test.

BTW, should I be pulling from ChainsDD's gc-ics branch, or your repo / ics branch?

@git-core

This comment has been minimized.

Show comment
Hide comment
@git-core

git-core Feb 29, 2012

Collaborator

I think it's a real bug though

No doubt, I do too.
It's introduced in commit "Possible fix for db crash " (00f1bb5), where we changed euid to Superuser uid early on startup. I completely missed the fact that while we can freely switch back to root, 'cause we still hold root privileges as saved set-user-ID, we can't switch to all other uids.
By chance, I've added the code that switches euid back to 0 in populate_environment(), so we hold euid 0 in allow() again. Thus, the bug shall be fixed (for ICS only) in gc-ics (w/rebase, that's why you have to drop your current gc-ics:HEAD).

Collaborator

git-core commented Feb 29, 2012

I think it's a real bug though

No doubt, I do too.
It's introduced in commit "Possible fix for db crash " (00f1bb5), where we changed euid to Superuser uid early on startup. I completely missed the fact that while we can freely switch back to root, 'cause we still hold root privileges as saved set-user-ID, we can't switch to all other uids.
By chance, I've added the code that switches euid back to 0 in populate_environment(), so we hold euid 0 in allow() again. Thus, the bug shall be fixed (for ICS only) in gc-ics (w/rebase, that's why you have to drop your current gc-ics:HEAD).

@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Feb 29, 2012

Ok, so I re-cloned ChainsDD's gc-ics branch, now I see your call to seteuid in the get_parent_env function. However after incorporating the above change to move the call to populate_environment, I'm still seeing the same logcat errors and failure when trying 'su 1000' from a non-root shell.

I believe the problem is this block

if (seteuid(st.st_uid)) {
    PLOGE("seteuid (%lu)", st.st_uid);
    deny(&ctx);
}

dballow = database_check(&ctx);
switch (dballow) {
    case DB_DENY: deny(&ctx);
    case DB_ALLOW: allow(&ctx);
    case DB_INTERACTIVE: break;
    default: deny(&ctx);
}

where seteuid is called again, changing away from root, before calling allow. Since we moved the call to populate_environment out of allow to fix the previous error, we still get this one :/

erik360 commented Feb 29, 2012

Ok, so I re-cloned ChainsDD's gc-ics branch, now I see your call to seteuid in the get_parent_env function. However after incorporating the above change to move the call to populate_environment, I'm still seeing the same logcat errors and failure when trying 'su 1000' from a non-root shell.

I believe the problem is this block

if (seteuid(st.st_uid)) {
    PLOGE("seteuid (%lu)", st.st_uid);
    deny(&ctx);
}

dballow = database_check(&ctx);
switch (dballow) {
    case DB_DENY: deny(&ctx);
    case DB_ALLOW: allow(&ctx);
    case DB_INTERACTIVE: break;
    default: deny(&ctx);
}

where seteuid is called again, changing away from root, before calling allow. Since we moved the call to populate_environment out of allow to fix the previous error, we still get this one :/

@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Feb 29, 2012

Bah, nevermind. I just realized your patch supersedes the earlier one. Successful test just now, inheriting LD_LIBRARY_PATH works and so does 'su 1000' from non-root shell. Nice :)

erik360 commented Feb 29, 2012

Bah, nevermind. I just realized your patch supersedes the earlier one. Successful test just now, inheriting LD_LIBRARY_PATH works and so does 'su 1000' from non-root shell. Nice :)

@perettigiuliano

This comment has been minimized.

Show comment
Hide comment
@perettigiuliano

perettigiuliano Feb 29, 2012

Hi masters,
is there a way to test this release without setting up all enviroment to compile it? I mean, is there someone who could send me binary of this release for testing? I am in the middle of development of my first commercial application but this ICS problem stops me very bad! On HC i was ok but on ICS nothing works anymore. Thanx.

Hi masters,
is there a way to test this release without setting up all enviroment to compile it? I mean, is there someone who could send me binary of this release for testing? I am in the middle of development of my first commercial application but this ICS problem stops me very bad! On HC i was ok but on ICS nothing works anymore. Thanx.

@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Feb 29, 2012

Yeah, you have to setup the whole build environment. But I have it done obviously, so I'll post a link with a compiled version for you to test out. git-core is committing a few things today, so I'll probably make sure I have the latest sources and do a fresh build tonight then post it.

erik360 commented Feb 29, 2012

Yeah, you have to setup the whole build environment. But I have it done obviously, so I'll post a link with a compiled version for you to test out. git-core is committing a few things today, so I'll probably make sure I have the latest sources and do a fresh build tonight then post it.

@git-core

This comment has been minimized.

Show comment
Hide comment
@git-core

git-core Feb 29, 2012

Collaborator

Definitely, not today. I plan to implement and test all new stuff during next weekend. Even everything is needed is a move of a pair of lines in the code.
The commit policies for gc and gc-ics branches are quite different things.

Collaborator

git-core commented Feb 29, 2012

Definitely, not today. I plan to implement and test all new stuff during next weekend. Even everything is needed is a move of a pair of lines in the code.
The commit policies for gc and gc-ics branches are quite different things.

@perettigiuliano

This comment has been minimized.

Show comment
Hide comment
@perettigiuliano

perettigiuliano Feb 29, 2012

Ok thanx,
Where can i find info how to setup build environment?

Ok thanx,
Where can i find info how to setup build environment?

@czuares

This comment has been minimized.

Show comment
Hide comment
@czuares

czuares Feb 29, 2012

@perettigiuliano I just compiled the binary for ICS (as I have a similar interest in this getting fixed).
I've hosted it here

Additionally, the wiki page for this project has all the information you need to setup an environment.

czuares commented Feb 29, 2012

@perettigiuliano I just compiled the binary for ICS (as I have a similar interest in this getting fixed).
I've hosted it here

Additionally, the wiki page for this project has all the information you need to setup an environment.

@perettigiuliano

This comment has been minimized.

Show comment
Hide comment
@perettigiuliano

perettigiuliano Feb 29, 2012

Thanx for the binary but int the wiki page i can't see anything, just a welcome message...

Thanx for the binary but int the wiki page i can't see anything, just a welcome message...

@czuares

This comment has been minimized.

Show comment
Hide comment
@czuares

czuares Feb 29, 2012

@perettigiuliano The instructions are on git-core's wiki page

czuares commented Feb 29, 2012

@perettigiuliano The instructions are on git-core's wiki page

@perettigiuliano

This comment has been minimized.

Show comment
Hide comment
@perettigiuliano

perettigiuliano Feb 29, 2012

Thanx men, i am searching at wiki link at the top of this page. I will never ever find that link without you...

Thanx men, i am searching at wiki link at the top of this page. I will never ever find that link without you...

@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Feb 29, 2012

Actually, it's linked from Issue #1 in this wiki... that's how I found it.

erik360 commented Feb 29, 2012

Actually, it's linked from Issue #1 in this wiki... that's how I found it.

@czuares

This comment has been minimized.

Show comment
Hide comment
@czuares

czuares Feb 29, 2012

I just recompiled using chainsDD's gc-ics branch and re-uploaded the binary. I had originally used git-core's ICS branch and I'm having better results with this version.

czuares commented Feb 29, 2012

I just recompiled using chainsDD's gc-ics branch and re-uploaded the binary. I had originally used git-core's ICS branch and I'm having better results with this version.

@git-core

This comment has been minimized.

Show comment
Hide comment
@git-core

git-core Mar 1, 2012

Collaborator

What was wrong w/ git-core:ics branch? They're virtually the same as long as I don't forget to pull from gc-ics to ics. Or do you really mean the events of the past two weeks?

Collaborator

git-core commented Mar 1, 2012

What was wrong w/ git-core:ics branch? They're virtually the same as long as I don't forget to pull from gc-ics to ics. Or do you really mean the events of the past two weeks?

@czuares

This comment has been minimized.

Show comment
Hide comment
@czuares

czuares Mar 1, 2012

@git-core It was actually a problem in my own code. The two builds work the same.

czuares commented Mar 1, 2012

@git-core It was actually a problem in my own code. The two builds work the same.

@erik360

This comment has been minimized.

Show comment
Hide comment
@erik360

erik360 Mar 1, 2012

Actually, there is a problem with git-core:ics, I just thought you knew about it already, or it was somehow intentional. This is actually why I had asked above which repo I should be using, git-core:ics or ChainsDD:gc-ics. Your branch is missing this small block from line 128 in the get_parent_env function in su.c

if (seteuid(0)) {
    PLOGE("seteuid (root)");
    return NULL;
}

which fixes my 'su 1000' bug.

erik360 commented Mar 1, 2012

Actually, there is a problem with git-core:ics, I just thought you knew about it already, or it was somehow intentional. This is actually why I had asked above which repo I should be using, git-core:ics or ChainsDD:gc-ics. Your branch is missing this small block from line 128 in the get_parent_env function in su.c

if (seteuid(0)) {
    PLOGE("seteuid (root)");
    return NULL;
}

which fixes my 'su 1000' bug.

git-core pushed a commit that referenced this issue Mar 3, 2012

git-core
Change euid back to root before finally set a new id
Fixes a bug reported in a comment to the issue #9 against su-binary
(see #9 (comment) for details).

Thanks to Erik Pilsits (epilsits@gmail.com) for the report.

Add the euid change before populate_environment() anticipating we have to merge
the commit "Inherit LD_LIBRARY_PATH ...", which needs root credentials in order to
successfully open /proc/<ppid>/environ.
@perettigiuliano

This comment has been minimized.

Show comment
Hide comment
@perettigiuliano

perettigiuliano Mar 12, 2012

But in the end has the issue described in #6 been fixed or not?

But in the end has the issue described in #6 been fixed or not?

@git-core

This comment has been minimized.

Show comment
Hide comment
@git-core

git-core Mar 13, 2012

Collaborator

You could check yourself, could you?

Collaborator

git-core commented Mar 13, 2012

You could check yourself, could you?

@perettigiuliano

This comment has been minimized.

Show comment
Hide comment
@perettigiuliano

perettigiuliano Mar 13, 2012

I see the fix, thanx.
but is there a way to test binary without setup all build enviroment?
In more simple way, is there some good fellow who can give to me binary?
Thanx again...

I see the fix, thanx.
but is there a way to test binary without setup all build enviroment?
In more simple way, is there some good fellow who can give to me binary?
Thanx again...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment