Since prototypes with "extern" refer to kernel functions, they make no
sense in userspace, so reject them automatically.
Signed-off-by: Mike Frysinger <[email protected]>
---
scripts/headers_check.pl | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/scripts/headers_check.pl b/scripts/headers_check.pl
index 488a3b1..15b9bc6 100644
--- a/scripts/headers_check.pl
+++ b/scripts/headers_check.pl
@@ -33,6 +33,7 @@ foreach my $file (@files) {
while ($line = <FH>) {
$lineno++;
check_include();
+ check_prototypes();
}
close FH;
}
@@ -54,3 +55,11 @@ sub check_include
}
}
}
+
+sub check_prototypes
+{
+ if ($line =~ m/^\s*extern\b/) {
+ printf STDERR "$filename:$lineno: extern's make no sense in userspace\n";
+ $ret = 1;
+ }
+}
--
1.6.0.6
On Sat, Dec 27, 2008 at 02:43:36AM -0500, Mike Frysinger wrote:
> Since prototypes with "extern" refer to kernel functions, they make no
> sense in userspace, so reject them automatically.
Hi Mike.
I do agree with this - but adding this check resulted in:
usr/include/linux/coda_psdev.h:90: extern's make no sense in userspace
usr/include/linux/elf-fdpic.h:62: extern's make no sense in userspace
usr/include/linux/elf.h:379: extern's make no sense in userspace
usr/include/linux/elf.h:387: extern's make no sense in userspace
usr/include/linux/elf.h:401: extern's make no sense in userspace
usr/include/linux/elf.h:402: extern's make no sense in userspace
usr/include/linux/fs.h:24: extern's make no sense in userspace
usr/include/linux/fs.h:41: extern's make no sense in userspace
usr/include/linux/fs.h:42: extern's make no sense in userspace
usr/include/linux/fs.h:49: extern's make no sense in userspace
usr/include/linux/fs.h:51: extern's make no sense in userspace
usr/include/linux/fs.h:54: extern's make no sense in userspace
usr/include/linux/hid.h:74: extern's make no sense in userspace
usr/include/linux/in6.h:47: extern's make no sense in userspace
usr/include/linux/in6.h:49: extern's make no sense in userspace
usr/include/linux/nubus.h:296: extern's make no sense in userspace
usr/include/linux/nubus.h:298: extern's make no sense in userspace
usr/include/linux/nubus.h:302: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:687: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:995: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:997: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1467: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1760: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1764: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1766: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1769: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1771: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1805: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1948: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1949: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1950: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1951: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1962: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1963: extern's make no sense in userspace
usr/include/linux/reiserfs_fs.h:1964: extern's make no sense in userspace
usr/include/linux/socket.h:29: extern's make no sense in userspace
usr/include/linux/sound.h:33: extern's make no sense in userspace
usr/include/linux/sound.h:34: extern's make no sense in userspace
usr/include/linux/sound.h:35: extern's make no sense in userspace
usr/include/linux/sound.h:36: extern's make no sense in userspace
usr/include/linux/sound.h:37: extern's make no sense in userspace
usr/include/linux/sound.h:39: extern's make no sense in userspace
usr/include/linux/sound.h:40: extern's make no sense in userspace
usr/include/linux/sound.h:41: extern's make no sense in userspace
usr/include/linux/sound.h:42: extern's make no sense in userspace
usr/include/linux/soundcard.h:1047: extern's make no sense in userspace
usr/include/linux/soundcard.h:1048: extern's make no sense in userspace
usr/include/linux/soundcard.h:1049: extern's make no sense in userspace
usr/include/linux/soundcard.h:1050: extern's make no sense in userspace
usr/include/linux/soundcard.h:1051: extern's make no sense in userspace
usr/include/linux/soundcard.h:1053: extern's make no sense in userspace
usr/include/linux/soundcard.h:1055: extern's make no sense in userspace
usr/include/linux/soundcard.h:1056: extern's make no sense in userspace
usr/include/linux/soundcard.h:1061: extern's make no sense in userspace
usr/include/linux/soundcard.h:1062: extern's make no sense in userspace
usr/include/linux/soundcard.h:1078: extern's make no sense in userspace
usr/include/linux/soundcard.h:1079: extern's make no sense in userspace
And we need to get these fixed before we apply this patch.
Otherwise we will break every allyesconfig/allmodconfig builds
Sam
>
>
> And we need to get these fixed before we apply this patch.
> Otherwise we will break every allyesconfig/allmodconfig builds
I turned them into warnings for now.
And when I was there I fixed the TODO:
>From 2bbc376f97c4a75fbbb603b34f73db235a8cd2c1 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <[email protected]>
Date: Sat, 27 Dec 2008 19:52:20 +0100
Subject: [PATCH] kbuild: check for leaked CONFIG_ symbols to userspace
Signed-off-by: Sam Ravnborg <[email protected]>
---
scripts/headers_check.pl | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/scripts/headers_check.pl b/scripts/headers_check.pl
index 5bdd975..72924a7 100644
--- a/scripts/headers_check.pl
+++ b/scripts/headers_check.pl
@@ -16,7 +16,7 @@
#
# 2) It is checked that prototypes does not use "extern"
#
-# 3) TODO: check for leaked CONFIG_ symbols
+# 3) Check for leaked CONFIG_ symbols
use strict;
@@ -36,6 +36,7 @@ foreach my $file (@files) {
$lineno++;
check_include();
check_prototypes();
+ check_config();
}
close FH;
}
@@ -64,3 +65,11 @@ sub check_prototypes
printf STDERR "$filename:$lineno: extern's make no sense in userspace\n";
}
}
+
+sub check_config
+{
+ if ($line =~ m/[^a-zA-Z0-9_]+CONFIG_([a-zA-Z0-9]+)[^a-zA-Z0-9]/) {
+ printf STDERR "$filename:$lineno: leaks CONFIG_$1 to userspace where it is not valid\n";
+ }
+}
+
--
1.6.0.2.GIT
On Saturday 27 December 2008 13:54:15 Sam Ravnborg wrote:
> > And we need to get these fixed before we apply this patch.
> > Otherwise we will break every allyesconfig/allmodconfig builds
>
> I turned them into warnings for now.
i figured you'd choke when you saw the existing failure list :)
-mike
On Saturday 27 December 2008 13:43:55 Sam Ravnborg wrote:
> On Sat, Dec 27, 2008 at 02:43:36AM -0500, Mike Frysinger wrote:
> > Since prototypes with "extern" refer to kernel functions, they make no
> > sense in userspace, so reject them automatically.
>
> I do agree with this
so the next question is about using "extern" in general. unless you have a
nice way of detecting function prototypes (i certainly dont have a magic regex
for this that is multiline, etc...), i think we should be using "extern" at
least in every header that goes to userspace so that we can catch things.
-mike
On Saturday 27 December 2008 13:54:15 Sam Ravnborg wrote:
> > And we need to get these fixed before we apply this patch.
> > Otherwise we will break every allyesconfig/allmodconfig builds
>
> I turned them into warnings for now.
> And when I was there I fixed the TODO:
>
> From 2bbc376f97c4a75fbbb603b34f73db235a8cd2c1 Mon Sep 17 00:00:00 2001
> From: Sam Ravnborg <[email protected]>
> Date: Sat, 27 Dec 2008 19:52:20 +0100
> Subject: [PATCH] kbuild: check for leaked CONFIG_ symbols to userspace
btw, do you maintain these in a git tree somewhere, or you just posting them
to lkml ? the two kbuild trees under your name on kernel.org/git seems to be
lacking this change. i was looking at adding another check: barf if the
header uses __[us]{8,16,32,64} types but does not include linux/types.h.
-mike
On Mon, Dec 29, 2008 at 05:15:08AM -0500, Mike Frysinger wrote:
> On Saturday 27 December 2008 13:54:15 Sam Ravnborg wrote:
> > > And we need to get these fixed before we apply this patch.
> > > Otherwise we will break every allyesconfig/allmodconfig builds
> >
> > I turned them into warnings for now.
> > And when I was there I fixed the TODO:
> >
> > From 2bbc376f97c4a75fbbb603b34f73db235a8cd2c1 Mon Sep 17 00:00:00 2001
> > From: Sam Ravnborg <[email protected]>
> > Date: Sat, 27 Dec 2008 19:52:20 +0100
> > Subject: [PATCH] kbuild: check for leaked CONFIG_ symbols to userspace
>
> btw, do you maintain these in a git tree somewhere, or you just posting them
> to lkml ? the two kbuild trees under your name on kernel.org/git seems to be
> lacking this change. i was looking at adding another check: barf if the
> header uses __[us]{8,16,32,64} types but does not include linux/types.h.
I push regurlary to kbuild-next.git but you hit a window where I had
asked Linus to pull and thus I avoided any updates.
I the meantime I managed to delete the patches I had queued up so
I need to do a bit of digging before I have a fully updated
kbuild-next.git again.
Sam
On Saturday 27 December 2008, Mike Frysinger wrote:
> +
> +sub check_prototypes
> +{
> +???????if ($line =~ m/^\s*extern\b/) {
> +???????????????printf STDERR "$filename:$lineno: extern's make no sense in userspace\n";
> +???????????????$ret = 1;
> +???????}
> +}
Unfortunately, this way you can only detect prototypes with an explicit
'extern' in them, not the equivalent form with an implicit extern.
Not sure if we can detect the other form reliably from perl.
Arnd <><
On Wednesday 31 December 2008 11:58:03 Arnd Bergmann wrote:
> On Saturday 27 December 2008, Mike Frysinger wrote:
> > +sub check_prototypes
> > +{
> > + if ($line =~ m/^\s*extern\b/) {
> > + printf STDERR "extern's make no sense in userspace\n";
> > + $ret = 1;
> > + }
> > +}
>
> Unfortunately, this way you can only detect prototypes with an explicit
> 'extern' in them, not the equivalent form with an implicit extern.
>
> Not sure if we can detect the other form reliably from perl.
yes, i already mentioned both of these issues. that's why i suggested we
start adding "extern" to all the headers (or at least the userspace ones).
-mike
On Wednesday 31 December 2008, Mike Frysinger wrote:
> On Wednesday 31 December 2008 11:58:03 Arnd Bergmann wrote:
>
> > Not sure if we can detect the other form reliably from perl.
>
> yes, i already mentioned both of these issues. ?that's why i suggested we
> start adding "extern" to all the headers (or at least the userspace ones).
The general trend seems to be to remove the "extern" keyword from
kernel headers, because it is not interpreted, just like the 'auto'
keyword. In fact, only few of them currently carry the extern keyword:
$ ctags -f- -R --c-kinds=p obj/usr/include/ | grep extern | wc -l
33
$ ctags -f- -R --c-kinds=p obj/usr/include/ | grep -v extern | wc -l
191
Note that this is a completely separate issue from extern variable
declarations, which should to be disallowed in any case, and are easy
to detect as they always require the extern keyword.
Arnd <><
On Sat, 3 Jan 2009 00:59:18 +0100
Arnd Bergmann <[email protected]> wrote:
> On Wednesday 31 December 2008, Mike Frysinger wrote:
> > On Wednesday 31 December 2008 11:58:03 Arnd Bergmann wrote:
> >
> > > Not sure if we can detect the other form reliably from perl.
> >
> > yes, i already mentioned both of these issues. that's why i
> > suggested we start adding "extern" to all the headers (or at least
> > the userspace ones).
>
> The general trend seems to be to remove the "extern" keyword from
> kernel headers, because it is not interpreted, just like the 'auto'
> keyword. In fact, only few of them currently carry the extern keyword:
>
> $ ctags -f- -R --c-kinds=p obj/usr/include/ | grep extern | wc -l
> 33
>
> $ ctags -f- -R --c-kinds=p obj/usr/include/ | grep -v extern | wc -l
> 191
>
> Note that this is a completely separate issue from extern variable
> declarations, which should to be disallowed in any case, and are easy
> to detect as they always require the extern keyword.
in addition... what business does the kernel have to provide function
prototypes to userspace, other than the system call prototypes?
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
On Saturday 03 January 2009, Arjan van de Ven wrote:
> in addition... what business does the kernel have to provide function
> prototypes to userspace, other than the system call prototypes?
This is the point I raised in my initial comment: we should warn about
all of them if possible. Note that the syscalls.h file is not even
exported. I can't think of a single case where a function protoype
should be exported from the kernel to user space.
Arnd <><
On Friday 02 January 2009 19:09:57 Arnd Bergmann wrote:
> On Saturday 03 January 2009, Arjan van de Ven wrote:
> > in addition... what business does the kernel have to provide function
> > prototypes to userspace, other than the system call prototypes?
>
> This is the point I raised in my initial comment: we should warn about
> all of them if possible. Note that the syscalls.h file is not even
> exported. I can't think of a single case where a function protoype
> should be exported from the kernel to user space.
right, and unless people someone can come up with some magic perl code to
detect function prototypes, i suggest we start adding extern keywords ...
-mike
On Saturday 03 January 2009, Mike Frysinger wrote:
> On Friday 02 January 2009 19:09:57 Arnd Bergmann wrote:
> > On Saturday 03 January 2009, Arjan van de Ven wrote:
> > > in addition... what business does the kernel have to provide function
> > > prototypes to userspace, other than the system call prototypes?
> >
> > This is the point I raised in my initial comment: we should warn about
> > all of them if possible. Note that the syscalls.h file is not even
> > exported. I can't think of a single case where a function protoype
> > should be exported from the kernel to user space.
>
> right, and unless people someone can come up with some magic perl code to
> detect function prototypes, i suggest we start adding extern keywords ...
In the presence of /usr/bin/ctags-exuberant, we can use that to find all of
these. If it doesn't exist, just don't check or warn about the fact that
we might be missing checks.
Arnd <><
Hi.
On Sat, 2008-12-27 at 02:43 -0500, Mike Frysinger wrote:
> Since prototypes with "extern" refer to kernel functions, they make no
> sense in userspace, so reject them automatically.
>
> Signed-off-by: Mike Frysinger <[email protected]>
> ---
> scripts/headers_check.pl | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/scripts/headers_check.pl b/scripts/headers_check.pl
> index 488a3b1..15b9bc6 100644
> --- a/scripts/headers_check.pl
> +++ b/scripts/headers_check.pl
> @@ -33,6 +33,7 @@ foreach my $file (@files) {
> while ($line = <FH>) {
> $lineno++;
> check_include();
> + check_prototypes();
> }
> close FH;
> }
> @@ -54,3 +55,11 @@ sub check_include
> }
> }
> }
> +
> +sub check_prototypes
> +{
> + if ($line =~ m/^\s*extern\b/) {
> + printf STDERR "$filename:$lineno: extern's make no sense in userspace\n";
s/extern's/externs/
> + $ret = 1;
> + }
> +}
Regards,
Nigel