[RFC] [PATCH] make add_taint() inline

[RFC] [PATCH] make add_taint() inline

[donate]

From: Randy Dunlap <rdunlap@xenotime.net>

add_taint() is a trivial function.
No need to call it out-of-line, just make it inline and
remove its export.

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---

 arch/x86_64/kernel/mce_intel.c |    1 +
 include/linux/kernel.h         |    5 ++++-
 kernel/panic.c                 |    6 ------
 3 files changed, 5 insertions(+), 7 deletions(-)

diff -Naurp linux-2613-git10/arch/x86_64/kernel/mce_intel.c~add_taint_inline linux-2613-git10/arch/x86_64/kernel/mce_intel.c
--- linux-2613-git10/arch/x86_64/kernel/mce_intel.c~add_taint_inline	2005-09-10 21:58:37.000000000 -0700
+++ linux-2613-git10/arch/x86_64/kernel/mce_intel.c	2005-09-10 21:58:55.000000000 -0700
@@ -5,6 +5,7 @@
 
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/kernel.h>
 #include <linux/percpu.h>
 #include <asm/processor.h>
 #include <asm/msr.h>
diff -Naurp linux-2613-git10/include/linux/kernel.h~add_taint_inline linux-2613-git10/include/linux/kernel.h
--- linux-2613-git10/include/linux/kernel.h~add_taint_inline	2005-08-28 16:41:01.000000000 -0700
+++ linux-2613-git10/include/linux/kernel.h	2005-09-10 21:54:13.000000000 -0700
@@ -172,7 +172,10 @@ extern int panic_timeout;
 extern int panic_on_oops;
 extern int tainted;
 extern const char *print_tainted(void);
-extern void add_taint(unsigned);
+static inline void add_taint(unsigned flag)
+{
+	tainted |= flag;
+}
 
 /* Values used for system_state */
 extern enum system_states {
diff -Naurp linux-2613-git10/kernel/panic.c~add_taint_inline linux-2613-git10/kernel/panic.c
--- linux-2613-git10/kernel/panic.c~add_taint_inline	2005-08-28 16:41:01.000000000 -0700
+++ linux-2613-git10/kernel/panic.c	2005-09-10 21:55:04.000000000 -0700
@@ -167,9 +167,3 @@ const char *print_tainted(void)
 		snprintf(buf, sizeof(buf), "Not tainted");
 	return(buf);
 }
-
-void add_taint(unsigned flag)
-{
-	tainted |= flag;
-}
-EXPORT_SYMBOL(add_taint);

Re: [RFC] [PATCH] make add_taint() inline

[Kyle Moffett]

On Sep 11, 2005, at 13:44:37, donate wrote:
> From: Randy Dunlap <rdunlap@xenotime.net>
>
> add_taint() is a trivial function.
> No need to call it out-of-line, just make it inline and
> remove its export.

Actually, in this case it might be better to leave add_taint
exported, add and export a new function get_taint(), and then
remove all export of the variable "tainted".  I've actually
seen one case where some module removed taint bits.  I don't
remember where or why, but it seemed really bad at the time,
and still does.  Also, does the tainted variable need any
kind of locking?  What happens if two CPUs try to taint the
kernel simultaneously?

Cheers,
Kyle Moffett

--
Premature optimization is the root of all evil in programming
   -- C.A.R. Hoare

Re: [RFC] [PATCH] make add_taint() inline

[Andrew Morton]

donate <donate@madrone.org> wrote:
>
> From: donate <donate@madrone.org>

Who is this?

> From: Randy Dunlap <rdunlap@xenotime.net>

>  add_taint() is a trivial function.
>  No need to call it out-of-line, just make it inline and
>  remove its export.

