Segfault using toybox stat on Android emulator about toybox HOT 10 CLOSED

FYI, since your github bug reports are already public, I've been cc-ing
my replies to the toybox mailing list where I actually hold development
discussions and try to keep a record of stuff. I don't ever expect to
use any of Github's features except the ability to publish/mirror a repo.

For example, see:

http://jsbackus.com/foss/fedora/2016/07/31/taking-over-a-project.html

On 08/11/2016 04:08 AM, Matthias Urhahn wrote:

Can reproduce this with toybox 0.7.1 build against 2016.03 buildroot on
a [email protected].

|root@hammerhead:/sdcard # toybox_sdm stat -c "%u"
twrp-3.0.0-0-hammerhead.img < 0 root@hammerhead:/sdcard # toybox_sdm
stat -c "%U" twrp-3.0.0-0-hammerhead.img < Segmentation fault

The problem here isn't actually android-specific, it's that there's no
NULL check for the lookup having failed. (Oops.)

I have a better lookup function for this already in ls (which returns
the numeric ID as a string if it can't get a name, so the lookup never
fails), which is probably what I thought it was using.

root@hammerhead:/sdcard # toybox_sdm stat -c "%g"
twrp-3.0.0-0-hammerhead.img < 1015 root@hammerhead:/sdcard # toybox_sdm
stat -c "%G" twrp-3.0.0-0-hammerhead.img < Segmentation fault
139|root@hammerhead:/sdcard # |

I would guess resolving/lookup of group id or user id (names?) causes
the segfault.

No, failing to resolve then trying to dereference the resulting NULL
pointer anyway does.

That said, this behavior is different from ubuntu:

$ stat -c '%U %G' stat
landley landley
$ ./stat -c '%U %G' stat
landley landley

We're padding to 8 chars, they're not. (Why? Dunno, I think the original
submission did that.)

$ sudo chown 12345:23456 stat
...
$ ./stat -c '%U' stat
12345
$ ./stat -c '%G' stat
23456
$ stat -c '%G' stat
UNKNOWN
$ stat -c '%U' stat
UNKNOWN

I'd worry that "UNKNOWN" had a special meaning if I couldn't also make a
user named "UNKNOWN". :)

Ok, ubuntu isn't indenting at all. Hmmm...

$ stat /bin/ls
File: ‘/bin/ls’
Size: 110080 Blocks: 216 IO Block: 4096 regular file
Device: 801h/2049d Inode: 393412 Links: 1
Access: (0755/-rwxr-xr-x) Uid: ( 0/ root) Gid: ( 0/ root)
Access: 2016-08-12 22:16:36.184015479 -0500
Modify: 2016-03-10 13:10:57.000000000 -0600
Change: 2016-03-27 22:44:13.960855491 -0500
Birth: -

And that's why the submitter was identing. Because the default output
does variable length indents, padding to 8 characters. Hmmm...

$ stat -c "%11G" /bin/ls
root

Of course they did that. Why wouldn't they do that?

Right, I actually_have_code_for_this, it's the insanitize() function
in seq.c, and I genericized next_printf() out into lib/lib.c in case it
came up again.

The reason for this insane sanitization pass is that printf has
dangerous options: %n writes to the stack, and %7p can prints the 7th
argument even when you haven't got 7 arguments. (How this handles %lld
arguments I have no idea. I guess it assumes they're all integers? Added
back when everythign was promoted to "int" when it was passed on the
stack, which the people implementing 64 bits decided not to move to long
because CLEARLY that would waste too much memory and we can't have that,
because the people with 64k memory wasting 3 out of 4 bytes for a char
were SO much more resource constrained than we are today... Grrr.)

Anyway, you have to filter out possible $ and %n and so on to sanely
pass user-supplied %BLAHx infixes along to your own printf logic. Hence
the function to do that.

It works using the stock toybox that ships with Android 6.0.1 on the
Nexus5 (--version shows |c96e42498c99-android|):

|root@hammerhead:/sdcard # stat -c "%G" twrp-3.0.0-0-hammerhead.img root
root@hammerhead:/sdcard # stat -c "%U" twrp-3.0.0-0-hammerhead.img root |

