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

[Regression Bug] suCommand fails on devices without su -c support #877

Open
hongwen000 opened this issue Jul 3, 2024 · 2 comments
Open

Comments

@hongwen000
Copy link

hongwen000 commented Jul 3, 2024

Bug Report Submission

[Bug] suCommand fails on devices without su -c support

Description
In the 8.2.1 version, the application was updated to support non-magisk SU implementations (su 0). The corresponding code was:

val suCommand get() =
    if (isMountMaster)
        "su --mount-master 0"
    else
        "su 0"

However, the current code has changed to:

var suCommand: List<String> = listOf()
    get() {
        if (field.isEmpty()) {
            field = when {
                hasNsEnter -> listOf(
                    "su",
                    "-c",
                    "nsenter --mount=/proc/1/ns/mnt sh"
                )
                hasMountMasterOption -> listOf("su", "--mount-master")
                else -> listOf("su")
            }
        }
        return field
    }

This change causes failures on devices that do not support su -c. I am using AOSP's native su.

Steps To Reproduce

  1. Use a device with AOSP's native su.
  2. Try to backup the apk and data of an app, then failes and saying su -c is not supported.

Expected behavior
Commands should execute successfully on devices that do not support su -c, similar to the behavior in version 8.2.1.

Screenshots
N/A

System Information

  • Device: sunfish
  • Android Version: [version]
  • ROM: AOSP
  • App's Version: 8.3.6

Additional Notes

  • The issue persists in the latest GitHub version.

Possible Solution
Revert to the logic used in version 8.2.1 for suCommand or implement a check to avoid using -c if the device does not support it. Here is a suggested modification:

val suCommand: List<String> by lazy {
    when {
        hasNsEnter -> listOf(
            "su",
            "0",
            "nsenter",
            "--mount=/proc/1/ns/mnt",
            "sh"
        )
        hasMountMasterOption -> listOf("su", "--mount-master", "0")
        else -> listOf("su", "0")
    }
}

Thanks for reporting.

@hongwen000
Copy link
Author

su information on my device:

sunfish:/ # su --help
usage: su [WHO [COMMAND...]]

Switch to WHO (default 'root') and run the given COMMAND (default sh).

WHO is a comma-separated list of user, group, and supplementary groups
in that order.

@hg42
Copy link
Collaborator

hg42 commented Oct 8, 2024

the latest pumpkins (see Telegram group, pinned messages of Bug Reports & Feature Requests) have made the suCommand editable...
it is tried first, so if it works (which means some tests are successful, which is rudimentary for now), it is taken.
So this should work for all cases, you just have to ensure that root commands work like expected.

Note, the commands are piped at stdin into that running shell/su/whatever.
It is usually necessary to provide the nsenter functionality, either your su does it by default or it must have an option like --mount-master that handles the mount namespace (or your Android is old..).

suCommand is executed by libsu as the shell and also by NB itself for the tar commands (to be able to pipe date in and out).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants