LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* CONFIG_MARKERS
@ 2008-01-22 19:13 Jon Masters
  2008-01-23  3:00 ` CONFIG_MARKERS Frank Ch. Eigler
  0 siblings, 1 reply; 23+ messages in thread
From: Jon Masters @ 2008-01-22 19:13 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: Rusty Russell

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.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: CONFIG_MARKERS
  2008-01-22 19:13 CONFIG_MARKERS Jon Masters
@ 2008-01-23  3:00 ` Frank Ch. Eigler
  2008-01-23  3:10   ` CONFIG_MARKERS Mathieu Desnoyers
  0 siblings, 1 reply; 23+ messages in thread
From: Frank Ch. Eigler @ 2008-01-23  3:00 UTC (permalink / raw)
  To: mathieu.desnoyers; +Cc: Linux Kernel Mailing List, Rusty Russell, Jon Masters


Jon Masters <jcm@redhat.com> 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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: CONFIG_MARKERS
  2008-01-23  3:00 ` CONFIG_MARKERS Frank Ch. Eigler
@ 2008-01-23  3:10   ` Mathieu Desnoyers
  2008-01-23  4:17     ` CONFIG_MARKERS Jon Masters
  0 siblings, 1 reply; 23+ messages in thread
From: Mathieu Desnoyers @ 2008-01-23  3:10 UTC (permalink / raw)
  To: Frank Ch. Eigler
  Cc: Linux Kernel Mailing List, Rusty Russell, Jon Masters, Christoph Hellwig

* Frank Ch. Eigler (fche@redhat.com) wrote:
> 
> Jon Masters <jcm@redhat.com> 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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: CONFIG_MARKERS
  2008-01-23  3:10   ` CONFIG_MARKERS Mathieu Desnoyers
@ 2008-01-23  4:17     ` Jon Masters
  2008-01-23 13:14       ` CONFIG_MARKERS Frank Ch. Eigler
  0 siblings, 1 reply; 23+ messages in thread
From: Jon Masters @ 2008-01-23  4:17 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Frank Ch. Eigler, Linux Kernel Mailing List, Rusty Russell,
	Christoph Hellwig


On Tue, 2008-01-22 at 22:10 -0500, Mathieu Desnoyers wrote:
> * Frank Ch. Eigler (fche@redhat.com) wrote:
> > 
> > Jon Masters <jcm@redhat.com> 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.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: CONFIG_MARKERS
  2008-01-23  4:17     ` CONFIG_MARKERS Jon Masters
@ 2008-01-23 13:14       ` Frank Ch. Eigler
  2008-01-23 14:48         ` CONFIG_MARKERS Mathieu Desnoyers
  0 siblings, 1 reply; 23+ messages in thread
From: Frank Ch. Eigler @ 2008-01-23 13:14 UTC (permalink / raw)
  To: Jon Masters
  Cc: Mathieu Desnoyers, Frank Ch. Eigler, Linux Kernel Mailing List,
	Rusty Russell, Christoph Hellwig

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: CONFIG_MARKERS
  2008-01-23 13:14       ` CONFIG_MARKERS Frank Ch. Eigler
@ 2008-01-23 14:48         ` Mathieu Desnoyers
  2008-01-23 15:01           ` CONFIG_MARKERS Mathieu Desnoyers
  2008-01-24  5:25           ` CONFIG_MARKERS Valdis.Kletnieks
  0 siblings, 2 replies; 23+ messages in thread
From: Mathieu Desnoyers @ 2008-01-23 14:48 UTC (permalink / raw)
  To: Frank Ch. Eigler
  Cc: Jon Masters, Linux Kernel Mailing List, Rusty Russell,
	Christoph Hellwig, Linus Torvalds, Andrew Morton

* Frank Ch. Eigler (fche@redhat.com) 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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: CONFIG_MARKERS
  2008-01-23 14:48         ` CONFIG_MARKERS Mathieu Desnoyers
@ 2008-01-23 15:01           ` Mathieu Desnoyers
  2008-01-23 16:33             ` CONFIG_MARKERS Jon Masters
  2008-01-24  5:25           ` CONFIG_MARKERS Valdis.Kletnieks
  1 sibling, 1 reply; 23+ messages in thread
From: Mathieu Desnoyers @ 2008-01-23 15:01 UTC (permalink / raw)
  To: Frank Ch. Eigler
  Cc: Jon Masters, Linux Kernel Mailing List, Rusty Russell,
	Christoph Hellwig, Linus Torvalds, Andrew Morton

* Mathieu Desnoyers (mathieu.desnoyers@polymtl.ca) wrote:
> * Frank Ch. Eigler (fche@redhat.com) 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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: CONFIG_MARKERS
  2008-01-23 15:01           ` CONFIG_MARKERS Mathieu Desnoyers
@ 2008-01-23 16:33             ` Jon Masters
  2008-01-23 17:11               ` CONFIG_MARKERS Mathieu Desnoyers
  0 siblings, 1 reply; 23+ messages in thread
From: Jon Masters @ 2008-01-23 16:33 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Frank Ch. Eigler, Linux Kernel Mailing List, Rusty Russell,
	Christoph Hellwig, Linus Torvalds, Andrew Morton

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.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: CONFIG_MARKERS
  2008-01-23 16:33             ` CONFIG_MARKERS Jon Masters
@ 2008-01-23 17:11               ` Mathieu Desnoyers
  0 siblings, 0 replies; 23+ messages in thread
From: Mathieu Desnoyers @ 2008-01-23 17:11 UTC (permalink / raw)
  To: Jon Masters
  Cc: Frank Ch. Eigler, Linux Kernel Mailing List, Rusty Russell,
	Christoph Hellwig, Linus Torvalds, Andrew Morton

* Jon Masters (jonathan@jonmasters.org) 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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: CONFIG_MARKERS
  2008-01-23 14:48         ` CONFIG_MARKERS Mathieu Desnoyers
  2008-01-23 15:01           ` CONFIG_MARKERS Mathieu Desnoyers
@ 2008-01-24  5:25           ` Valdis.Kletnieks
  2008-01-24  6:19             ` CONFIG_MARKERS Jon Masters
  1 sibling, 1 reply; 23+ messages in thread
From: Valdis.Kletnieks @ 2008-01-24  5:25 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Frank Ch. Eigler, Jon Masters, Linux Kernel Mailing List,
	Rusty Russell, Christoph Hellwig, Linus Torvalds, Andrew Morton

[-- Attachment #1: Type: text/plain, Size: 1524 bytes --]

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".


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: CONFIG_MARKERS
  2008-01-24  5:25           ` CONFIG_MARKERS Valdis.Kletnieks
@ 2008-01-24  6:19             ` Jon Masters
  2008-01-24 12:47               ` [PATCH] Linux Kernel Markers Support for Proprierary Modules Mathieu Desnoyers
  0 siblings, 1 reply; 23+ messages in thread
From: Jon Masters @ 2008-01-24  6:19 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: Mathieu Desnoyers, Frank Ch. Eigler, Linux Kernel Mailing List,
	Rusty Russell, Christoph Hellwig, Linus Torvalds, Andrew Morton

On Thu, 2008-01-24 at 00:25 -0500, Valdis.Kletnieks@vt.edu 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.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH] Linux Kernel Markers Support for Proprierary Modules
  2008-01-24  6:19             ` CONFIG_MARKERS Jon Masters
@ 2008-01-24 12:47               ` Mathieu Desnoyers
  2008-01-24 18:27                 ` Valdis.Kletnieks
                                   ` (3 more replies)
  0 siblings, 4 replies; 23+ messages in thread
From: Mathieu Desnoyers @ 2008-01-24 12:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Valdis.Kletnieks, Frank Ch. Eigler, Linux Kernel Mailing List,
	Rusty Russell, Christoph Hellwig, Linus Torvalds, Jon Masters

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 <jcm@redhat.com> 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 (fche@redhat.com) 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 (fche@redhat.com) 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 (Valdis.Kletnieks@vt.edu) 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 <mathieu.desnoyers@polymtl.ca>
CC: "Frank Ch. Eigler" <fche@redhat.com>
CC: Jon Masters <jcm@redhat.com>
CC: Rusty Russell <rusty@rustcorp.com.au>
CC: Christoph Hellwig <hch@infradead.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
---
 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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules
  2008-01-24 12:47               ` [PATCH] Linux Kernel Markers Support for Proprierary Modules Mathieu Desnoyers
@ 2008-01-24 18:27                 ` Valdis.Kletnieks
  2008-01-24 20:35                 ` Jon Masters
                                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 23+ messages in thread
From: Valdis.Kletnieks @ 2008-01-24 18:27 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Andrew Morton, Frank Ch. Eigler, Linux Kernel Mailing List,
	Rusty Russell, Christoph Hellwig, Linus Torvalds, Jon Masters

[-- Attachment #1: Type: text/plain, Size: 929 bytes --]

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?

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules
  2008-01-24 12:47               ` [PATCH] Linux Kernel Markers Support for Proprierary Modules Mathieu Desnoyers
  2008-01-24 18:27                 ` Valdis.Kletnieks
@ 2008-01-24 20:35                 ` Jon Masters
  2008-01-25  1:27                 ` Rusty Russell
  2008-01-25  7:56                 ` Jan Engelhardt
  3 siblings, 0 replies; 23+ messages in thread
From: Jon Masters @ 2008-01-24 20:35 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Andrew Morton, Valdis.Kletnieks, Frank Ch. Eigler,
	Linux Kernel Mailing List, Rusty Russell, Christoph Hellwig,
	Linus Torvalds


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 <jcm@jonmasters.org>

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.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules
  2008-01-24 12:47               ` [PATCH] Linux Kernel Markers Support for Proprierary Modules Mathieu Desnoyers
  2008-01-24 18:27                 ` Valdis.Kletnieks
  2008-01-24 20:35                 ` Jon Masters
@ 2008-01-25  1:27                 ` Rusty Russell
  2008-01-25  7:56                 ` Jan Engelhardt
  3 siblings, 0 replies; 23+ messages in thread
From: Rusty Russell @ 2008-01-25  1:27 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Andrew Morton, Valdis.Kletnieks, Frank Ch. Eigler,
	Linux Kernel Mailing List, Christoph Hellwig, Linus Torvalds,
	Jon Masters

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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules
  2008-01-24 12:47               ` [PATCH] Linux Kernel Markers Support for Proprierary Modules Mathieu Desnoyers
                                   ` (2 preceding siblings ...)
  2008-01-25  1:27                 ` Rusty Russell
@ 2008-01-25  7:56                 ` Jan Engelhardt
  2008-01-25  8:03                   ` Valdis.Kletnieks
  2008-01-25 15:31                   ` Jon Masters
  3 siblings, 2 replies; 23+ messages in thread
From: Jan Engelhardt @ 2008-01-25  7:56 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Andrew Morton, Valdis.Kletnieks, Frank Ch. Eigler,
	Linux Kernel Mailing List, Rusty Russell, Christoph Hellwig,
	Linus Torvalds, Jon Masters


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 (fche@redhat.com) 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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules
  2008-01-25  7:56                 ` Jan Engelhardt
@ 2008-01-25  8:03                   ` Valdis.Kletnieks
  2008-01-25 16:32                     ` Alan Cox
  2008-01-25 15:31                   ` Jon Masters
  1 sibling, 1 reply; 23+ messages in thread
From: Valdis.Kletnieks @ 2008-01-25  8:03 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Mathieu Desnoyers, Andrew Morton, Frank Ch. Eigler,
	Linux Kernel Mailing List, Rusty Russell, Christoph Hellwig,
	Linus Torvalds, Jon Masters

[-- Attachment #1: Type: text/plain, Size: 414 bytes --]

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....


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules
  2008-01-25  7:56                 ` Jan Engelhardt
  2008-01-25  8:03                   ` Valdis.Kletnieks
@ 2008-01-25 15:31                   ` Jon Masters
  2008-01-25 16:01                     ` Jan Engelhardt
  2008-01-26  3:27                     ` Rusty Russell
  1 sibling, 2 replies; 23+ messages in thread
From: Jon Masters @ 2008-01-25 15:31 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Mathieu Desnoyers, Andrew Morton, Valdis.Kletnieks,
	Frank Ch. Eigler, Linux Kernel Mailing List, Rusty Russell,
	Christoph Hellwig, Linus Torvalds

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.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules
  2008-01-25 15:31                   ` Jon Masters
@ 2008-01-25 16:01                     ` Jan Engelhardt
  2008-01-26  3:27                     ` Rusty Russell
  1 sibling, 0 replies; 23+ messages in thread
From: Jan Engelhardt @ 2008-01-25 16:01 UTC (permalink / raw)
  To: Jon Masters
  Cc: Mathieu Desnoyers, Andrew Morton, Valdis.Kletnieks,
	Frank Ch. Eigler, Linux Kernel Mailing List, Rusty Russell,
	Christoph Hellwig, Linus Torvalds


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).

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules
  2008-01-25  8:03                   ` Valdis.Kletnieks
@ 2008-01-25 16:32                     ` Alan Cox
  0 siblings, 0 replies; 23+ messages in thread
From: Alan Cox @ 2008-01-25 16:32 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: Jan Engelhardt, Mathieu Desnoyers, Andrew Morton,
	Frank Ch. Eigler, Linux Kernel Mailing List, Rusty Russell,
	Christoph Hellwig, Linus Torvalds, Jon Masters

On Fri, 25 Jan 2008 03:03:03 -0500
Valdis.Kletnieks@vt.edu 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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules
  2008-01-25 15:31                   ` Jon Masters
  2008-01-25 16:01                     ` Jan Engelhardt
@ 2008-01-26  3:27                     ` Rusty Russell
  2008-01-26  4:21                       ` Jon Masters
  1 sibling, 1 reply; 23+ messages in thread
From: Rusty Russell @ 2008-01-26  3:27 UTC (permalink / raw)
  To: Jon Masters
  Cc: Jan Engelhardt, Mathieu Desnoyers, Andrew Morton,
	Valdis.Kletnieks, Frank Ch. Eigler, Linux Kernel Mailing List,
	Christoph Hellwig, Linus Torvalds

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.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules
  2008-01-26  3:27                     ` Rusty Russell
@ 2008-01-26  4:21                       ` Jon Masters
  2008-01-27 10:48                         ` Jan Engelhardt
  0 siblings, 1 reply; 23+ messages in thread
From: Jon Masters @ 2008-01-26  4:21 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Jan Engelhardt, Mathieu Desnoyers, Andrew Morton,
	Valdis.Kletnieks, Frank Ch. Eigler, Linux Kernel Mailing List,
	Christoph Hellwig, Linus Torvalds

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.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Linux Kernel Markers Support for Proprierary Modules
  2008-01-26  4:21                       ` Jon Masters
@ 2008-01-27 10:48                         ` Jan Engelhardt
  0 siblings, 0 replies; 23+ messages in thread
From: Jan Engelhardt @ 2008-01-27 10:48 UTC (permalink / raw)
  To: Jon Masters
  Cc: Rusty Russell, Mathieu Desnoyers, Andrew Morton,
	Valdis.Kletnieks, Frank Ch. Eigler, Linux Kernel Mailing List,
	Christoph Hellwig, Linus Torvalds


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}?

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2008-01-27 10:48 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-01-22 19:13 CONFIG_MARKERS Jon Masters
2008-01-23  3:00 ` CONFIG_MARKERS Frank Ch. Eigler
2008-01-23  3:10   ` CONFIG_MARKERS Mathieu Desnoyers
2008-01-23  4:17     ` CONFIG_MARKERS Jon Masters
2008-01-23 13:14       ` CONFIG_MARKERS Frank Ch. Eigler
2008-01-23 14:48         ` CONFIG_MARKERS Mathieu Desnoyers
2008-01-23 15:01           ` CONFIG_MARKERS Mathieu Desnoyers
2008-01-23 16:33             ` CONFIG_MARKERS Jon Masters
2008-01-23 17:11               ` CONFIG_MARKERS Mathieu Desnoyers
2008-01-24  5:25           ` CONFIG_MARKERS Valdis.Kletnieks
2008-01-24  6:19             ` CONFIG_MARKERS Jon Masters
2008-01-24 12:47               ` [PATCH] Linux Kernel Markers Support for Proprierary Modules Mathieu Desnoyers
2008-01-24 18:27                 ` Valdis.Kletnieks
2008-01-24 20:35                 ` Jon Masters
2008-01-25  1:27                 ` Rusty Russell
2008-01-25  7:56                 ` Jan Engelhardt
2008-01-25  8:03                   ` Valdis.Kletnieks
2008-01-25 16:32                     ` Alan Cox
2008-01-25 15:31                   ` Jon Masters
2008-01-25 16:01                     ` Jan Engelhardt
2008-01-26  3:27                     ` Rusty Russell
2008-01-26  4:21                       ` Jon Masters
2008-01-27 10:48                         ` Jan Engelhardt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).