So this could be a toolchain issue?

Nope, another tangent entirely. :)

Working on it.

Thanks,

Rob

from toybox.

On 07/21/2016 02:58 AM, Michael Eder wrote:

|toybox stat| is crashing on Android emulator running Android 5.1.1
(Lollipop)

|root@generic:/data/local # ./toybox-armv6l stat script.sh File:
`script.sh' Size: 14722 Blocks: 32 IO Blocks: 4096 regular file Device:
1f01h/7937d Inode: 14524 Links: 1 Access: (770/-rwxrwx---Segmentation
fault |

Hmmm... Under qemu-system-arm I get:

$ ./toybox stat toybox
File: `toybox'
Size: 297768 Blocks: 588 IO Blocks: 1024 regular file
Device: 810h/2064d Inode: 23 Links: 1
Access: (555/-r-xr-xr-x) Uid: (0/ root) Gid: (0/ root)
Access: 2016-07-22 06:43:49.980000000
Modify: 2016-07-22 06:43:50.010000000
Change: 2016-07-22 06:43:50.020000000

I tried building current git from source, and "wget
http://landley.net/toybox/bin/toybox-armv6l" and both behaved the same
for me.

Possibly qemu isn't complaining about an unaligned access correctly? (I
thought it was...)

I tried with the latest release version from the toybox website
http://landley.net/toybox/bin/ for arm-v6l as well as a version built
from latest git using your cross-compiler for armv6 from your homepage:

|root@generic:/data/local # ./toybox stat toybox File: `toybox' Size:
467296 Blocks: 920 IO Blocks: 4096 regular file Device: 1f01h/7937d
Inode: 14531 Links: 1 Access: (555/-r-xr-xr-xSegmentation fault |

It seems that the emulator is running ARMv7l, but AFAIK it should be
compatible to ARMv6l binaries:

Yeah, it should.

You're running a binary statically built against uClibc with the old gcc
4.2 toolchain, so it's not a bionic thing nor a toolchain thing. You
built current git so it's not version skew.

What it really sounds like is the vanilla qemu I'm using and the android
emulator you're using are doing something different? Or maybe a kernel
.config issue? (I rebuilt qemu from current git to make sure it wasn't
the oldish version i was using, and I still get the full unsegfaulted
output.

Now it's possible what's happening is that the next thing it tries to do
is fetch the uid, and the android kernel code is behaving differently
because of how they've set up user ID's.

Or maybe it's a problem trying to display a uid that's not in
/etc/passwd, which sadly I can't easily test here at the moment because
I just noticed that "chown 123:123 toybox" is saying:

chown: user '123'

Which is broken in a couple ways (there should be a ":errno message" on
the end of that if it's an actual error, and numeric user IDs should
work even if they're not in /etc/passwd) so I need to off and fix THAT
before cycling back to this. :)

If I can't reproduce this with a bit more fiddling here, I should
download this android emulator and build environment. I'm guessing that
would be from... Android NDK version 10c?

https://developer.android.com/about/versions/android-5.0-changes.html
https://developer.android.com/ndk/downloads/revision_history.html

(Sigh, is the NDK standalone with its own emulator or does it assume you
have the SDK installed already? The NDK page assumes you already know
this. Oh well, I can dig through it, lemme fix the chown thing first...)

Thanks,

Rob

from toybox.

Hi Rob,

first of all thanks for the quick reply and your awesome work on this project!

To be honest, I used a ROM that was a little bit modified, but in order to make reproducing easier to you I tried from scratch on a different machine using the official images from Google. The problem occurs there, too.

First of all, you actually don't need the NDK, but the SDK. It contains a tool called android in its tools/ directory which brings the SDK manager when launched without parameters. It is going to tell you to install a lot of stuff, but I reproduced it using only the following items:

• Android 5.1.1 (API 22) SDK platform
• Android 5.1.1 (API 22) ARM EABI v7a System Image
• Google APIs ARM EABI v7a Image
• Android SDK tools
• Android SDK platform-tools
• Android SDK build-tools

Most of them may neither be required, but it's a good foundation and since this isn't about the minimum required packages I did not investigate which package actually contains adb, avd and so on.

So, after installing the packages, I ran android avd to create a new launchable configuration. I used the Nexus 5 preset, enabled GPU acceleration and basically kept the defaults to bring the ARM-v7 image up. After it booted, I pushed the toybox via adb push /path/to/toybox /data/local/ to the emulator and adb shell gives a root shell on the emulator. I changed to /data/local/, made the toybox executable and run it's stat on the file itself, getting the same result:

/toybox-armv6l stat toybox-armv6l                                             <
  File: `toybox-armv6l'
  Size: 444376   Blocks: 872     IO Blocks: 4096    regular file
Device: 1f01h/7937d  Inode: 14176    Links: 1
Access: (777/-rwxrwxrwxSegmentation fault 

I hope this description saves your time.

I'd like to note that android has no /etc/passwd, it seems they are storing this information somewhere else. Since Android's permission system relies on applications having their own UID Google built its own solution for user management. Apart from that, the file is owned by user and group root which I silently assume to exist.

from toybox.

works fine on master built against bionic:

$ adb shell stat /system/bin/toybox
  File: `/system/bin/toybox'
  Size: 366184   Blocks: 720     IO Blocks: 4096    regular file
Device: 1030dh/66317d    Inode: 385  Links: 1
Access: (755/-rwxr-xr-x)    Uid: (0/    root)   Gid: (2000/   shell)
Access: 2016-07-29 23:34:19.000000000
Modify: 2016-07-29 23:34:19.000000000
Change: 2016-07-29 23:34:19.000000000

from toybox.

Can reproduce this with toybox 0.7.1 build against 2016.03 buildroot on a [email protected].

root@hammerhead:/sdcard # toybox_sdm stat -c "%u" twrp-3.0.0-0-hammerhead.img                          <
0
root@hammerhead:/sdcard # toybox_sdm stat -c "%U" twrp-3.0.0-0-hammerhead.img                          <
Segmentation fault
root@hammerhead:/sdcard # toybox_sdm stat -c "%g" twrp-3.0.0-0-hammerhead.img                          <
1015
root@hammerhead:/sdcard # toybox_sdm stat -c "%G" twrp-3.0.0-0-hammerhead.img                          <
Segmentation fault
139|root@hammerhead:/sdcard #

I would guess resolving/lookup of group id or user id (names?) causes the segfault.

It works using the stock toybox that ships with Android 6.0.1 on the Nexus5 (--version shows c96e42498c99-android):

root@hammerhead:/sdcard # stat -c "%G" twrp-3.0.0-0-hammerhead.img
    root
root@hammerhead:/sdcard # stat -c "%U" twrp-3.0.0-0-hammerhead.img
    root

So this could be a toolchain issue?

Busybox build against buildroot-2016 doesn't work either, but returns UNKNOWN instead of segfaulting.

from toybox.

I think the segfault originates here, because group_name is null in TT.group_name->gr_name. (Same goes for user name).

Busybox does a NULL check to return UNKNOWN.

from toybox.

I've been cc-ing my replies to the toybox mailing list
(...)
For example, see: http://jsbackus.com/foss/fedora/2016/07/31/taking-over-a-project.html

I don't mind the CC, I've also signed up to get a dialy digest, just find contribution (and issue tracking) through GitHub more comfortable (I also think it attracts more contributions).
Is this a problem?

from toybox.

On 08/14/2016 06:37 AM, Matthias Urhahn wrote:

I've been cc-ing my replies to the toybox mailing list
(...)
For example, see:
http://jsbackus.com/foss/fedora/2016/07/31/taking-over-a-project.html

I don't mind the CC, I've also signed up to get a dialy digest, just
find contribution (and issue tracking) through GitHub more comfortable
(I also think it attracts more contributions).
Is this a problem?

Nah, it's fine. Just letting you know where my primary communication
channels are. I can keep replying to this, I just like to keep a record
there so I can go back and find stuff.

Thanks,

Rob

from toybox.

@landley I think I could submit a patch that fixes the segfault using the getusername / getgroupname functions you mentioned. Should I do that?

from toybox.

I made a fresh build from git, seems to be fixed now. Thanks!

from toybox.