2008-01-22 19:14:16

by Jon Masters

[permalink] [raw]
Subject: CONFIG_MARKERS

Yo,

I notice in module.c:

#ifdef CONFIG_MARKERS
if (!mod->taints)
marker_update_probe_range(mod->markers,
mod->markers + mod->num_markers, NULL, NULL);
#endif

Is this an attempt to not set a marker for proprietary modules? If so,
then this really should be the following conditional instead, because,
really we're not guaranteeing there won't be other taints (e.g. in RHEL
we already have the module signing patch, and then there's also the
TAINT_FORCED_MODULE, which arguably isn't a "taint" for markers):

#ifdef CONFIG_MARKERS
if (!(mod->taints & TAINT_PROPRIETARY_MODULE))
marker_update_probe_range(mod->markers,
mod->markers + mod->num_markers, NULL, NULL);
#endif

Or am I missing something?

Jon.


2008-01-23 03:02:03

by Frank Ch. Eigler

[permalink] [raw]
Subject: Re: CONFIG_MARKERS


Jon Masters <[email protected]> writes:

> I notice in module.c:
>
> #ifdef CONFIG_MARKERS
> if (!mod->taints)
> marker_update_probe_range(mod->markers,
> mod->markers + mod->num_markers, NULL, NULL);
> #endif
>
> Is this an attempt to not set a marker for proprietary modules? [...]

I can't seem to find any discussion about this aspect. If this is the
intent, it seems misguided to me. There may instead be a relationship
to TAINT_FORCED_{RMMOD,MODULE}. Mathieu?

- FChE

2008-01-23 03:10:22

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: CONFIG_MARKERS

* Frank Ch. Eigler ([email protected]) wrote:
>
> Jon Masters <[email protected]> writes:
>
> > I notice in module.c:
> >
> > #ifdef CONFIG_MARKERS
> > if (!mod->taints)
> > marker_update_probe_range(mod->markers,
> > mod->markers + mod->num_markers, NULL, NULL);
> > #endif
> >
> > Is this an attempt to not set a marker for proprietary modules? [...]
>
> I can't seem to find any discussion about this aspect. If this is the
> intent, it seems misguided to me. There may instead be a relationship
> to TAINT_FORCED_{RMMOD,MODULE}. Mathieu?
>
> - FChE

On my part, its mostly a matter of not crashing the kernel when someone
tries to force modprobe of a proprietary module (where the checksums
doesn't match) on a kernel that supports the markers. Not doing so
causes the markers to try to find the marker-specific information in
struct module which doesn't exist and OOPSes.

Christoph's point of view is rather more drastic than mine : it's not
interesting for the kernel community to help proprietary modules writers,
so it's a good idea not to give them marker support. (I CC'ed him so he
can clarify his position).

Mathieu

--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

2008-01-23 04:18:19

by Jon Masters

[permalink] [raw]
Subject: Re: CONFIG_MARKERS


On Tue, 2008-01-22 at 22:10 -0500, Mathieu Desnoyers wrote:
> * Frank Ch. Eigler ([email protected]) wrote:
> >
> > Jon Masters <[email protected]> writes:
> >
> > > I notice in module.c:
> > >
> > > #ifdef CONFIG_MARKERS
> > > if (!mod->taints)
> > > marker_update_probe_range(mod->markers,
> > > mod->markers + mod->num_markers, NULL, NULL);
> > > #endif
> > >
> > > Is this an attempt to not set a marker for proprietary modules? [...]
> >
> > I can't seem to find any discussion about this aspect. If this is the
> > intent, it seems misguided to me. There may instead be a relationship
> > to TAINT_FORCED_{RMMOD,MODULE}. Mathieu?
> >
> > - FChE
>
> On my part, its mostly a matter of not crashing the kernel when someone
> tries to force modprobe of a proprietary module (where the checksums
> doesn't match) on a kernel that supports the markers. Not doing so
> causes the markers to try to find the marker-specific information in
> struct module which doesn't exist and OOPSes.
>
> Christoph's point of view is rather more drastic than mine : it's not
> interesting for the kernel community to help proprietary modules writers,
> so it's a good idea not to give them marker support. (I CC'ed him so he
> can clarify his position).

Right. I thought that was your collective opinion, and I happen to
personally agree with you, but my question was more that you should be
explicitly comparing to whether it's proprietary and not just whether
the taints field is set - there are other flags in there too.

Jon.

2008-01-23 13:21:35

by Frank Ch. Eigler

[permalink] [raw]
Subject: Re: CONFIG_MARKERS

Hi -

On Tue, Jan 22, 2008 at 11:17:40PM -0500, Jon Masters wrote:
> On Tue, 2008-01-22 at 22:10 -0500, Mathieu Desnoyers wrote:
> > [...]
> > > > Is this an attempt to not set a marker for proprietary modules? [...]
> > >
> > > I can't seem to find any discussion about this aspect. If this is the
> > > intent, it seems misguided to me. There may instead be a relationship
> > > to TAINT_FORCED_{RMMOD,MODULE}. Mathieu?

> > On my part, its mostly a matter of not crashing the kernel when someone
> > tries to force modprobe of a proprietary module (where the checksums
> > doesn't match) on a kernel that supports the markers. Not doing so
> > causes the markers to try to find the marker-specific information in
> > struct module which doesn't exist and OOPSes.

But you have the wrong target: it is not proprietary modules that have
this risk but those built out-of-tree without checksums. Maybe
oopsing in this case is not so bad; or the check could just limit itself to
FORCED_MODULE.


> > Christoph's point of view is rather more drastic than mine : it's not
> > interesting for the kernel community to help proprietary modules writers,
> > so it's a good idea not to give them marker support. (I CC'ed him so he
> > can clarify his position).
> Right. I thought that was your collective opinion

Another way of looking at this though is that by allowing/encouraging
proprietary module writers to include markers, we and their users get
new diagnostic capabilities. It constitutes a little bit of opening
up, which IMO we should reward rather than punish.


- FChE

2008-01-23 14:48:25

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: CONFIG_MARKERS

* Frank Ch. Eigler ([email protected]) wrote:
> Hi -
>
> On Tue, Jan 22, 2008 at 11:17:40PM -0500, Jon Masters wrote:
> > On Tue, 2008-01-22 at 22:10 -0500, Mathieu Desnoyers wrote:
> > > [...]
> > > > > Is this an attempt to not set a marker for proprietary modules? [...]
> > > >
> > > > I can't seem to find any discussion about this aspect. If this is the
> > > > intent, it seems misguided to me. There may instead be a relationship
> > > > to TAINT_FORCED_{RMMOD,MODULE}. Mathieu?
>
> > > On my part, its mostly a matter of not crashing the kernel when someone
> > > tries to force modprobe of a proprietary module (where the checksums
> > > doesn't match) on a kernel that supports the markers. Not doing so
> > > causes the markers to try to find the marker-specific information in
> > > struct module which doesn't exist and OOPSes.
>
> But you have the wrong target: it is not proprietary modules that have
> this risk but those built out-of-tree without checksums. Maybe
> oopsing in this case is not so bad; or the check could just limit itself to
> FORCED_MODULE.
>

I guess that for this one I could have a :

if (!mod->taints & TAINT_FORCED_MODULE)
...


>
> > > Christoph's point of view is rather more drastic than mine : it's not
> > > interesting for the kernel community to help proprietary modules writers,
> > > so it's a good idea not to give them marker support. (I CC'ed him so he
> > > can clarify his position).
> > Right. I thought that was your collective opinion
>
> Another way of looking at this though is that by allowing/encouraging
> proprietary module writers to include markers, we and their users get
> new diagnostic capabilities. It constitutes a little bit of opening
> up, which IMO we should reward rather than punish.
>
>

This specific one is a kernel policy matter, and I personally don't
have a strong opinion about it. I agree that you raise a good counter
argument : it can be useful to proprietary modules users to be able to
extract tracing information from those modules to argue with their
vendors that their driver/hardware is broken (a tracer is _very_ useful
in that kind of situation). However, it is also useful to proprieraty
module writers who can benefit from the merged kernel/modules traces.
Do we want to give them this ability ? It would surely help writing
better proprieraty kernel modules. Do we want this, or rather prefer to
put more pressure on them so they open their code ?

I will let others fight in the mud on this one. :)

