<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">---------- Forwarded message ---------<br>From: <b class="gmail_sendername" dir="auto">Jazzoo Watchman</b> <span dir="auto"><<a href="mailto:jazzoowatchman@gmail.com">jazzoowatchman@gmail.com</a>></span><br>Date: Wed, Nov 25, 2020 at 8:14 AM<br>Subject: Re: [Toybox] config2help SEGFAULT<br>To: Rob Landley <<a href="mailto:rob@landley.net">rob@landley.net</a>><br></div><br><br><div dir="ltr"><div>Rob,</div><div><br></div><div>Thank you for your time. I have learned a great deal studying your work and following you on and off since your time at the "fool."</div><div><br></div><div>My post was not meant in any way to criticize or second guess the code in config2help.c. I seek to understand if my analysis follows the intent of the author(s) or I missed the point entirely. And on the off-chance that I can help-out, to do so.<br></div><div></div><div><br></div><div>Cloned Toybox into a new folder.<br></div><div></div><div><br></div><div>Toybox builds cleanly. <br></div><div><br></div><div>I apologize for that distraction and thank you for the generous gift of your time. <br></div><div><br></div><div>I'm trying to build a development environment for an arm926tj-s variant device for a device that the OED and OEM have NOT released their source nor their build tools.</div><div><br></div><div>Aboriginal-1.4.5 seems like a good starting point to generate the tools I'd need.</div><div></div><div><br></div><div>Aboriginal has not been "touched" in 5 years ... but one can hope to stand on the shoulders ... <br></div><div><br></div><div>If you can/will point me in a more fruitful direction I'd be grateful.<br></div><div><br></div><div>The following are the pieces to reproduce should you deem fit to comment. Otherwise, please disregard; no harm, no foul.<br></div><div></div><div></div><div><br></div><div>Thanks again for your time,</div><div><br></div><div><br></div><div>James<br></div><div><br></div><div>Build Environment</div><div></div><div>* Linux 4.19.0-12-amd64 #1 SMP Debian 4.19.152-1 (2020-10-18) x86_64 GNU/Linux</div><div></div><div>* gcc (Debian 8.3.0-6) 8.3.0</div><div></div><div>* GNU ld (GNU Binutils for Debian) 2.31.1</div><div>* ldd (Debian GLIBC 2.28-10) 2.28</div><div><br></div><div>Cloned aboriginal-1.4.5 into an empty folder</div><div></div><div><br></div><div>ran ./build.sh armv5l</div><div><br></div><div>... snip out the clean parts ...<br></div><div>=== toybox (host host-tools)<br>Snapshot 'toybox'...<br>scripts/genconfig.sh<br>cc -o kconfig/conf kconfig/conf.c kconfig/zconf.tab.c -DKBUILD_NO_NLS=1 \<br> -DPROJECT_NAME=\"ToyBox\"<br>kconfig/conf -D /dev/null Config.in > /dev/null<br>scripts/make.sh<br>Generate headers from toys/*/*.c...<br>generated/newtoys.h Library probe.......<br>Make generated/config.h from .config.<br>generated/flags.h generated/globals.h generated/help.h<br>scripts/make.sh: line 218: 17069 Segmentation fault generated/config2help Config.in $KCONFIG_CONFIG > generated/help.h<br>make: *** [Makefile:19: toybox] Error 1<br><br>Exiting due to errors (host host-tools toybox)<br><br>real 0m4.541s<br>user 0m3.272s<br>sys 0m1.597s</div><div><br></div><div><br></div><div>On 11/24/20 12:35 PM, Jazzoo Watchman wrote:</div><div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> Hello all,<br>
> Hope this finds you well.<br>
> <br>
> Trying to run config2help it segfaults.<br>
<br>
It doesn't for me, which means I can't reproduce this, which means I won't know<br>
if I've fixed it.<br>
<br>
You don't mention how you did it. Which distro? Which compiler version? Which<br>
build flags? I'm guessing you're using some kind of page poisoning?<br>
<br>
> Pulling the source from Toybox and building with<br>
> gcc -0 -g -Wall<br>
> <br>
> That too segfaults so I think I have the latest version (the source seems to be<br>
> mostly contained in a single file scripts/config2help.c).<br>
> <br>
> Google(ing) turned up many hits of fairly recent (circa 2018+) conversations on<br>
> or about this same complaint. <br>
> <br>
> Digging into the logic of the code, it seems there may be some cruft/age to the<br>
> logic and some mishandling of pointer use AFTER free (as shown by valgrind). In<br>
<br>
It's more that it's a quick and dirty build time tool that works on known<br>
inputs, so I got it working and then didn't care. The only way it could be<br>
"exploitable" is somehow tricking someone into running the build with different<br>
inputs...?<br>
<br>
The comment at the top of mkflags.c is a bit more explicit:<br>
<br>
// This is intentionally crappy code because we control the inputs. It leaks<br>
// memory like a sieve and segfaults if malloc returns null, but does the job.<br>
<br>
I have the following diff in my tree as a todo:<br>
<br>
--- a/scripts/config2help.c<br>
+++ b/scripts/config2help.c<br>
@@ -146,7 +146,8 @@ char *dlist_zap(struct double_list **help)<br>
struct double_list *dd = dlist_pop(help);<br>
char *s = dd->data;<br>
<br>
- free(dd);<br>
+ memset(dd, 0xaa, sizeof(struct double_list));<br>
+// free(dd);<br>
<br>
return s;<br>
}<br>
<br>
Which might or might not be related to what you're seeing because you didn't<br>
tell me how to reproduce what you're seeing. To be honest, the "fix" I'd do is<br>
just remove the free and leak the data because again: quick and dirty build tool<br>
working on known input? generated/flags.raw is like 10k of data, it can't leak<br>
enough to matter over the lifetime of the tool's run.<br>
<br>
(The only reason I have the patch is in case I got around to doing a better fix<br>
than that.)<br>
<br>
> "some" aforementioned Google(ing) complaints seemed so stem from an update of<br>
> their glibc. I can attest to the same as my builds were going smoothly until I<br>
> did an apt update; apt upgrade .<br>
<br>
Ok, so you're using a debian derivative of some sort. This is the first<br>
information you've given me about your build environment. I'm guessing you<br>
"upgraded" gcc (or llvm?) to a newer version that enables an address<br>
sanitization flag by default? But that's just a guess because you didn't SAY.<br>
<br>
> My point.<br>
> <br>
> 1.)<br>
> Is the latest source (hopefully) with a fix for the "use after free" segfault in<br>
> "<a href="https://github.com/landley/toybox/blob/master/scripts/config2help.c" rel="noreferrer" target="_blank">https://github.com/landley/toybox/blob/master/scripts/config2help.c</a>"<br>
<br>
Given that you apparently haven't built that, now I have to ask "what version of<br>
toybox _did_ you build"?<br>
<br>
I don't know what you built, _and_ I don't know how you built it.<br>
<br>
> 2.)<br>
> Following the code in 1.) it seems that some of the logic may be more complex<br>
> than the input would suggest.<br>
<br>
It turns out config2help.c did have a reason to exist, yes.<br>
<br>
> The code inspecting the generated "sym" list while looping through the entries<br>
> in ".config" and setting sym->enabled ... indicates to me that ".config"<br>
> controls which "processed" "symbols" get spit-out as "#defined HELPXXX". <br>
> <br>
> Is that true and or intended?<br>
<br>
The config controls what output is produced. More output has been added over<br>
time to a loop that already had selection logic in it. As I said: quick and<br>
dirty build tool, which has had its design goals shift out from under it a bit<br>
over the years.<br>
<br>
I have a post-1.0 todo item to rewrite kconfig (so I'm not using a stale<br>
snapshot of gplv2 kernel code), and as part of that all this build<br>
infrastructure might change, or go away and get completely redone. Dunno yet,<br>
haven't designed the replacement.<br>
<br>
> The flow seems to indicate that multiple "symbols" of the same "name" may/do<br>
> exist over multiple Config.in (and "sourced") files; and "duplicates"<br>
> need/should to be "collated".<br>
<br>
The nature of kconfig is there's dependency information: symbols depend on other<br>
symbols. I repurposed kconfig's compile-time help text to provide each command's<br>
--help output, which requires the help text of dependent symbols to be collated<br>
into a single help block with the main command symbol's help text (the one that<br>
controls whether or not a command by that name gets installed), and THAT<br>
requires the help text to have a common structure sections can be parsed out and<br>
processed differently.<br>
<br>
We used to make much heavier use of this (like busybox does), but these days<br>
I've ripped out most command sub-options, with only a few like CONFIG_LSPCI_TEXT<br>
remaining. If I unify all of them like that, then most of the help text<br>
processing can go away. But I haven't done that yet because it has side effects<br>
I need to think through.<br>
<br>
We might still need the FLAG_ enable and disable logic to let the dead code<br>
elimination part of the compiler's optimizer shrink commands in certain<br>
configurations, but at the same time without command options that would only<br>
apply to multiple commands in the same command.c file, and most of the time<br>
those FORCE_FLAGS to disable that behavior when it would apply (for reasons<br>
probably explained in code.html?) So MAYBE that goes too...? Given that LLVM<br>
also has __COUNTER__ possibly mkflags.c isn't needed at all? Except could that<br>
work with the re-include of flags.h to CLEANUP_xxx ? In theory I can snapshot<br>
the current value of __COUNTER__ and then subtract to get "how many times since<br>
last reset", and MAYBE grep/sed can chop out the optstr and expand it, but is<br>
that really cleaner...?<br>
<br>
tl;dr: I used to use certain techniques a lot, these days I don't and have<br>
removed most of the old instances of them, but there's still a few I haven't<br>
chased down. If I DO chase them down, I need to do a design and analysis pass to<br>
figure out what support infrastructure would still be needed, and I haven't yet.<br>
I've been busy with other things, and it's overshadowed by the need to write a<br>
new menuconfig from scratch. And none of this is pressing because the old one<br>
works for now and I have higher priority todo items.<br>
<br>
> In practice; I haven't seen "duplicate" "symbols"<br>
> and wonder if mashing "duplicates" together wouldn't lead to probable<br>
> error/nonsense.<br>
<br>
CONFIG_LSPCI_TEXT has the help text:<br>
<br>
usage: lspci [-n] [-i FILE ]<br>
<br>
-n Numeric output (repeat for readable and numeric)<br>
-i PCI ID database (default /usr/share/misc/pci.ids)<br>
<br>
Which needs to be spliced into the CONFIG_LSPCI help text. That sort of thing<br>
used to be a lot more common. I haven't yet decided to completely eliminate it<br>
(permanently hardwiring all such options on), and worked out what subset of<br>
config2help.c would still be needed if I did, because I'm busy with things like<br>
trying to finish toys/*/sh.c<br>
<br>
> 3.)<br>
> If the above are/is true; would a rewrite with less "dependencies" and a "clean"<br>
> valgrind run be at-all welcome?<br>
<br>
I haven't done a new design pass on the build infrastructure, in part because<br>
deciding what's stale and what isn't requires janitorial work on the rest of the<br>
tree to make sure it's no longer used (eliminating the last few users), and then<br>
work out the best way to do what's left, which isn't necessarily a subset of the<br>
existing code but may be something entirely different. For one thing, I'm most<br>
of the way through writing a new shell with known behavior which I could then<br>
rely on to have things like "wait -n" so scripts/make.sh could more reliably<br>
keep multiple processors active instead of using the "portable" (to old bash<br>
versions at least) wait for specific $PID version which isn't necessarily the<br>
nex tone to finish because gcc instances take varying amounts of time to run<br>
depending on what .c file they were given... which means maybe I could write a<br>
config2help.c replacement as a shell function using variable arrays, I don't<br>
know yet?<br>
<br>
You seem to be volunteering to understand what toybox needs better than I do.<br>
<br>
> In closing, am I just lost in the woods? If so, could someone spin me around<br>
> and nudge me in the correct direction?<br>
<br>
You had a build break. You didn't tell me how to reproduce the build break<br>
you're seeing so I can tell whether or not I fixed it. That APPEARS to be the<br>
problem at hand.<br>
<br>
One quick and dirty fix to your build break is not to run address sanitization<br>
on build-time tools. (HOSTCC vs target CC.) Alternately, I can remove the free()<br>
so it just leaks that field, because it's just a compile time tool working on<br>
10k of known input.<br>
<br>
Rob<br>
</blockquote></div></div></div>
</div></div>