[Toybox] [PATCH] blkdiscard (for review)

[enh]

On Mon, Apr 13, 2020 at 8:45 PM Rob Landley <rob at landley.net> wrote:
>
> On 4/13/20 8:02 PM, enh wrote:
> > 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() :-)
>
> Yeah, agreed here. :)
>
> CFG_TOYBOX_FREE is a bit like having config options within a command in that my
> position on doing that has shifted over the years. I've always prioritized
> "simple" over "small" in toybox, but I've changed my mind on what "simple" means.
>
> "A codepath with less in it" is one type of simple, but "one consistent codepath
> that always behaves the same way" is another type of simple, and these days I
> lean towards the second a bit more strongly. I want to know how it behaves. I
> want that question to have a consistent answer.
>
> > 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));
>
> That's not in mount, that's in ls?

yes. like i said, CFG_TOYBOX_DEBUG was our *workaround for ls* but it
*broke mount* instead. the thread about this was entitled "DEBUG +
NORECURSE "mount" crash when not root" but i don't think it ever got
fixed.

> Meh, it's one cache line. I took out the test to always do the memset.

thanks. i'll revert our patch and get us back in sync.

> > 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.
>
> Half the point is to help me keep track of what cleanup I would need to do if I
> convert commands into MAYFORK.
>
> >> 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.
>
> Ok.
>
> > (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.)
>
> Good to know.
>
> >> 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 :-/
>
> Cool. I haven't met Kernighan but I've watched several long computerphile
> interviews with him on youtube.
>
> (I read Peter Salus' "A quarter century of unix" but haven't yet read The Daemon
> and the Penguin. Partly because the intro was talking about parts I'd lived
> through, and that's somehow less interesting. Less to learn from it, I guess.)
>
> Rob