for this one, we could add, instead :

if (!mod->taints & (TAINT_FORCED_MODULE | TAINT_PROPRIETARY_MODULE))

Mathieu

> - FChE

--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

2008-01-23 15:01:54

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: CONFIG_MARKERS

* Mathieu Desnoyers ([email protected]) wrote:
> * Frank Ch. Eigler ([email protected]) wrote:
> > Hi -
> >
> > On Tue, Jan 22, 2008 at 11:17:40PM -0500, Jon Masters wrote:
> > > On Tue, 2008-01-22 at 22:10 -0500, Mathieu Desnoyers wrote:
> > > > [...]
> > > > > > Is this an attempt to not set a marker for proprietary modules? [...]
> > > > >
> > > > > I can't seem to find any discussion about this aspect. If this is the
> > > > > intent, it seems misguided to me. There may instead be a relationship
> > > > > to TAINT_FORCED_{RMMOD,MODULE}. Mathieu?
> >
> > > > On my part, its mostly a matter of not crashing the kernel when someone
> > > > tries to force modprobe of a proprietary module (where the checksums
> > > > doesn't match) on a kernel that supports the markers. Not doing so
> > > > causes the markers to try to find the marker-specific information in
> > > > struct module which doesn't exist and OOPSes.
> >
> > But you have the wrong target: it is not proprietary modules that have
> > this risk but those built out-of-tree without checksums. Maybe
> > oopsing in this case is not so bad; or the check could just limit itself to
> > FORCED_MODULE.
> >
>
> I guess that for this one I could have a :
>
> if (!mod->taints & TAINT_FORCED_MODULE)
> ...
>

as one could notice: missing parenthesis
if (!(mod->taints & TAINT_FORCED_MODULE))

>
> >
> > > > Christoph's point of view is rather more drastic than mine : it's not
> > > > interesting for the kernel community to help proprietary modules writers,
> > > > so it's a good idea not to give them marker support. (I CC'ed him so he
> > > > can clarify his position).
> > > Right. I thought that was your collective opinion
> >
> > Another way of looking at this though is that by allowing/encouraging
> > proprietary module writers to include markers, we and their users get
> > new diagnostic capabilities. It constitutes a little bit of opening
> > up, which IMO we should reward rather than punish.
> >
> >
>
> This specific one is a kernel policy matter, and I personally don't
> have a strong opinion about it. I agree that you raise a good counter
> argument : it can be useful to proprietary modules users to be able to
> extract tracing information from those modules to argue with their
> vendors that their driver/hardware is broken (a tracer is _very_ useful
> in that kind of situation). However, it is also useful to proprieraty
> module writers who can benefit from the merged kernel/modules traces.
> Do we want to give them this ability ? It would surely help writing
> better proprieraty kernel modules. Do we want this, or rather prefer to
> put more pressure on them so they open their code ?
>
> I will let others fight in the mud on this one. :)
>
> for this one, we could add, instead :
>
> if (!mod->taints & (TAINT_FORCED_MODULE | TAINT_PROPRIETARY_MODULE))
>

