Re: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h

["Zack Weinberg" <zack@owlfolio.org>]

On Mon, Jul 10, 2023, at 5:03 PM, Xi Ruoyao via Libc-alpha wrote:
> On Mon, 2023-07-10 at 16:55 -0400, Zack Weinberg wrote:
>> On Mon, Jul 10, 2023, at 4:33 PM, Jeff Law via Libc-alpha wrote:
>> > Essentially up to the point where the UB happens we have to preserve
>> > visible side effects.  After the point where UB happens anything goes 
>> > and our goal has been mark the paths through the CFG as dying at that 
>> > point and forcing an immediate halt of the program (via __buitin_trap()).
>> > 
>> > There this all gets fuzzy is something like the NULL pointer property 
>> > where the fact that a pointer must be non-null can backward propagate. 
>> > ie, if a parameter is marked as non-null, then we will mark the 
>> > corresponding SSA_NAME in the compiler as non-null.  Thus if there was 
>> > some comparison of the SSA_NAME against NULL (perhaps well before the 
>> > call), we'll optimize away that comparison.
>> 
>> Yep, see, that in and of itself is dangerous.
>> 
>> The bright line I would draw is: optimizations based on the assumption
>> that control cannot proceed past the point where UB occurs are OK;
>> optimizations based on the assumption that control cannot *reach* the
>> point where UB occurs are *not* OK.  Removing a comparison to NULL,
>> based on the observation that *later in some execution trace* the
>> program will definitely dereference that pointer, falls in the latter
>> category, *even if* there are no externally visible side effects in
>> between the two points.
>
> Why it's not OK if no externally visible side effects is between the two
> points?

The short answer is that the C standard's definition of "externally visible side effect" is insufficient for the needs of certain security-critical programs. In particular, *how long it takes for the program to crash* may be an exploitable timing channel. (Look up "bomb oracle attack" in the cryptography literature if that seems impossible.)

> So what should we do now?  Remove all __nonnull from the headers?

That would be my choice. Or, leave them in as documentation, but make the macro expand to nothing except with specific compilers that are known not to optimize unsafely (this may be an empty set at present)

> Or "some __nonnull are OK but my is not"?!

I don't like that we have __nonnull at all. But adding it to stdio.h functions in particular is especially dangerous because of how widely used they are. I would say the same thing if you  were adding it to string.h or stdlib.h.

zw