Re: [PATCH] fortify: Use WARN instead of BUG for now

[Kees Cook]

On Wed, Jul 26, 2017 at 5:52 AM, Daniel Micay <danielmicay@xxxxxxxxx> wrote:
> It should just be renamed from fortify_panic -> fortify_error, including
> in arch/x86/boot/compressed/misc.c and arch/x86/boot/compressed/misc.c.

Somehow I missed these. I'll send a v2. I wonder why those didn't trip
in my build...

> It can use WARN instead of BUG by with a 'default n', !COMPILE_TEST
> option to use BUG again. Otherwise it needs to be patched downstream
> when that's wanted.

I figure that'll be a separate conversation. For now, we'll do WARN.

> I don't think splitting it is the right approach to improving the
> runtime error handling. That only makes sense for the compile-time
> errors due to the limitations of __attribute__((error)). Can we think
> about that before changing it? Just make it use WARN for now.

Part of Linus's objection was the vague error report. Since the split
didn't bloat things very much, it seemed okay to me.

> The best debugging experience would be passing along the sizes and
> having the fortify_error function convert that into nice error messages.
> For memcpy(p, q, n), n can be larger than both the detected sizes of p
> and q, not just either one. The error should just be saying the function
> name and printing the copy size and maximum sizes of p and q. That's
> going to increase the code size too but I think splitting it will be
> worse and it goes in the wrong direction in terms of complexity. It's
> going to make future extensions / optimization harder if it's split.

Maybe we could do two phases? One to s/BUG/WARN/ and the second to
improve the message?

-Kees

-- 
Kees Cook
Pixel Security