here too
if (!(mod->taints & (TAINT_FORCED_MODULE | TAINT_PROPRIETARY_MODULE)))

Which remembers me to never write code before my first coffee in the
morning ;)

Mathieu

> Mathieu
>
> > - FChE
>
> --
> Mathieu Desnoyers
> Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

2008-01-23 16:34:30

by Jon Masters

[permalink] [raw]
Subject: Re: CONFIG_MARKERS

On Wed, 2008-01-23 at 10:01 -0500, Mathieu Desnoyers wrote:

> if (!(mod->taints & (TAINT_FORCED_MODULE | TAINT_PROPRIETARY_MODULE)))
>
> Which remembers me to never write code before my first coffee in the
> morning ;)

I switched to decaf and joined a gym...for someone who used to have 23
shots of espresso some days, it's a change.

But more importantly, are you going to take care of patching this up, or
shall I send something out? It's up to you.

Jon.

2008-01-23 17:11:53

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: CONFIG_MARKERS

* Jon Masters ([email protected]) wrote:
> On Wed, 2008-01-23 at 10:01 -0500, Mathieu Desnoyers wrote:
>
> > if (!(mod->taints & (TAINT_FORCED_MODULE | TAINT_PROPRIETARY_MODULE)))
> >
> > Which remembers me to never write code before my first coffee in the
> > morning ;)
>
> I switched to decaf and joined a gym...for someone who used to have 23
> shots of espresso some days, it's a change.
>

