Re: CONFIG_* symbols in UAPI headers?
Paul Bolle schreef op di 09-04-2019 om 21:11 [+0200]: > Does it still apply? It does, cleanly. Output is (for v5.1-rc4): ./usr/include/asm-generic/fcntl.h:119: leaks CONFIG_64BIT to userspace where it is not valid ./usr/include/asm-generic/mman-common.h:22: leaks CONFIG_MMAP_ALLOW_UNINITIALIZED 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/flat.h:17: leaks CONFIG_BINFMT_SHARED_FLAT 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/pktcdvd.h:37: leaks CONFIG_CDROM_PKTCDVD_WCACHE 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/hw_breakpoint.h:27: leaks CONFIG_HAVE_MIXED_BREAKPOINTS_REGS 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/asm/auxvec.h:14: leaks CONFIG_IA32_EMULATION 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 Which suggests it might be doable to silence these warnings. Paul Bolle
Re: CONFIG_* symbols in UAPI headers?
Christoph Hellwig schreef op ma 08-04-2019 om 14:46 [+0200]: > There are a few similar issues, like struct elf_prstatus having > a different layout depending on CONFIG_BINFMT_ELF_FDPIC, or > MAX_SHARED_LIBS defending on CONFIG_BINFMT_SHARED_FLAT. I've had the patch pasted below in a local branch for over five years, but never dared to push it upstream. Does it still apply? Thanks, Paul Bolle -- >8 --- >From 0f73c8ee776c197e3029c4eed21af0f121a8f9d3 Mon Sep 17 00:00:00 2001 From: Paul Bolle Date: Tue, 4 Feb 2014 22:22:48 +0100 Subject: [PATCH] headers_check: enable check for CONFIG_ leakage The check for leaked CONFIG_ symbols was disabled in v2.6.30, because it generated too much noise. But a (rather simplistic) preprocessing of comments suffices to silence the noise (ie, no false positives are reported anymore). So add some preprocessing of comments and enable check_config() again. Signed-off-by: Paul Bolle --- scripts/headers_check.pl | 33 - 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/scripts/headers_check.pl b/scripts/headers_check.pl index b6aec5e4365f..8e67017c1b67 100755 --- a/scripts/headers_check.pl +++ b/scripts/headers_check.pl @@ -36,13 +36,36 @@ foreach my $file (@files) { open(my $fh, '<', $filename) or die "$filename: $!\n"; $lineno = 0; + my $in_comment = 0; + my $swap_state = 0; while ($line = <$fh>) { $lineno++; - _include(); - _asm_types(); - _sizetypes(); - _declarations(); - # Dropped for now. Too much noise _config(); + + # strip inline comments + $line =~ s|/\*.*\*/||; + + # try to handle multi line comments + if ($in_comment == 0 and $line =~ m|/\*|) { + $line =~ s|/\*.*$||; + # we still need to check (the first half of) this line + # so we set $in_comment after the checks + $swap_state = 1; + } + if ($in_comment == 1 and $line =~ m|\*/|) { + $line =~ s|^.*\*/||; + $in_comment = 0; + } + unless ($in_comment) { + check_include(); + check_asm_types(); + check_sizetypes(); + check_declarations(); + check_config(); + } + if ($swap_state) { + $in_comment = 1; + $swap_state = 0; + } } close $fh; } -- 2.17.2
Re: CONFIG_* symbols in UAPI headers?
Arnd Bergmann wrote: > > I just stumbled over the MAP_UNINITIALIZED defintion, initially > > added by: > > > > commit ea637639591def87a54cea811cbac796980cb30d > > Author: Jie Zhang > > Date: Mon Dec 14 18:00:02 2009 -0800 > > > > nommu: fix malloc performance by adding uninitialized flag > > > > The defintion depends on CONFIG_MMAP_ALLOW_UNINITIALIZED, which > > will never be set by userspace. How is this supposed to work? > > > > Shoudn't we define the symbol unconditionally and just turn it > > into a no-op in the implementation? Yes. > Right, good catch. That should work. It can probably be done > by adding another check before the conditional, like: > >/* clear anonymous mappings that don't ask for uninitialized data */ >if (!vma->vm_file && >!(IS_ENABLED(CONFIG_MMAP_ALLOW_UNINITIALIZED) && > (flags & MAP_UNINITIALIZED)) >memset((void *)region->vm_start, 0, > region->vm_end - region->vm_start); Sounds good. > > There are a few similar issues, like struct elf_prstatus having > > a different layout depending on CONFIG_BINFMT_ELF_FDPIC, or > > MAX_SHARED_LIBS defending on CONFIG_BINFMT_SHARED_FLAT. Because the kernel code uses that header and that struct too, so you'd break compilation of binfmt_elf_fdpic.c. There is a way round it - and that's to copy the struct into the non-UAPI backing header and delete the conditional section from the UAPI one. You'd have to stop the non-UAPI header from #including the UAPI header, though, and you'd have to hope that no one is trying to set it in userspace (gdb doesn't). David
Re: CONFIG_* symbols in UAPI headers?
On Mon, Apr 8, 2019 at 2:46 PM Christoph Hellwig wrote: > > I just stumbled over the MAP_UNINITIALIZED defintion, initially > added by: > > commit ea637639591def87a54cea811cbac796980cb30d > Author: Jie Zhang > Date: Mon Dec 14 18:00:02 2009 -0800 > > nommu: fix malloc performance by adding uninitialized flag > > The defintion depends on CONFIG_MMAP_ALLOW_UNINITIALIZED, which > will never be set by userspace. How is this supposed to work? > > Shoudn't we define the symbol unconditionally and just turn it > into a no-op in the implementation? Right, good catch. That should work. It can probably be done by adding another check before the conditional, like: /* clear anonymous mappings that don't ask for uninitialized data */ if (!vma->vm_file && !(IS_ENABLED(CONFIG_MMAP_ALLOW_UNINITIALIZED) && (flags & MAP_UNINITIALIZED)) memset((void *)region->vm_start, 0, region->vm_end - region->vm_start); > There are a few similar issues, like struct elf_prstatus having > a different layout depending on CONFIG_BINFMT_ELF_FDPIC, or > MAX_SHARED_LIBS defending on CONFIG_BINFMT_SHARED_FLAT. I thought we had cleaned them out a long time ago, but it looks like those have been there since the uapi headers got split out, and are still wrong. Why doesn't scripts/headers_check.pl find those? Arnd
CONFIG_* symbols in UAPI headers?
I just stumbled over the MAP_UNINITIALIZED defintion, initially added by: commit ea637639591def87a54cea811cbac796980cb30d Author: Jie Zhang Date: Mon Dec 14 18:00:02 2009 -0800 nommu: fix malloc performance by adding uninitialized flag The defintion depends on CONFIG_MMAP_ALLOW_UNINITIALIZED, which will never be set by userspace. How is this supposed to work? Shoudn't we define the symbol unconditionally and just turn it into a no-op in the implementation? There are a few similar issues, like struct elf_prstatus having a different layout depending on CONFIG_BINFMT_ELF_FDPIC, or MAX_SHARED_LIBS defending on CONFIG_BINFMT_SHARED_FLAT.