On Wed, May 13, 2020 at 04:13:53PM -0700, Linus Torvalds wrote:
> The check itself doesn't seem worth it. If your worry is that an empty
> asm() can be optimized away, then don't use an empty asm!
gcc guys said we should use that since the first attempt using
__attribute__((optimize("-fno-stack-protector")))
didn't work because, well, that attribute turned out to be "not suitable in
production code". :)
Full thread here:
https://lore.kernel.org/lkml/20200314164451.346497-1-slyfox@xxxxxxxxxx/
> In other words, the only reason for that check seems to be a worry
> that simply isn't worth having.
Yes, that was me asking for a way to check whether any future gccs would
violate that. But if they'd do that, they would break a lot of code
depending on it.
> In fact, I think the check is wrong anyway, since the main thing I can
> see that would do a tailcall despite the empty asm is link-time
> optimizations that that check doesn't even check for!
>
> So everything I see there just screams "the check is bogus" to me. The
> check doesn't work, and if it were to work it only means that the
> prevent_tail_call_optimization() thing is too fragile.
So I did test it trivially by removing the asm("") and then it would
tailcall optimize. But we didn't think about LTO so hm, that would
probably break it.
> Just put a full memory barrier in there, with an actual "mfence"
> instruction or whatever, so that you know that the check is pointless,
> and so that you know that a link-time optimizer can't turn the
> call+return into a tailcall.
Right, the intention here was to have it arch-agnostic in
include/linux/compiler.h because powerpc might need it too soon:
arch/powerpc/kernel/smp.c:1296: boot_init_stack_canary();
Looking at them, they do have an mb() too so how about this then
instead?
#define prevent_tail_call_optimization() mb()
Thx.
--
Regards/Gruss,
Boris.
SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