I only have two a day, in the morning and exercise regularly. But still,
coding with only one eye opened is not recommended. :)

> But more importantly, are you going to take care of patching this up, or
> shall I send something out? It's up to you.
>

I would prefer to wait until we have some clear statement on this issue
before I submit a patch (allowing markers in proprietary modules or
not).

As soon as we have an answer, I'll cook the one-liner.

Mathieu

> Jon.
>
>

--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

2008-01-24 05:27:23

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: CONFIG_MARKERS

On Wed, 23 Jan 2008 09:48:12 EST, Mathieu Desnoyers said:

> This specific one is a kernel policy matter, and I personally don't
> have a strong opinion about it. I agree that you raise a good counter
> argument : it can be useful to proprietary modules users to be able to
> extract tracing information from those modules to argue with their
> vendors that their driver/hardware is broken (a tracer is _very_ useful
> in that kind of situation).

Amen, brother. Been there, done that, got the tshirt (not on Linux, but
other operating systems).

> However, it is also useful to proprieraty
> module writers who can benefit from the merged kernel/modules traces.
> Do we want to give them this ability ?

The proprietary module writer has the *source* for the kernel and their module.
There's no way you can prevent the proprietary module writers from using this
feature as long as you allow other module writers to use it.

> It would surely help writing
> better proprieraty kernel modules.

The biggest complaint against proprietary modules is that they make it
impossible for *us* to debug. And you want to argue *against* a feature that
would allow them to develop better code that causes less crashes, and therefor
less people *asking* for us to debug it?

Remember - when a user tries a Linux box with a proprietary module, and the
experience sucks because the module sucks, they will walk away thinking
"Linux sucks", not "That module sucks".


Attachments:
(No filename) (226.00 B)

2008-01-24 06:20:11

by Jon Masters

[permalink] [raw]
Subject: Re: CONFIG_MARKERS

On Thu, 2008-01-24 at 00:25 -0500, [email protected] wrote:

> Remember - when a user tries a Linux box with a proprietary module, and the
> experience sucks because the module sucks, they will walk away thinking
> "Linux sucks", not "That module sucks".

Worse, if they're technically inclined, they'll think Linux sucks for
encoding philosophical policy into the kernel. Remember, a proprietary
driver is only "illegal" to distribute, it's not an infringement for
someone to write a non-GPL driver, or to have one on their computer.

Jon.

2008-01-24 12:52:21

by Mathieu Desnoyers

[permalink] [raw]
Subject: [PATCH] Linux Kernel Markers Support for Proprierary Modules

There seems to be good arguments for markers to support proprierary modules. So
I am throwing this one-liner in and let's see how people react. It only makes
sure that a module that has been "forced" to be loaded won't have its markers
used. It is important to leave this check to make sure the kernel does not crash
by expecting the markers part of the struct module by mistake in the case there
is an incorrect checksum.

It applies fine on 2.6.24-rc8-git3.

Discussion so far :
http://lkml.org/lkml/2008/1/22/226

Jon Masters <[email protected]> writes:
I notice in module.c:

