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

Add rootfs detection by path of /bin/sh #394

Merged
merged 3 commits into from Nov 18, 2022
Merged

Conversation

jlucius
Copy link
Contributor

@jlucius jlucius commented Nov 16, 2022

Signed-off-by: Jens Lucius git@jenslucius.de

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Fix detection of android root path

  • What is the current behavior? (You can also link to an open issue here)
    Does not detect root path

  • What is the new behavior (if this is a feature change)? If possible add a screenshot.

Detect root path by looking for /bin/sh

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

No

  • Other information:

Signed-off-by: Jens Lucius <git@jenslucius.de>
@jlucius
Copy link
Contributor Author

jlucius commented Nov 16, 2022

Fixes #391

Copy link
Member

@m-1-k-3 m-1-k-3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution. Could you check my note

if [[ -d "$R_PATH" ]]; then
ROOT_PATH+=( "$R_PATH" )
if [[ -n "$MECHANISM" ]] && ! echo "$MECHANISM" | grep -q "file names"; then
MECHANISM="$MECHANISM / file names"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's change this from file names to something like shells. Could you please also change this in bash detection?
I think we can improve this detection a bit if we are going to pimp the find command the following way:

find "$SEARCH_PATH" -xdev -path "*bin/sh" -exec file {} \; | grep "ELF" | cut -d: -f1 | sed -E 's/\/.?bin\/sh//'

Same for the bash block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added this, I also reversed the logic of settings the MECHANISM, still have to test if it works correctly. I also want to do more tests, because it feels odd that my filesystem was not detected as root filesystem when looking at the other checks.

- Reverse logic for setting the root dir detection method message
- Add type "shell" for root dir detection

Signed-off-by: Jens Lucius <git@jenslucius.de>
MECHANISM="$MECHANISM / file names"
elif ! echo "$MECHANISM" | grep -q "binary interpreter"; then
MECHANISM="file names"
if [[ -z "$MECHANISM" ]]; then
Copy link
Member

@m-1-k-3 m-1-k-3 Nov 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there an alignment issue?

if [[ -n "$MECHANISM" ]] && ! echo "$MECHANISM" | grep -q "dir names"; then
MECHANISM="$MECHANISM / dir names"
elif ! echo "$MECHANISM" | grep -q "binary interpreter"; then
if [[ -z "$MECHANISM" ]]; then
Copy link
Member

@m-1-k-3 m-1-k-3 Nov 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alignment issue? Which editor are you using? Please set it up to automatically use 2 spaces for a tab

elif [[ -n "$MECHANISM" ]] && ! echo "$MECHANISM" | grep -q "busybox"; then
MECHANISM="$MECHANISM / busybox"
fi
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something happens here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, was some copy and paste issue and I have set my editor to 2 spaces now

@m-1-k-3
Copy link
Member

m-1-k-3 commented Nov 17, 2022

Thanks for your improvements. Are you using check_project.sh ? The script will do a bunch of code checks

Signed-off-by: Jens Lucius <git@jenslucius.de>
@jlucius
Copy link
Contributor Author

jlucius commented Nov 18, 2022

I have also checked why my rootfs was not detected, but the Interpreter path points to some random directory that does not exist in the extracted file system and there are only 4 directories detected for the directory search (/lib, /lib64 /etc /bin), so that missing the at least 5 part, so the code seems to run fine.

@m-1-k-3
Copy link
Member

m-1-k-3 commented Nov 18, 2022

I have also checked why my rootfs was not detected, but the Interpreter path points to some random directory that does not exist in the extracted file system and there are only 4 directories detected for the directory search (/lib, /lib64 /etc /bin), so that missing the at least 5 part, so the code seems to run fine.

Yes, it looks like the interpreter does not always works. This is the reason for the other checks. From our experience these checks are working quite good but there is room for improvements. Probably we could combine the detection mechanisms in the future a bit more. With this we would be able to also detect unusual systems.

For now, thank you for your contribution. You made EMBA better :)

@m-1-k-3 m-1-k-3 merged commit d396652 into e-m-b-a:master Nov 18, 2022
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

Successfully merging this pull request may close these issues.

None yet

2 participants