[PATCH] D123544: [randstruct] Automatically randomize a structure of function pointers

Wed Apr 13 04:34:46 PDT 2022

aaron.ballman added a comment.

In D123544#3446814 <https://reviews.llvm.org/D123544#3446814>, @void wrote:

> In D123544#3446416 <https://reviews.llvm.org/D123544#3446416>, @aaron.ballman wrote:
>
>> However, I had forgotten that the base feature *requires* the user to pass a randomization seed via a flag in addition to requiring the attribute (thank you for bringing that back to my attention). Because this feature requires a feature flag to enable it, this behavior *is* a conforming extension (the user has to take an action to get the new behavior). That said, I'm still not convinced we want to do this automagically for users -- it's *really* easy for that flag to be set in a makefile somewhere and the user has no idea that their (non-designated) initialization is now a security vulnerability. If we had error diagnostics when the user is about to shoot their foot off, I'd be more comfortable with the automatic hardening behavior.
>
> We should definitely emit an error if a user is trying to use a default initializer for a structure that's up for randomization. It's something that affects the whole feature, not just structs of function pointers.

Agreed.

> Let me work on that. But otherwise are you okay with this patch?

Yes, I'm okay with the direction of this patch. However, because this still automatically randomizes some structure layouts once the user passes the flag, I think the diagnostic above should perhaps land first unless there's some strong reason not to. (To be honest, I probably should have insisted on it in the first patch, but this one feels significantly more dangerous because it involves function pointers specifically and is more of an "automatic" feature than the first patch.)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123544/new/

https://reviews.llvm.org/D123544