[Toybox] [PATCH] blkdiscard (for review)

[Rob Landley]

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?

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

> 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