[Toybox] [PATCH] blkdiscard (for review)

[enh]

On Fri, Apr 10, 2020 at 12:33 AM Rob Landley <rob at landley.net> wrote:
>
> On 4/9/20 12:54 AM, Patrick Oppenlander wrote:
> > Hi Rob,
> >
> > I needed blkdiscard for an embedded job so had a go at putting it together.
>
> Applied, let's see...
>
> > Length & offset parsing don't match util-linux where MB, GB are powers
> > of 1000 and M, MiB, G, GiB are powers of 1024.
> >
> > I didn't use xioctl to avoid leaking fd if CFG_TOYBOX_FREE is set. Is
> > there a better solution to this?
>
> CFG_TOYBOX_FREE is... squishy.
>
> In theory there's no point freeing memory and closing files right before a
> program exits, because the process is about to go away and the OS will clean all
> that up. But the android guys like running stuff under valgrand and such to find
> resource leaks (even when they don't necessarily matter).

TL;DR: always use xioctl() :-)

i think you're confusing a few different things here. we do have fd
checks, yes. we're also moving as fast as we can to a world where the
hardware faults on memory errors
(https://android-developers.googleblog.com/2020/02/detecting-memory-corruption-bugs-with-hwasan.html
being our latest missive on the subject).

historically we had to use CFG_TOYBOX_DEBUG when were were having asan
issues with ls, but CFG_TOYBOX_DEBUG broke mount. so i committed a
local patch (afair our only patch that isn't upstream):

  memset(totals, 0, sizeof(totals));
  if (CFG_TOYBOX_ON_ANDROID || CFG_TOYBOX_DEBUG) memset(len, 0, sizeof(len));

to ensure toybox ls was asan-clean, and stopped setting
CFG_TOYBOX_DEBUG. even after the mount bug was fixed, i didn't
re-enable CFG_TOYBOX_DEBUG since it's much less well tested and not
intended to be used.

as for CFG_TOYBOX_FREE, afaik we've never used that.

> There are long-running
> programs that shouldn't leak (if "sed" leaked memory for each line of input it
> processed it would run out of memory handling a large enough input file), but
> something like nice/sleep/nproc/mount/insmod ever freeing memory is kind of
> optional.
>
> The use cases I had in mind when I put in CFG_TOYBOX_FREE were:
>
> A) the busybox space I was leaving behind (routers and such)
>
> B) knoppix as described in https://www.youtube.com/watch?v=MkJkyMuBm3g#t=1m18s
>
> C) Countering Trusting Trust
> (https://www.cs.cmu.edu/~rdriley/487/papers/Thompson_1984_ReflectionsonTrustingTrust.pdf)
> not just through diverse double compiling (https://dwheeler.com/trusting-trust/)
> but through a fully auditable system small enough people can examine the
> binaries if they really want to.
>
> All of which can benefit from saving a few bytes here and there, which argues
> for dropping the unnecessary cleanup entirely. But the arguments in _favor_ of
> having it were:
>
> D) Any command that can fully clean up after itself could run MAYFORK from the
> toybox shell, which is faster.
>
> E) If somebody wants to run this stuff on bare metal (or glue it into a
> bootloader like u-boot; some context where you haven't GOT an OS cleaning up
> after you) then each command has to clean up after itself because nothing else will.
>
> Since then Android showed up and ate 80% of the attention, and they like having
> the cleanup because valgrind and their own automated auditing, so they switch
> the option on in their configs.

see above. no we don't.

(but we are moving to a world where code that isn't asan clean just
won't run on production hardware. luckily hwasan is a good enough
approximation and fast enough to run in continuous testing that these
days we're catching the bugs pretty quickly.)

> But for a command that's never going to run
> mayfork and never be run from a bare metal context... eh? The leak only ever
> causes a real world problem in the D and E cases, and "blkdiscard" isn't
> something there's any point in micro-optimizing. (Running test/echo/true in the
> shell's process is a signifcant enough timesaver to be worth a nofork audit.)
>
> Rob
>
> P.S. Alright, I'll explain (C):
>
> Long ago the inventor of Unix modified his C compiler to recognize when it was
> compling the login program and insert a backdoor root account, then modified the
> C compiler to recognize when it was compiling _itself_ and insert two backdoors:
> one for the login program, and the new one for itself. Then he deleted the
> backdoor from the compiler source, but gave the BSD guys the hacked binary that
> would trojan both programs in a self-replicating way.

off-topic, but i heard that Kernighan wrote a new book "UNIX: A
History and a Memoir". seems like it's only available from amazon
though :-/

> Years later somebody gave him a lifetime achievement award and he explained what
> he'd done in his acceptance speech. Wheeler's PHD thesis was "if you build with
> two different compilers you can clean that sort of trojan out because they'd
> have to recognize _each_other_ and predict you were going to do that".
>
> MY fix is to make a system small and simple enough that a sufficiently trained
> group of professional reverse engineers can make it through "objdump -d" of the
> whole thing in a reasonable amount of time, and then build the system under that
> known good base. (This is what the Norad guys were doing in the video in (B)
> which his why they were using the smallest and simplest implementations they
> could find for high-security systems.) And we're doing the hardware side of that
> over at https://j-core.org.)
>
> Still Rob
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net