#ifdef CONFIG_MARKERS
if (!mod->taints)
marker_update_probe_range(mod->markers,
mod->markers + mod->num_markers, NULL, NULL);
#endif

Is this an attempt to not set a marker for proprietary modules? [...]

* Frank Ch. Eigler ([email protected]) wrote:
I can't seem to find any discussion about this aspect. If this is the
intent, it seems misguided to me. There may instead be a relationship
to TAINT_FORCED_{RMMOD,MODULE}. Mathieu?

- FChE

On Tue, 2008-01-22 at 22:10 -0500, Mathieu Desnoyers wrote:
On my part, its mostly a matter of not crashing the kernel when someone
tries to force modprobe of a proprietary module (where the checksums
doesn't match) on a kernel that supports the markers. Not doing so
causes the markers to try to find the marker-specific information in
struct module which doesn't exist and OOPSes.

Christoph's point of view is rather more drastic than mine : it's not
interesting for the kernel community to help proprietary modules writers,
so it's a good idea not to give them marker support. (I CC'ed him so he
can clarify his position).

* Frank Ch. Eigler ([email protected]) wrote:
[...]
Another way of looking at this though is that by allowing/encouraging
proprietary module writers to include markers, we and their users get
new diagnostic capabilities. It constitutes a little bit of opening
up, which IMO we should reward rather than punish.


* Vladis Ketnieks ([email protected]) wrote:
On Wed, 23 Jan 2008 09:48:12 EST, Mathieu Desnoyers said:

> This specific one is a kernel policy matter, and I personally don't
> have a strong opinion about it. I agree that you raise a good counter
> argument : it can be useful to proprietary modules users to be able to
> extract tracing information from those modules to argue with their
> vendors that their driver/hardware is broken (a tracer is _very_ useful
> in that kind of situation).

Amen, brother. Been there, done that, got the tshirt (not on Linux, but
other operating systems).

> However, it is also useful to proprieraty
> module writers who can benefit from the merged kernel/modules traces.
> Do we want to give them this ability ?

The proprietary module writer has the *source* for the kernel and their module.
There's no way you can prevent the proprietary module writers from using this
feature as long as you allow other module writers to use it.

> It would surely help writing
> better proprieraty kernel modules.

The biggest complaint against proprietary modules is that they make it
impossible for *us* to debug. And you want to argue *against* a feature that
would allow them to develop better code that causes less crashes, and therefor
less people *asking* for us to debug it?

Remember - when a user tries a Linux box with a proprietary module, and the
experience sucks because the module sucks, they will walk away thinking
"Linux sucks", not "That module sucks".

