[RFC PATCH] kbuild: re-implement detection of CONFIG options leaked to user-space

[RFC PATCH] kbuild: re-implement detection of CONFIG options leaked to user-space

[Masahiro Yamada]

scripts/headers_check.pl can detect references to CONFIG options in
exported headers, but it has been disabled for more than a decade.

Reverting commit 7e3fa5614117 ("kbuild: drop check for CONFIG_ in
headers_check") would emit the following warnings for headers_check
on x86:

usr/include/mtd/ubi-user.h:283: leaks CONFIG_MTD_UBI_BEB_LIMIT to userspace where it is not valid
usr/include/linux/elfcore.h:62: leaks CONFIG_BINFMT_ELF_FDPIC to userspace where it is not valid
usr/include/linux/atmdev.h:104: leaks CONFIG_COMPAT to userspace where it is not valid
usr/include/linux/raw.h:17: leaks CONFIG_MAX_RAW_DEVS to userspace where it is not valid
usr/include/linux/pktcdvd.h:37: leaks CONFIG_CDROM_PKTCDVD_WCACHE to userspace where it is not valid
usr/include/linux/videodev2.h:2465: leaks CONFIG_VIDEO_ADV_DEBUG to userspace where it is not valid
usr/include/linux/bpf.h:249: leaks CONFIG_EFFICIENT_UNALIGNED_ACCESS to userspace where it is not valid
usr/include/linux/bpf.h:819: leaks CONFIG_CGROUP_NET_CLASSID to userspace where it is not valid
usr/include/linux/bpf.h:1011: leaks CONFIG_IP_ROUTE_CLASSID to userspace where it is not valid
usr/include/linux/bpf.h:1742: leaks CONFIG_BPF_KPROBE_OVERRIDE to userspace where it is not valid
usr/include/linux/bpf.h:1747: leaks CONFIG_FUNCTION_ERROR_INJECTION to userspace where it is not valid
usr/include/linux/bpf.h:1936: leaks CONFIG_XFRM to userspace where it is not valid
usr/include/linux/bpf.h:2184: leaks CONFIG_BPF_LIRC_MODE2 to userspace where it is not valid
usr/include/linux/bpf.h:2210: leaks CONFIG_BPF_LIRC_MODE2 to userspace where it is not valid
usr/include/linux/bpf.h:2227: leaks CONFIG_SOCK_CGROUP_DATA to userspace where it is not valid
usr/include/linux/bpf.h:2311: leaks CONFIG_NET to userspace where it is not valid
usr/include/linux/bpf.h:2348: leaks CONFIG_NET to userspace where it is not valid
usr/include/linux/bpf.h:2422: leaks CONFIG_BPF_LIRC_MODE2 to userspace where it is not valid
usr/include/linux/bpf.h:2528: leaks CONFIG_NET to userspace where it is not valid
usr/include/linux/eventpoll.h:82: leaks CONFIG_PM_SLEEP to userspace where it is not valid
usr/include/linux/hw_breakpoint.h:27: leaks CONFIG_HAVE_MIXED_BREAKPOINTS_REGS to userspace where it is not valid
usr/include/linux/cm4000_cs.h:26: leaks CONFIG_COMPAT to userspace where it is not valid
usr/include/linux/pkt_cls.h:301: leaks CONFIG_NET_CLS_ACT to userspace where it is not valid
usr/include/asm-generic/unistd.h:651: leaks CONFIG_MMU to userspace where it is not valid
usr/include/asm-generic/fcntl.h:119: leaks CONFIG_64BIT to userspace where it is not valid
usr/include/asm-generic/bitsperlong.h:9: leaks CONFIG_64BIT to userspace where it is not valid
usr/include/asm/e820.h:14: leaks CONFIG_NODES_SHIFT to userspace where it is not valid
usr/include/asm/e820.h:39: leaks CONFIG_X86_PMEM_LEGACY to userspace where it is not valid
usr/include/asm/e820.h:49: leaks CONFIG_INTEL_TXT to userspace where it is not valid
usr/include/asm/mman.h:7: leaks CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS to userspace where it is not valid
usr/include/asm/auxvec.h:14: leaks CONFIG_IA32_EMULATION to userspace where it is not valid

Most of these are false positives because scripts/headers_check.pl
parses comment lines.

It is also false negative. arch/x86/include/uapi/asm/auxvec.h contains
CONFIG_IA32_EMULATION and CONFIG_X86_64, but the only former is reported.

It would be possible to fix scripts/headers_check.pl, of course.
However, we already have some duplicated checks between headers_check
and CONFIG_UAPI_HEADER_TEST. At this moment of time, there are still
dozens of headers excluded from the header test (usr/include/Makefile),
but we might be able to remove headers_check when the time comes.

I re-implemented it in scripts/headers_install.sh by using sed because
the most of code in scripts/headers_install.sh is written is sed.

This patch works like this:

[1] Run scripts/unifdef first because we need to drop the code
    surrounded by #ifdef __KERNEL__ ... #endif

[2] Remove all C style comments. The sed code is somewhat complicated
    since we need to deal with both single and multi line comments.

    Precisely speaking, a comment block is replaced with a space just
    in case.

      CONFIG_FOO/* this is a comment */CONFIG_BAR

    should be converted into:

      CONFIG_FOO CONFIG_BAR

    instead of:

      CONFIG_FOOCONFIG_BAR

[3] Match CONFIG_... pattern. It correctly matches to all CONFIG options
    that appear in a single line.

After this commit, you will see the following warnings, all of which
are real ones.

warning: include/uapi/linux/elfcore.h: leaks CONFIG_BINFMT_ELF_FDPIC to user-space
warning: include/uapi/linux/atmdev.h: leaks CONFIG_COMPAT to user-space
warning: include/uapi/linux/raw.h: leaks CONFIG_MAX_RAW_DEVS to user-space
warning: include/uapi/linux/pktcdvd.h: leaks CONFIG_CDROM_PKTCDVD_WCACHE to user-space
warning: include/uapi/linux/eventpoll.h: leaks CONFIG_PM_SLEEP to user-space
warning: include/uapi/linux/hw_breakpoint.h: leaks CONFIG_HAVE_MIXED_BREAKPOINTS_REGS to user-space
warning: include/uapi/asm-generic/fcntl.h: leaks CONFIG_64BIT to user-space
warning: arch/x86/include/uapi/asm/mman.h: leaks CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS to user-space
warning: arch/x86/include/uapi/asm/auxvec.h: leaks CONFIG_IA32_EMULATION to user-space
warning: arch/x86/include/uapi/asm/auxvec.h: leaks CONFIG_X86_64 to user-space

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

I was playing with sed yesterday, but the resulted code might be unreadable.

Sed scripts tend to be somewhat unreadable.
I just wondered which language is appropriate for this?
Maybe perl, or what else? I am not good at perl, though.

Maybe, it will be better to fix existing warnings
before enabling this check.
If somebody takes a closer look at them, that would be great.

 scripts/headers_install.sh | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/scripts/headers_install.sh b/scripts/headers_install.sh
index bbaf29386995..73d95e457090 100755
--- a/scripts/headers_install.sh
+++ b/scripts/headers_install.sh
@@ -41,5 +41,34 @@ sed -E -e '
 scripts/unifdef -U__KERNEL__ -D__EXPORTED_HEADERS__ $TMPFILE > $OUTFILE
 [ $? -gt 1 ] && exit 1
 
+# Remove /* ... */ style comments, and find CONFIG_ references in code
+configs=$(sed -e '
+:comment
+	s:/\*[^*][^*]*:/*:
+	s:/\*\*\**\([^/]\):/*\1:
+	t comment
+	s:/\*\*/: :
+	t comment
+	/\/\*/! b check
+	N
+	b comment
+:print
+	P
+	D
+:check
+	s:^[^[:alnum:]_][^[:alnum:]_]*::
+	t check
+	s:^\(CONFIG_[[:alnum:]_]*\):\1\n:
+	t print
+	s:^[[:alnum:]_][[:alnum:]_]*::
+	t check
+	d
+' $OUTFILE)
+
+for c in $configs
+do
+	echo "warning: $INFILE: leaks $c to user-space" >&2
+done
+
 rm -f $TMPFILE
 trap - EXIT
-- 
2.17.1

Re: [RFC PATCH] kbuild: re-implement detection of CONFIG options leaked to user-space

[Arnd Bergmann]

On Tue, Aug 6, 2019 at 6:38 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> I was playing with sed yesterday, but the resulted code might be unreadable.
>
> Sed scripts tend to be somewhat unreadable.
> I just wondered which language is appropriate for this?
> Maybe perl, or what else? I am not good at perl, though.

I like the sed version, in particular as it seems to do the job and
I'm not volunteering to write it in anything else.

> Maybe, it will be better to fix existing warnings
> before enabling this check.

Yes, absolutely.

> If somebody takes a closer look at them, that would be great.

Let's see:

> warning: include/uapi/linux/elfcore.h: leaks CONFIG_BINFMT_ELF_FDPIC to user-space

This one is nontrivial, since it defines two incompatible layouts for
this structure,
and the fdpic version is currently not usable at all from user space. Also,
the definition breaks configurations that have both CONFIG_BINFMT_ELF
and CONFIG_BINFMT_ELF_FDPIC enabled, which has become possible
with commit 382e67aec6a7 ("ARM: enable elf_fdpic on systems with an MMU").

The best way forward I see is to duplicate the structure definition, adding
a new 'struct elf_fdpic_prstatus', and using that in fs/binfmt_elf_fdpic.c.
The same change is required in include/linux/elfcore-compat.h.

> warning: include/uapi/linux/atmdev.h: leaks CONFIG_COMPAT to user-space

The "#define COMPAT_ATM_ADDPARTY" can be moved to include/linux/atmdev.h,
it's not needed in the uapi header

> warning: include/uapi/linux/raw.h: leaks CONFIG_MAX_RAW_DEVS to user-space

This has never been usable, I'd just remove MAX_RAW_MINORS and change
drivers/char/raw.c to use CONFIG_MAX_RAW_DEVS

> warning: include/uapi/linux/pktcdvd.h: leaks CONFIG_CDROM_PKTCDVD_WCACHE to user-space

USE_WCACHING can be moved to drivers/block/pktcdvd.c

> warning: include/uapi/linux/eventpoll.h: leaks CONFIG_PM_SLEEP to user-space

ep_take_care_of_epollwakeup() should not be in the header at all I think.
Commit 95f19f658ce1 ("epoll: drop EPOLLWAKEUP if PM_SLEEP is disabled")
was wrong to move it out of fs/eventpoll.c, and I'd just move it back
as an inline function. (Added Amit to Cc for clarification).

> warning: include/uapi/linux/hw_breakpoint.h: leaks CONFIG_HAVE_MIXED_BREAKPOINTS_REGS to user-space

enum bp_type_idx started out in kernel/events/hw_breakpoint.c
and was later moved to a header which then became public. I
don't think it was ever meant to be public though. We either want
to move it back, or change the CONFIG_HAVE_MIXED_BREAKPOINTS_REGS
macro to an __ARCH_HAVE_MIXED_BREAKPOINTS_REGS that
is explicitly set in a header file by x86 and superh.

> warning: include/uapi/asm-generic/fcntl.h: leaks CONFIG_64BIT to user-space

The #ifdef could just be changed to
#if __BITS_PER_LONG == 32

We could also do this differently, given that most 64-bit architectures define
the same macros in their arch/*/include/asm/compat.h files (parisc and mips
use different values).

> warning: arch/x86/include/uapi/asm/mman.h: leaks CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS to user-space

I think arch_vm_get_page_prot should not be in the uapi header,
other architectures have it in arch/powerpc/include/asm/mman.h

> warning: arch/x86/include/uapi/asm/auxvec.h: leaks CONFIG_IA32_EMULATION to user-space
> warning: arch/x86/include/uapi/asm/auxvec.h: leaks CONFIG_X86_64 to user-space

It looks like this definition has always been wrong, across several
changes that made it wrong in different ways.

AT_VECTOR_SIZE_ARCH is supposed to define the size of the extra
aux vectors, which is meant to be '2' for i386 tasks, and '1' for
x86_64 tasks, regardless of how the kernel is configured. I looked at
this for a bit but it's hard to tell how to fix this without introducing
possible regressions. Note how 'mm->saved_auxv' uses this
size and gets copied between kernel and user space.

       Arnd

Re: [RFC PATCH] kbuild: re-implement detection of CONFIG options leaked to user-space

[Masahiro Yamada]

Hi Arnd,

On Tue, Aug 6, 2019 at 6:00 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Aug 6, 2019 at 6:38 AM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > I was playing with sed yesterday, but the resulted code might be unreadable.
> >
> > Sed scripts tend to be somewhat unreadable.
> > I just wondered which language is appropriate for this?
> > Maybe perl, or what else? I am not good at perl, though.
>
> I like the sed version, in particular as it seems to do the job and
> I'm not volunteering to write it in anything else.
>
> > Maybe, it will be better to fix existing warnings
> > before enabling this check.
>
> Yes, absolutely.
>
> > If somebody takes a closer look at them, that would be great.
>
> Let's see:

Thanks for looking into these!

If possible, could you please send patches to fix them?
We can start with low-hanging fruits to reduce the number of warnings.

Thank you.

-- 
Best Regards
Masahiro Yamada

Re: [RFC PATCH] kbuild: re-implement detection of CONFIG options leaked to user-space

[Christoph Hellwig]

On Tue, Aug 06, 2019 at 11:00:19AM +0200, Arnd Bergmann wrote:
> > I was playing with sed yesterday, but the resulted code might be unreadable.
> >
> > Sed scripts tend to be somewhat unreadable.
> > I just wondered which language is appropriate for this?
> > Maybe perl, or what else? I am not good at perl, though.
> 
> I like the sed version, in particular as it seems to do the job and
> I'm not volunteering to write it in anything else.

Did anyone not like sed?  I have to say I do like scripts using sed and
awk because they are fairly readable and avoid dependencies on "big"
scripting language and their optional modules that sooner or later get
pulled in.

> This one is nontrivial, since it defines two incompatible layouts for
> this structure,
> and the fdpic version is currently not usable at all from user space. Also,
> the definition breaks configurations that have both CONFIG_BINFMT_ELF
> and CONFIG_BINFMT_ELF_FDPIC enabled, which has become possible
> with commit 382e67aec6a7 ("ARM: enable elf_fdpic on systems with an MMU").
> 
> The best way forward I see is to duplicate the structure definition, adding
> a new 'struct elf_fdpic_prstatus', and using that in fs/binfmt_elf_fdpic.c.
> The same change is required in include/linux/elfcore-compat.h.

Yeah, this is a mess.  David Howells suggested something similar when
I brought the issue to his attention last time.