linux-kernel - Re: [PATCH] x86-64: fix unwind annotations in entry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20090312115501.GA10391@elte.hu>
Date:	Thu, 12 Mar 2009 12:55:01 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Jan Beulich <jbeulich@...ell.com>
Cc:	Alexander van Heukelum <heukelum@...tmail.fm>,
	Cyrill Gorcunov <gorcunov@...il.com>, tglx@...utronix.de,
	linux-kernel@...r.kernel.org, hpa@...or.com,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] x86-64: fix unwind annotations in entry_64.S

* Jan Beulich <jbeulich@...ell.com> wrote:

> > The current annotations might be completely broken but at 
> > least they _look_ structured and attempt to bring some order 
> > into the CFI annotations chaos.
> 
> There was no chaos. Everything was working fine (and was 
> understandable to someone familiar with CFI annotations). 
> [...]

Well, in the patch you remove a ton of incorrect annotations, 
which all came from the old code. So the old code was all but 
fine - it was ugly and kept breaking all the time.

The thing is, we were this -->.<-- close to removing _all_ CFI 
annotations from x86 assembly files about a year ago. I actually 
wrote all the patches for that and committed them back then.

The reason: they made the assembly code even harder to read and 
distracted from the primary, functional purpose of the code - 
which is functionality. Writing assembly code is hard enough, we 
dont need hundreds of additional CFI annotations to obscure them 
as well.

So when you add back CFI annotations this should be your main 
driving principle: make them as unintrusive as possible. Merge 
them to existing frame layout macros wherever possible. Dont add 
raw CFI annotations. You dont seem to understand and respect any 
of these principles.

You seem to regard them as on par with code but they are not. 
They are at most an afterthought and we will not tolerate them 
if they look ugly - and heck does your patch look ugly.

So we want to hide the CFI details as much as possible 
technically, and make it all as self-maintaining as possible.

Document each and every type of annotation and perhaps also 
write up a small blurb in one of the include files about the 
rules and practices needed for good CFI annotations. That way 
you teach others how to keep it all working fine. Perhaps think 
about writing a KGDB drivn self-test that finds CFI annotation 
bugs - _anything_ that brings this code out of obscurity.

I dont mind quirky features that external entities like your 
dwarf2 unwinder rely on - as long as they benefit the code and 
as long as you participate in the process and as long a you are 
willing to structure it all in a clean way. Coming out of the 
corporate woodwork every 6 months and complaining that "it is 
broken currently" and then going back into hiding without 
changing the dynamics of the code is not enough.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/