Re: [Toybox] Debugging of the mkpasswd.c ASAN error (It's glibc's fault)
On 3/4/24 20:09, Oliver Webb wrote: > Okay, so ASAN is doing _something_ that replaces the call to crypt with > something else, and since we only do > -lcrypt "as-needed" it does... something. Which means that crypt isn't really > being called. > This is a WEIRD bug, why is ASAN replacing the symbol for crypt so we don't > have to -lcrypt to get it... > > The answer is to declare -lcrypt (-Wl,--as-needed doesn't work). While > somehow keeping compatibility with musl > (which doesn't split libcrypt and libc). More porability.sh stuff, We'd need > a mechanism to detect a glibc build tho No, we need to acknowledge that a recently glibc broke crypt (violating posix) and provide our own crypt() implementation in lib/ because gnu sucks. Working on it... Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] Debugging of the mkpasswd.c ASAN error (It's glibc's fault)
On Monday, March 4th, 2024 at 17:58, Rob Landley wrote:
> Eh, it's us triggering it. Presumably we did something if a zillion other
> people
> haven't seen it. That said, a null pointer dereference isn't an off by one
> error
> or "allocation isn't quite large enough because the buffer's 22 bytes long and
> they're traversing it 32 bits at a time" or some such. That's "the logic took
> a
> wrong turn somewhere".
Did some more testing:
$ echo 'char *crypt(char *, char *); int main(void) { crypt("a", "AA"); }' |
gcc -xc -fsanitize=address - -o mkpasswd && ./mkpasswd
[ASAN error]
$ echo 'char *crypt(char *, char *); int main(void) { crypt("a", "AA"); }' |
gcc -xc - -o mkpasswd && ./mkpasswd
/sbin/ld: /tmp/ccKw47oU.o: in function `main':
:(.text+0x19): undefined reference to `crypt'
collect2: error: ld returned 1 exit status
$ echo 'char *crypt(char *, char *); int main(void) { crypt("a", "AA"); }' |
gcc -xc -fsanitize=address -lcrypt - -o mkpasswd && ./mkpasswd
$
Wha...
Okay, so ASAN is doing _something_ that replaces the call to crypt with
something else, and since we only do
-lcrypt "as-needed" it does... something. Which means that crypt isn't really
being called.
This is a WEIRD bug, why is ASAN replacing the symbol for crypt so we don't
have to -lcrypt to get it...
The answer is to declare -lcrypt (-Wl,--as-needed doesn't work). While somehow
keeping compatibility with musl
(which doesn't split libcrypt and libc). More porability.sh stuff, We'd need a
mechanism to detect a glibc build tho
- Oliver Webb
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] find(1) -name vs -wholename
On 3/4/24 18:03, enh wrote: > On Mon, Mar 4, 2024 at 3:31 PM Rob Landley wrote: >> >> On 3/4/24 12:19, enh via Toybox wrote: >> > obviously the patch is trivial, but i can't think of an existing >> > toybox tool that has one of these "you're holding it wrong" errors, >> > but this is one that i do find useful: >> >> I thought there was one in tar but couldn't find it. Gzip has "need -f to >> read TTY". >> >> I'm not conceptually against "this CAN'T work" errors. (Except this isn't an >> error, it prints to stderr and then exits with 0. Seems a bit indecisive...) > > /facepalm > > (probably no-one noticed yet because it's most likely to be hit > interactively. still seems like a bug though!) Well, a warning isn't exactly an error...? It's design-level indecision. Is this a problem or not? >> > where i'm left wondering why >> > it can't just do the right thing... since `/` is illegal in a POSIX >> > name, what other interpretation could there be? but, still, better >> > than nothing.) >> >> I'd be happy to do the right thing instead? Fairly minor code change either >> way. Thinking about it more, the "right thing" might be for -name to match the trailing whole entries, so if you "find toybox -name pending/git.c" it could come up with "toybox/toys/pending/git.c". Or in my case, my ~/toybox work directory has... 21 toybox repo directories under it (basically instead of branches+stash), so "find . -name toys/*/git.c" could find all the instances of that file using a more shell-like expansion syntax, even if there are subdirectories in the way (such as, real example, ~/toybox/android/toybox/toys/pending/git.c). This is getting us away from "minor code change", though. But I think I already have code for this in... tar.c maybe? All that --anchored stuff calling do_filter() calling fnmatch. Not that hard to do it again with a model at hand. :) > yeah, i'm not sure why coreutils doesn't do that --- perhaps to avoid > the question of whether `-name bits/syscall.h` means `-wholename > .*bits/sycalls.h` or `-wholename .*/bits/syscall.h`? > > (`-path` with a trailing `/` is a similarly unhelpful sharp corner.) My brain is FRIED by packing to move, and I would need test cases with proposed output to follow the distinctions you're making there. I'm also a little confused about what "-wholename" is for given that the shell already does path expansion? I guess it's so you don't get your wildcards back as a result when there's no match? Hmmm... ah, I see, * can eat slashes here, and the shell won't do that. And... what does -path do again? In the toybox directory, none of these produce a result with the host find or toybox find: find . -path pending find . -path toys/pending find . -path toys/pending/ip.c It's been some years since I implemented this stuff. What do the tests do... $ mkdir dir $ touch dir/file $ find . -wholename 'dir*e' find: ‘./dirtest/subdir’: Permission denied $ find dir -wholename 'dir*e' dir/file Ah, the ./ at the start is preventing both -wholename and -path from matching, because of course. >> We could even ping the coreutils guys about that, since they recently agreed >> to >> add -x when I grumped at them. (I'm moving house! It's very stressful!) >> Speaking >> of, I just remembered to ping busybox list about that... Alas, still no cut >> -DF >> in coreutils, last I checked... > > tbh, since starting to read the coreutils list i'm _less_ convinced > that anyone really thinks about anything, Coreutils is gnu. > and especially not about interactions between things. It's very gnu. Your expectations weren't low enough. There's a reason I started poking at busybox back in 2002. The gnu project was announced in 1983, and gnu/hurd remains unusable essentially today. > (i saw the -x,--swap thread but didn't > have the energy to point out the -x,--exchange would have been quite a > bit less unclear...) I know, but I only cared about A) the short option, B) having mv finally able to call that rename() functionality the linux kernel added ten years ago. (Which came up again recently with a patch implementing atomic exchange in the VFAT driver.) Letting the --longopt smell like that particular gatekeeper is fine with me, I never voluntarily use them, and it's sort of the opposite of an ablative duck: https://bwiggs.com/notebook/queens-duck/ He got to keep --swap. He _wanted_ it to be called --swap. Which meant he wanted the feature to go in, because otherwise it couldn't be called --swap. >> Rob Rob P.S. Oddly enough, while Linux beat gnu because gnu sucked, Linux beat BSD using the standard disruptive technology playbook Clayton Christensen described in the Innovator's Dilemma in 1997. The problem FreeBSD had back in 1991 was it was big iron tech ported down to PCs and didn't fit comfortably in the smaller space. A bit like IBM's OS/2, which was full of System Object Model implementations of Common Object Request Broker Architecture, in triplicate.
[Toybox] [PATCH] tests for sha3sum
Nothing to really note here. I noticed there were no tests for sha3sum, so I added some in the attached patch - Oliver Webb From c5ccd89fe39c453ec532bc823cb8828f80deeebe Mon Sep 17 00:00:00 2001 From: Oliver Webb Date: Mon, 4 Mar 2024 18:18:06 -0600 Subject: [PATCH] tests for sha3sum --- tests/sha1sum.test | 22 -- tests/sha3sum.test | 1 + 2 files changed, 17 insertions(+), 6 deletions(-) create mode 12 tests/sha3sum.test diff --git a/tests/sha1sum.test b/tests/sha1sum.test index fa4298d6..d9dacd86 100755 --- a/tests/sha1sum.test +++ b/tests/sha1sum.test @@ -6,42 +6,52 @@ # These tests are based on RFC3174 which were based on FIPS PUB 180-1 -if [ "$CMDNAME" == sha1sum ]; then +case "$CMDNAME" in + sha1sum) ABC=a9993e364706816aba3e25717850c26c9cd0d89d ABCLONG=84983e441c3bd26ebaae4aa1f95129e5e54670f1 MILNUL=34aa973cd4c4daa4f61eeb2bdbad27316534016f DIGITS=dea356a2cddd90c7a7ecedc5ebb563934f460452 DEF=589c22335a381f122d129225f5c0ba3056ed5811 SEQ=f70b7b8768a1183d6d1cd79d3b076d9eb5156350 -elif [ "$CMDNAME" == sha224sum ]; then + ;; sha224sum) ABC=23097d223405d8228642a477bda255b32aadbce4bda0b3f7e36c9da7 ABCLONG=75388b16512776cc5dba5da1fd890150b0c6455cb4f58b1952522525 MILNUL=20794655980c91d8bbb4c1ea97618a4bf03f42581948b2ee4ee7ad67 DIGITS=567f69f168cd7844e65259ce658fe7aadfa25216e68eca0eb7ab8262 DEF=b6773126557f37fbc9b24e7b6adedc05d3eb3923fe3feeb369812d16 SEQ=d8b406f661bc690a1f58241f37c94e04d0a1af74af867b88f18e -elif [ "$CMDNAME" == sha256sum ]; then + ;; sha256sum) ABC=ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad ABCLONG=248d6a61d20638b8e5c026930c3e6039a33ce45964ff2167f6ecedd419db06c1 MILNUL=cdc76e5c9914fb9281a1c7e284d73e67f1809a48a497200e046d39ccc7112cd0 DIGITS=594847328451bdfa85056225462cc1d867d877fb388df0ce35f25ab5562bfbb5 DEF=cb8379ac2098aa165029e3938a51da0bcecfc008fd6795f401178647f96c5b34 SEQ=8060aa0ac20a3e5db2b67325c98a0122f2d09a612574458225dcb9a086f87cc3 -elif [ "$CMDNAME" == sha384sum ]; then + ;; sha384sum) ABC=cb00753f45a35e8bb5a03d699ac65007272c32ab0eded1631a8b605a43ff5bed8086072ba1e7cc2358baeca134c825a7 ABCLONG=3391fdddfc8dc7393707a65b1b4709397cf8b1d162af05abfe8f450de5f36bc6b0455a8520bc4e6f5fe95b1fe3c8452b MILNUL=9d0e1809716474cb086e834e310a4a1ced149e9c00f248527972cec5704c2a5b07b8b3dc38ecc4ebae97ddd87f3d8985 DIGITS=2fc64a4f500ddb6828f6a3430b8dd72a368eb7f3a8322a70bc84275b9c0b3ab00d27a5cc3c2d224aa6b61a0d79fb4596 DEF=180c325cccb299e76ec6c03a5b5a7755af8ef499906dbf531f18d0ca509e4871b0805cac0f122b962d54badc6119f3cf SEQ=7d2a49098f0df0f3c152ca9916a3864542258b2bd487e00ea33cb68e7d27c5c0f25b540d29f62fb33720846073c51b66 -elif [ "$CMDNAME" == sha512sum ]; then + ;; sha512sum) ABC=ddaf35a193617abacc417349ae20413112e6fa4e89a97ea20a964b55d39a2192992a274fc1a836ba3c23a3feebbd454d4423643ce80e2a9ac94fa54ca49f ABCLONG=204a8fc6dda82f0a0ced7beb8e08a41657c16ef468b228a8279be331a703c33596fd15c13b1b07f9aa1d3bea57789ca031ad85c7a71dd70354ec631238ca3445 MILNUL=e718483d0ce769644e2e42c7bc15b4638e1f98b13b2044285632a803afa973ebde0ff244877ea60a4cb0432ce577c31beb009c5c2c49aa2e4eadb217ad8cc09b DIGITS=89d05ba632c699c31231ded4ffc127d5a894dad412c0e024db872d1abd2ba8141a0f85072a9be1e2aa04cf33c765cb510813a39cd5a84c4acaa64d3f3fb7bae9 DEF=40a855bf0a93c1019d75dd5b59cd8157608811dd75c5977e07f3bc4be0cad98b22dde4db9ddb429fc2ad3cf9ca379fedf6c1dc4d4bb8829f10c2f0ee04a3 SEQ=3000c8961bb83de289fa8b407d0ea23f53a57ea11ddb0f782a4ccc0f586780822946053132794b177823c2974873d5dfb2ab1b6c45ae3328e2e703ca907f54d7 -fi + ;; sha3sum) + ABC=e642824c3f8cf24ad09234ee7d3c766fc9a3a5168d0c94ad73b46fdf + ABCLONG=8a24108b154ada21c9fd5574494479ba5c7e7ab76ef264ead0fcce33 + MILNUL=d69335b93325192e516a912e6d19a15cb51c6ed5c15243e7a7fd653c + DIGITS=c97e9b948bcd69bbc0e76b39fc82df963ae61665ee12099b6631ffe6 + DEF=52edc03c27124c2b83ebfd66f0669b6af40b44d4644c12d16b41f46b + SEQ=7049c446c0fb287872536a7737f61f9ac7100f8e6b421d11011505fb + ;; +esac + testcmd "abc" "" "$ABC -\n" "" "abc" testcmd "longer str" "" "$ABCLONG -\n"\ "" "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq" diff --git a/tests/sha3sum.test b/tests/sha3sum.test new file mode 12 index ..bc02ebe0 --- /dev/null +++ b/tests/sha3sum.test @@ -0,0 +1 @@ +sha1sum.test \ No newline at end of file -- 2.44.0 ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] find(1) -name vs -wholename
On Mon, Mar 4, 2024 at 3:31 PM Rob Landley wrote: > > On 3/4/24 12:19, enh via Toybox wrote: > > obviously the patch is trivial, but i can't think of an existing > > toybox tool that has one of these "you're holding it wrong" errors, > > but this is one that i do find useful: > > I thought there was one in tar but couldn't find it. Gzip has "need -f to > read TTY". > > I'm not conceptually against "this CAN'T work" errors. (Except this isn't an > error, it prints to stderr and then exits with 0. Seems a bit indecisive...) /facepalm (probably no-one noticed yet because it's most likely to be hit interactively. still seems like a bug though!) > Alas my find.c is dirty because of the whole pending environment measuring > mess: > > - TT.max_bytes = sysconf(_SC_ARG_MAX) - environ_bytes(); > + TT.max_bytes = child_env_free(0); > > Yet another open can of worms where I need to do heavy lifting to close a > tab... > > > ~/aosp-main-with-phones/prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.17-4.8$ > > find . -name bits/syscall.h > > find: warning: ‘-name’ matches against basenames only, but the given > > pattern contains a directory separator (‘/’), thus the expression will > > evaluate to false all the time. Did you mean ‘-wholename’? > > Code change is easy enough, something like: > > dprintf(2, "%s: -name can't match paths, try -wholename\n", > toys.which->name); > > > (of course, it's also a bit like the macOS `grep -r` "hey, i'm just > > going to sit here doing nothing because -r defaults to stdin rather > > than the `.` that you obviously intended" > > Which debian fixed ages ago. :) it was probably debian that caused me to get out of the habit of typing the `.` :-) > > where i'm left wondering why > > it can't just do the right thing... since `/` is illegal in a POSIX > > name, what other interpretation could there be? but, still, better > > than nothing.) > > I'd be happy to do the right thing instead? Fairly minor code change either > way. yeah, i'm not sure why coreutils doesn't do that --- perhaps to avoid the question of whether `-name bits/syscall.h` means `-wholename .*bits/sycalls.h` or `-wholename .*/bits/syscall.h`? (`-path` with a trailing `/` is a similarly unhelpful sharp corner.) > We could even ping the coreutils guys about that, since they recently agreed > to > add -x when I grumped at them. (I'm moving house! It's very stressful!) > Speaking > of, I just remembered to ping busybox list about that... Alas, still no cut > -DF > in coreutils, last I checked... tbh, since starting to read the coreutils list i'm _less_ convinced that anyone really thinks about anything, and especially not about interactions between things. (i saw the -x,--swap thread but didn't have the energy to point out the -x,--exchange would have been quite a bit less unclear...) > Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] Debugging of the mkpasswd.c ASAN error (It's glibc's fault)
On 3/4/24 12:53, Oliver Webb via Toybox wrote: > I took a closer look at the mkpassword.c error I reported in October. > The problem seems to only happen when ASAN is enabled, and is _inside_ > glibc's crypt(). My tree has its own crypt() I'm moving stuff to because glibc broke that too: https://github.com/landley/toybox/issues/464 You may remember the argument about lists of hash types that need supporting... This whole mess is part of the reason a release has been taking so long, I un-promoted a command when checking that in, and don't feel right doing a release without first fixing that. The thing is, I need proper regression tests for all the commands using this infrastructure, which ideally means I need them to run under mkroot (because I'm not letting tests modify /etc/passwd on my development laptop), but really I should test them in a debootstrap chroot... Which would be happening a lot faster if I wasn't in the process of moving out of texas. (All is boxes! Cardboard everywhere!) > Putting "dprintf(2, "%p, %p, %p -- %s, %s\n", toys.optargs, toybuf, > salt, *toys.optargs ? : toybuf, salt);" > Just before the final xprintf (should be xputs) call, when built with glibc > and ASAN gives: > > $ echo "Don't Panic" | ./mkpasswd > 0x50200010, 0x652862210180, 0x77bc39509020 -- Don't Panic, F. > AddressSanitizer:DEADLYSIGNAL > = > ==70951==ERROR: AddressSanitizer: SEGV on unknown address 0x (pc > 0x bp 0x7fff6ed82b00 sp 0x7fff6ed82a28 T0) > ==70951==Hint: pc points to the zero page. > ==70951==The signal is caused by a READ memory access. > ==70951==Hint: address points to the zero page. > #0 0x0 () > #1 0x6528621f38c3 in main [Path to main.c]:319 > #2 0x77bc3b957ccf (/usr/lib/libc.so.6+0x25ccf) (BuildId: > c0caa0b7709d3369ee575fcd7d7d0b0fc48733af) > > AddressSanitizer can not provide additional info. > SUMMARY: AddressSanitizer: SEGV () > ==70951==ABORTING > > I would build with ASAN using musl to see if it's ASAN's problem or glibc's > but I can't > get musl builds to include libasan And I can't get a static bionic to do so either, it wants to dlopen() a library. > This isn't password.c plumbing that's wrong here, it's either glibc's or > ASAN's problem. > And to fix it mkpasswd.c and any command that calls crypt() should probally > be built without > ASAN if possible (Could we put something in portability.sh here to fix it? Or > do we have to directly > change make.sh and/or single.sh) Eh, it's us triggering it. Presumably we did something if a zillion other people haven't seen it. That said, a null pointer dereference isn't an off by one error or "allocation isn't quite large enough because the buffer's 22 bytes long and they're traversing it 32 bits at a time" or some such. That's "the logic took a wrong turn somewhere". But what I need to do is finish changing the infrastructure and making regression tests for it... Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] find(1) -name vs -wholename
On 3/4/24 12:19, enh via Toybox wrote: > obviously the patch is trivial, but i can't think of an existing > toybox tool that has one of these "you're holding it wrong" errors, > but this is one that i do find useful: I thought there was one in tar but couldn't find it. Gzip has "need -f to read TTY". I'm not conceptually against "this CAN'T work" errors. (Except this isn't an error, it prints to stderr and then exits with 0. Seems a bit indecisive...) Alas my find.c is dirty because of the whole pending environment measuring mess: - TT.max_bytes = sysconf(_SC_ARG_MAX) - environ_bytes(); + TT.max_bytes = child_env_free(0); Yet another open can of worms where I need to do heavy lifting to close a tab... > ~/aosp-main-with-phones/prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.17-4.8$ > find . -name bits/syscall.h > find: warning: ‘-name’ matches against basenames only, but the given > pattern contains a directory separator (‘/’), thus the expression will > evaluate to false all the time. Did you mean ‘-wholename’? Code change is easy enough, something like: dprintf(2, "%s: -name can't match paths, try -wholename\n", toys.which->name); > (of course, it's also a bit like the macOS `grep -r` "hey, i'm just > going to sit here doing nothing because -r defaults to stdin rather > than the `.` that you obviously intended" Which debian fixed ages ago. :) > where i'm left wondering why > it can't just do the right thing... since `/` is illegal in a POSIX > name, what other interpretation could there be? but, still, better > than nothing.) I'd be happy to do the right thing instead? Fairly minor code change either way. We could even ping the coreutils guys about that, since they recently agreed to add -x when I grumped at them. (I'm moving house! It's very stressful!) Speaking of, I just remembered to ping busybox list about that... Alas, still no cut -DF in coreutils, last I checked... Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
[Toybox] Debugging of the mkpasswd.c ASAN error (It's glibc's fault)
I took a closer look at the mkpassword.c error I reported in October. The problem seems to only happen when ASAN is enabled, and is _inside_ glibc's crypt(). Putting "dprintf(2, "%p, %p, %p -- %s, %s\n", toys.optargs, toybuf, salt, *toys.optargs ? : toybuf, salt);" Just before the final xprintf (should be xputs) call, when built with glibc and ASAN gives: $ echo "Don't Panic" | ./mkpasswd 0x50200010, 0x652862210180, 0x77bc39509020 -- Don't Panic, F. AddressSanitizer:DEADLYSIGNAL = ==70951==ERROR: AddressSanitizer: SEGV on unknown address 0x (pc 0x bp 0x7fff6ed82b00 sp 0x7fff6ed82a28 T0) ==70951==Hint: pc points to the zero page. ==70951==The signal is caused by a READ memory access. ==70951==Hint: address points to the zero page. #0 0x0 () #1 0x6528621f38c3 in main [Path to main.c]:319 #2 0x77bc3b957ccf (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV () ==70951==ABORTING I would build with ASAN using musl to see if it's ASAN's problem or glibc's but I can't get musl builds to include libasan This isn't password.c plumbing that's wrong here, it's either glibc's or ASAN's problem. And to fix it mkpasswd.c and any command that calls crypt() should probally be built without ASAN if possible (Could we put something in portability.sh here to fix it? Or do we have to directly change make.sh and/or single.sh) - Oliver Webb ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
[Toybox] find(1) -name vs -wholename
obviously the patch is trivial, but i can't think of an existing toybox tool that has one of these "you're holding it wrong" errors, but this is one that i do find useful: ~/aosp-main-with-phones/prebuilts/gcc/linux-x86/host/x86_64-linux-glibc2.17-4.8$ find . -name bits/syscall.h find: warning: ‘-name’ matches against basenames only, but the given pattern contains a directory separator (‘/’), thus the expression will evaluate to false all the time. Did you mean ‘-wholename’? (of course, it's also a bit like the macOS `grep -r` "hey, i'm just going to sit here doing nothing because -r defaults to stdin rather than the `.` that you obviously intended" where i'm left wondering why it can't just do the right thing... since `/` is illegal in a POSIX name, what other interpretation could there be? but, still, better than nothing.) ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