Signed-off-by: Mathieu Desnoyers <[email protected]>
CC: "Frank Ch. Eigler" <[email protected]>
CC: Jon Masters <[email protected]>
CC: Rusty Russell <[email protected]>
CC: Christoph Hellwig <[email protected]>
CC: Linus Torvalds <[email protected]>
CC: Andrew Morton <[email protected]>
---
kernel/module.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6-lttng/kernel/module.c
===================================================================
--- linux-2.6-lttng.orig/kernel/module.c 2008-01-24 07:29:10.000000000 -0500
+++ linux-2.6-lttng/kernel/module.c 2008-01-24 07:37:56.000000000 -0500
@@ -1992,7 +1992,7 @@ static struct module *load_module(void _
add_kallsyms(mod, sechdrs, symindex, strindex, secstrings);

#ifdef CONFIG_MARKERS
- if (!mod->taints)
+ if (!(mod->taints & TAINT_FORCED_MODULE))
marker_update_probe_range(mod->markers,
mod->markers + mod->num_markers, NULL, NULL);
#endif
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

2008-01-24 18:28:48

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules

On Thu, 24 Jan 2008 07:47:04 EST, Mathieu Desnoyers said:
> I am throwing this one-liner in and let's see how people react. It only makes
> sure that a module that has been "forced" to be loaded won't have its markers
> used. It is important to leave this check to make sure the kernel does not crash
> by expecting the markers part of the struct module by mistake in the case there
> is an incorrect checksum.

I can live with that - if anything, a force-loaded GPL module deserves to lose
even more than a non-GPL module built against the current kernel. Quite
frankly, given that one of the reasons given for not liking closed modules is
"it's not maintainable", you'd *expect* that the infrastructure for allowing
a force-load of a module would have been thrown out entirely - is there anything
more unmaintainable than a module you *know* was built against different headers
and thus is using the wrong offsets for things?


Attachments:
(No filename) (226.00 B)

2008-01-24 20:45:41

by Jon Masters

[permalink] [raw]
Subject: Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules


On Thu, 2008-01-24 at 07:47 -0500, Mathieu Desnoyers wrote:
> There seems to be good arguments for markers to support proprierary modules. So
> I am throwing this one-liner in and let's see how people react. It only makes
> sure that a module that has been "forced" to be loaded won't have its markers
> used. It is important to leave this check to make sure the kernel does not crash
> by expecting the markers part of the struct module by mistake in the case there
> is an incorrect checksum.
>
> It applies fine on 2.6.24-rc8-git3.

I think this should go in.

Signed-off-by: Jon Masters <[email protected]>

Jon.

P.S. wondering out loud to myself, I finally realized the reason we need
a leaf struct_module function in kernel/module.c. We don't necessarily
have anything else checking for changes to struct module on module load
without this, and we have an embedded struct module in each module that
we memory map as we load the module. I did wonder what was protecting us
from that (especially forced loads). But Rusty does think of everything.

2008-01-25 01:29:39

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules

On Thursday 24 January 2008 23:47:04 Mathieu Desnoyers wrote:
> There seems to be good arguments for markers to support proprierary
> modules.

My attitude to this is simple: who cares about proprietary modules?

The current test is wrong, it should be checking for forced module loads
(which may not have marker information and may well crash the kernel). Of
course, all forced module loads can crash the kernel, but this is pretty
certain and it's simply to avoid.

We should just flat-out refuse to load a module with a module section too
small. That will cover the majority of this case anyway (*and* the
non-kallsysms case), and then we can remove this test altogether.

Cheers,
Rusty.

2008-01-25 08:02:24

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules


On Jan 24 2008 07:47, Mathieu Desnoyers wrote:
>On Tue, 2008-01-22 at 22:10 -0500, Mathieu Desnoyers wrote:
>On my part, its mostly a matter of not crashing the kernel when someone
>tries to force modprobe of a proprietary module (where the checksums
>doesn't match) on a kernel that supports the markers. Not doing so
>causes the markers to try to find the marker-specific information in
>struct module which doesn't exist and OOPSes.

>* Frank Ch. Eigler ([email protected]) wrote:
>[...]
>Another way of looking at this though is that by allowing/encouraging
>proprietary module writers to include markers, we and their users get
>new diagnostic capabilities. It constitutes a little bit of opening
>up, which IMO we should reward rather than punish.


Tackling this from a different angle:

I do not think there is a real reason to forceload a module, even
those with proprietary origin (vmware) or that are of
partially-closed nature (nvidia). vmware source is fully available,
so can be compiled with proper modinfo/vermagic/markers; nvidia uses
a build system trick to include an .o blob, but eventually its .ko
also ends up with a correct modinfo/vermagic.

Forceload is for people which like to trade an unstable system for
not having to install gcc and kernel-source.


>Remember - when a user tries a Linux box with a proprietary module, and the
>experience sucks because the module sucks, they will walk away thinking
>"Linux sucks", not "That module sucks".

So what is needed is an Oops with an explaining message
if (kernel_tainted) "blame that proprietary module first",
and make sure the user sees that oops even if in X.

2008-01-25 08:42:47

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules

On Fri, 25 Jan 2008 08:56:09 +0100, Jan Engelhardt said:
> So what is needed is an Oops with an explaining message
> if (kernel_tainted) "blame that proprietary module first",
> and make sure the user sees that oops even if in X.

The person who solves the "even if in X" problem will probably be nominated
for sainthood by the Linux community. "It just hung with flashing LED's" is
just too common an event....


Attachments:
(No filename) (226.00 B)

2008-01-25 15:32:20

by Jon Masters

[permalink] [raw]
Subject: Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules

On Fri, 2008-01-25 at 08:56 +0100, Jan Engelhardt wrote:

> So what is needed is an Oops with an explaining message
> if (kernel_tainted) "blame that proprietary module first",
> and make sure the user sees that oops even if in X.

The former is actually trivially doable with the module->taints bits. We
could have the equivalent of a neon flashing "blame this module" sign.

I also agree, we should stop force loading. Incompatible struct module,
etc. are really bad things to have mapped into a running kernel.

Jon.

2008-01-25 15:37:19

by Alan

[permalink] [raw]
Subject: Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules

On Fri, 25 Jan 2008 03:03:03 -0500
[email protected] wrote:

> On Fri, 25 Jan 2008 08:56:09 +0100, Jan Engelhardt said:
> > So what is needed is an Oops with an explaining message
> > if (kernel_tainted) "blame that proprietary module first",
> > and make sure the user sees that oops even if in X.
>
> The person who solves the "even if in X" problem will probably be nominated
> for sainthood by the Linux community. "It just hung with flashing LED's" is
> just too common an event....

I've been poking at it a bit but it will need X server help, or
eventually the kernel mode switch code.

Alan

2008-01-25 16:01:37

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules


On Jan 25 2008 10:31, Jon Masters wrote:
>On Fri, 2008-01-25 at 08:56 +0100, Jan Engelhardt wrote:
>
>> So what is needed is an Oops with an explaining message
>> if (kernel_tainted) "blame that proprietary module first",
>> and make sure the user sees that oops even if in X.
>
>The former is actually trivially doable with the module->taints bits. We
>could have the equivalent of a neon flashing "blame this module" sign.
>
>I also agree, we should stop force loading. Incompatible struct module,
>etc. are really bad things to have mapped into a running kernel.

Forceloading should be reserved for developers who know
when a symversion change is safe (which is rare in itself,
but still).

2008-01-26 03:28:33

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules

On Saturday 26 January 2008 02:31:30 Jon Masters wrote:
> On Fri, 2008-01-25 at 08:56 +0100, Jan Engelhardt wrote:
> > So what is needed is an Oops with an explaining message
> > if (kernel_tainted) "blame that proprietary module first",
> > and make sure the user sees that oops even if in X.
>
> The former is actually trivially doable with the module->taints bits. We
> could have the equivalent of a neon flashing "blame this module" sign.
>
> I also agree, we should stop force loading. Incompatible struct module,
> etc. are really bad things to have mapped into a running kernel.

I think there are two things here:
1) Currently we allow modules with no kallsyms info to be loaded into a
KALLSYMS kernel (and taint). A new option is needed to control this:
CONFIG_ACCEPT_NO_KALLSYMS under KERNEL_DEBUG which allows loading of
such "stripped" modules (a-la modprobe --force).

2) Unconditionally reject modules with a wrong module section size. Currently
we have no such check, which means without KALLSYMS, anything goes.

Thoughts?
Rusty.

2008-01-26 04:22:23

by Jon Masters

[permalink] [raw]
Subject: Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules

On Sat, 2008-01-26 at 14:27 +1100, Rusty Russell wrote:

> 2) Unconditionally reject modules with a wrong module section size. Currently
> we have no such check, which means without KALLSYMS, anything goes.

I favor the latter, since it's safest.

Jon.

2008-01-27 10:48:44

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules


On Jan 25 2008 23:21, Jon Masters wrote:
>On Sat, 2008-01-26 at 14:27 +1100, Rusty Russell wrote:
>
>> 2) Unconditionally reject modules with a wrong module section size. Currently
>> we have no such check, which means without KALLSYMS, anything goes.
>
>I favor the latter, since it's safest.

Is it possible to keep allowing the case where {section size is the
same and only the symversions are different}?