Well, presumably add_taint() was exported to modules for a reason.  If that
reason was valid then this patch requires that `tainted' be exported to
modules too.  And that allows naughty modules to trivially zero it out.

Re: [RFC] [PATCH] make add_taint() inline

[Randy.Dunlap]

On Sun, 11 Sep 2005 12:44:34 -0700 Andrew Morton wrote:

> donate <donate@madrone.org> wrote:
> >
> > From: donate <donate@madrone.org>
> 
> Who is this?

Just some little mistake that I have since corrected.
Sorry about that.

> > From: Randy Dunlap <rdunlap@xenotime.net>
> 
> >  add_taint() is a trivial function.
> >  No need to call it out-of-line, just make it inline and
> >  remove its export.
> 
> Well, presumably add_taint() was exported to modules for a reason.  If that
> reason was valid then this patch requires that `tainted' be exported to
> modules too.  And that allows naughty modules to trivially zero it out.

Got it, thanks.  That's why it said [RFC].

---
~Randy

Re: [RFC] [PATCH] make add_taint() inline

[Randy.Dunlap]

On Sun, 11 Sep 2005 14:22:08 -0400 Kyle Moffett wrote:

> On Sep 11, 2005, at 13:44:37, donate wrote:
> > From: Randy Dunlap <rdunlap@xenotime.net>
> >
> > add_taint() is a trivial function.
> > No need to call it out-of-line, just make it inline and
> > remove its export.
> 
> Actually, in this case it might be better to leave add_taint
> exported, add and export a new function get_taint(), and then
> remove all export of the variable "tainted".  I've actually
> seen one case where some module removed taint bits.  I don't

some in-tree module?

> remember where or why, but it seemed really bad at the time,
> and still does.  Also, does the tainted variable need any
> kind of locking?  What happens if two CPUs try to taint the
> kernel simultaneously?

Good question.  one wins?

It sure looks like a problem in theory.  I don't know that
we have ever seen a bug report related to it though.

Maybe Dave Jones's modprobe/insmod killer test on a big
multiprocessor system could do that one day.

---
~Randy

Re: [RFC] [PATCH] make add_taint() inline

[Kyle Moffett]

On Sep 11, 2005, at 18:56:37, Randy.Dunlap wrote:
> On Sun, 11 Sep 2005 14:22:08 -0400 Kyle Moffett wrote:
>
>> On Sep 11, 2005, at 13:44:37, donate wrote:
>>
>>> From: Randy Dunlap <rdunlap@xenotime.net>
>>>
>>> add_taint() is a trivial function.
>>> No need to call it out-of-line, just make it inline and
>>> remove its export.
>>
>> Actually, in this case it might be better to leave add_taint
>> exported, add and export a new function get_taint(), and then
>> remove all export of the variable "tainted".  I've actually
>> seen one case where some module removed taint bits.  I don't
>
> some in-tree module?

No, it was a BSD out-of-tree module, I don't even remember what
it was anymore, it's probably not significant, but if "tainted"
is exported, it seems like some binary module writers might get
the brilliant idea to twiddle it by hand.  How about something
like this to enforce the idea that taintedness is add-only:

void add_taint(unsigned int flags);
unsigned int get_taint(void);

static unsigned int tainted;

Then use atomic bit operations on platforms which have them and
a spinlock on those that don't.  My guess where the lack of
locking might cause issues is if we hit an MCE and bad_page at
the same time on SMP or something.  It seems like it would be
really rare in practice, but I guess that appropriately timed
badness in multiple drivers (perhaps one triggered by the other?)
could cause it to occur.

> Maybe Dave Jones's modprobe/insmod killer test on a big
> multiprocessor system could do that one day.

Or that :-D.  The only way I can see this dying or accidentally
untainting is if two processors try to add different types of
taint simultaneously.

Cheers,
Kyle Moffett

--
Somone asked me why I work on this free (http://www.fsf.org/philosophy/)
software stuff and not get a real job. Charles Shultz had the best  
answer:

"Why do musicians compose symphonies and poets write poems? They do  
it because
life wouldn't have any meaning for them if they didn't. That's why I  
draw
cartoons. It's my life."
   -- Charles Shultz