CONFIG_* symbols in UAPI headers?

CONFIG_* symbols in UAPI headers?

[Christoph Hellwig]

I just stumbled over the MAP_UNINITIALIZED defintion, initially
added by:

commit ea637639591def87a54cea811cbac796980cb30d
Author: Jie Zhang <jie.zhang@analog.com>
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.

Re: CONFIG_* symbols in UAPI headers?

[Arnd Bergmann]

On Mon, Apr 8, 2019 at 2:46 PM Christoph Hellwig <hch@lst.de> wrote:
>
> I just stumbled over the MAP_UNINITIALIZED defintion, initially
> added by:
>
> commit ea637639591def87a54cea811cbac796980cb30d
> Author: Jie Zhang <jie.zhang@analog.com>
> 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

Re: CONFIG_* symbols in UAPI headers?

[David Howells]

Arnd Bergmann <arnd@arndb.de> wrote:

> > I just stumbled over the MAP_UNINITIALIZED defintion, initially
> > added by:
> >
> > commit ea637639591def87a54cea811cbac796980cb30d
> > Author: Jie Zhang <jie.zhang@analog.com>
> > 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?

[Paul Bolle]

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 <pebolle@tiscali.nl>
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 <pebolle@tiscali.nl>
---
 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++;
-		&check_include();
-		&check_asm_types();
-		&check_sizetypes();
-		&check_declarations();
-		# Dropped for now. Too much noise &check_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?

[Paul Bolle]

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