Re: [PATCH v4 00/16] linux: generalize sections, ranges and linker tables

From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: "gnomes@lxorguk.ukuu.org.uk" <gnomes@lxorguk.ukuu.org.uk>,
	linux-ia64@vger.kernel.org, Jiri Kosina <jkosina@suse.cz>,
	"benh@kernel.crashing.org" <benh@kernel.crashing.org>,
	Ming Lei <ming.lei@canonical.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	platform-driver-x86@vger.kernel.org,
	James Bottomley <James.Bottomley@hansenpartnership.com>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	linux-arch <linux-arch@vger.kernel.org>,
	markus.heiser@darmarit.de,
	sparclinux <sparclinux@vger.kernel.org>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Russell King - ARM Linux <linux@arm.linux.org.uk>,
	linux-sh <linux-sh@vger.kernel.org>, Will Deacon <will>
Subject: Re: [PATCH v4 00/16] linux: generalize sections, ranges and linker tables
Date: Tue, 23 Aug 2016 01:06:38 +0200	[thread overview]
Message-ID: <20160822230638.GS3296@wotan.suse.de> (raw)
In-Reply-To: <CAGXu5j+_9374X7hCRn5MYTSph0BVuLndmVb4GJF6MXOt+faG1A@mail.gmail.com>

On Fri, Aug 19, 2016 at 03:29:24PM -0700, Kees Cook wrote:
> On Fri, Aug 19, 2016 at 2:32 PM,  <mcgrof@kernel.org> wrote:
> > From: "Luis R. Rodriguez" <mcgrof@kernel.org>
> >
> > This v4 addresses feedback from the previous v3 series [0], and also
> > addresses a huge array of additional tests against many architectures
> > outside of what 0-day provides. As I mentioned in my last v3 series,
> > 0-day had only found one issue with the series, a blackfin architecture
> > linker issue with the last series. Guenter Rock was kind enough to give
> > my series a test spin on his test bed and he found quite a bit of other
> > oddball issues with obscure architectures, and even on x86 with an old
> > toolchain. After a lot of work and coordinating with a few maintainers
> > I'm happy to report all issues found so far through all possible testing
> > I could do are now fixed, this series also addresses all feedback from
> > the last series, as such this goes submitted as PATCH form.
> >
> > In addressing fixing this work on a few architectures some of the previous
> > patches are further simplified. The kprobes port to linker tables is made
> > much easier now that I've addressed moving out core kprobe declarations
> > into asm-generic/kprobes.h. Refer to the patch "kprobes: move kprobe
> > declarations to asm-generic/kprobes.h". This makes for a much cleaner
> > solution across architectures.
> >
> > Boris feedback on making the code bit rot feature optional is addressed
> > by using a new Kconfig symbol for this, CONFIG_BUILD_AVOID_BITROT,
> > but given Greg's concerns over lack of clarity over what this was all about
> > I've ripped that functionality out into its own patch with a bit more
> > extensive documentation and re-wording. See the patch "kbuild: enable option
> > to force compile force-obj-y and force-lib-y". I hope makes it clear how
> > linker tables can help with avoiding code bit rot. I've gone with a new
> > Kconfig symbol CONFIG_BUILD_AVOID_BITROT given CONFIG_COMPILE_TEST is
> > not available on UML, this feature is desirable on all architectures.
> >
> > The documentation is revamped, now that the DocBook format is deprecated
> > I ported the documention into the trendy hipster Sphinx documentation
> > format.
> >
> > AT Boris' request I've adapated the userspace linker table application
> > forintegration into the kernel under tools/ to make it easier to keep
> > things in sync, however since this requires a bit of changes to some headers
> > in tools/ I'll submit that separately.
> >
> > [0] https://lkml.kernel.org/r/1469222687-1600-1-git-send-email-mcgrof@kernel.org
> >
> > If you'd like this in git-form, you can get it on the 20160819-linker-table-v4
> > branch of my linux-next tree on kernel.org, this also includes the series of
> > the linker table userspace sandbox:
> >
> > https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20160819-linker-table-v4
> >
> > Please let me know if there are any concerns or questions.
> 
> Thanks for the documentation and examples on this feature; I appreciate it! :)
> 
> While it seems like all the section declarations work in this series
> is designed for assembler source, I'm curious if I've missed a way to
> do this in .c source too.

Actually, the whole point to this originally was to provide helpers to
only do this in .c source code -- the fact that there are a few asm helpers
now is an after thought to provide parity of functionality to asm code to do
the exact same thing. The same macros provide the same functionality in
both C and asm code then and if there is some functionality lacking it may
for now just be on the asm side.

So for instance we have DECLARE_SECTION_RANGE() and that exist for C and
asm, but DECLARE_LINKTABLE() is currently only implemented for C code,
when and if we want it we can add it. DECLARE_SECTION_RANGE() has a demo
use in the userspace linker table sandbox.

> I'd love to avoid doing the crazy thing I'm currently doing in lkdtm 
> with section markings. Namely, I want to write a function in .c and have it
> moved into the .rodata section. The linkers get very very angry with me and I
> don't seem to be able to override the progbits to lose "x". Right now I'm
> doing an objcopy in
> drivers/misc/Makefile:
> 
> OBJCOPYFLAGS_lkdtm_rodata_objcopy.o := \
>                         --set-section-flags .text=alloc,readonly \
>                         --rename-section .text=.rodata
> targets += lkdtm_rodata.o lkdtm_rodata_objcopy.o
> $(obj)/lkdtm_rodata_objcopy.o: $(obj)/lkdtm_rodata.o FORCE
>         $(call if_changed,objcopy)
> 
> Thanks!

I'd have to take a closer look but from what you describe indeed this is
something that linker tables and section ranges is supposed to help with -- a
generic API to set sections in an architecture-agnostic way.  If you can
classify your functions with section ranges or linker tables then you should be
set. They flags used is whatever the architecture has decided to use for the
section as per the linker script and where each section falls under. If you
need more special tweaking (see flags in push_section_tbl()) then indeed some
newer APIs might be needed in C, as these flags tweaks are only provided for
asm right now but it'd be good to know if just using the existing APIs with
defaults for the section you needs for C code would not suffice.

If all you have is functions you can stick to section ranges, if these
functions are instead part of a larger data structure you need linker
tables. It sounds like you just have functions, and even if you needed
something outside of what is provided, say the flags, then adding a new
__LINUX_RANGE_FLAGS() could be added easily that also lets you specify
the flags you need.

For an example of section range us kprobes was ported over so we have
for instance:

# define __kprobes      __LINUX_RANGE(SECTION_TEXT, kprobes)

If you had something that needed custom flags you could see if
adding a __LINUX_RANGE_FLAGS() would suffice, but I'd be very
interested to know if the defaults used by the specified section
does not suffice.

Also  your use case seems to require a full file set to a section with specific
attributes. Although first instinct might be to use section ranges, if the
section range accessed some global variable or data structure in the file there
may be an issue, or you'd have to the also peg the data into a section. If this
is done it occurs to me an alternative is to just use a large data structure
to represent the full of functions and data, so to use a linker table that way
the association is done in one full swing with the definition of the table.

This of course is just at a cursory review.

  Luis

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel