RFC: virtualbox tainting.

RFC: virtualbox tainting.

[Dave Jones]

The number of bug reports we get from people with virtualbox loaded are
truly astonishing.  It's GPL, but sadly that doesn't mean it's good.
Nearly all of these bugs look like random corruption. (corrupt linked lists,
corrupt page tables, and just plain 'weird' crashes).

This diff adds tainting to the module loader to treat it as we do with stuff
from staging/ (crap). With this tainting in place, automatic bug filing tools
can opt out of automatically filing kernel bugs, and inform the user to file
bugs somewhere more appropriate.

Signed-off-by: Dave Jones <davej@redhat.com>

diff --git a/kernel/module.c b/kernel/module.c
index 04379f92..d26c9a3 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2653,6 +2653,10 @@ static int check_module_license_and_versions(struct module *mod)
 	if (strcmp(mod->name, "ndiswrapper") == 0)
 		add_taint(TAINT_PROPRIETARY_MODULE);
 
+	/* vbox is garbage. */
+	if (strcmp(mod->name, "vboxdrv") == 0)
+		add_taint(TAINT_CRAP);
+
 	/* driverloader was caught wrongly pretending to be under GPL */
 	if (strcmp(mod->name, "driverloader") == 0)
 		add_taint_module(mod, TAINT_PROPRIETARY_MODULE);

Re: RFC: virtualbox tainting.

[Greg KH]

On Thu, Oct 06, 2011 at 03:05:27PM -0400, Dave Jones wrote:
> The number of bug reports we get from people with virtualbox loaded are
> truly astonishing.  It's GPL, but sadly that doesn't mean it's good.
> Nearly all of these bugs look like random corruption. (corrupt linked lists,
> corrupt page tables, and just plain 'weird' crashes).
> 
> This diff adds tainting to the module loader to treat it as we do with stuff
> from staging/ (crap). With this tainting in place, automatic bug filing tools
> can opt out of automatically filing kernel bugs, and inform the user to file
> bugs somewhere more appropriate.
> 
> Signed-off-by: Dave Jones <davej@redhat.com>
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 04379f92..d26c9a3 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2653,6 +2653,10 @@ static int check_module_license_and_versions(struct module *mod)
>  	if (strcmp(mod->name, "ndiswrapper") == 0)
>  		add_taint(TAINT_PROPRIETARY_MODULE);
>  
> +	/* vbox is garbage. */
> +	if (strcmp(mod->name, "vboxdrv") == 0)
> +		add_taint(TAINT_CRAP);

Odds are we should have a list of these types of modules, as I think
there are a few others out there we should mark this way :)

But anyway, I like this, and I think I'll go add it to the openSUSE
kernels so we can avoid the numerous bug reports we get there as well
for this crappy driver.

	Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

greg k-h

Re: RFC: virtualbox tainting.

[Dave Jones]

On Thu, Oct 06, 2011 at 12:44:32PM -0700, Greg Kroah-Hartman wrote:
 
 > > diff --git a/kernel/module.c b/kernel/module.c
 > > index 04379f92..d26c9a3 100644
 > > --- a/kernel/module.c
 > > +++ b/kernel/module.c
 > > @@ -2653,6 +2653,10 @@ static int check_module_license_and_versions(struct module *mod)
 > >  	if (strcmp(mod->name, "ndiswrapper") == 0)
 > >  		add_taint(TAINT_PROPRIETARY_MODULE);
 > >  
 > > +	/* vbox is garbage. */
 > > +	if (strcmp(mod->name, "vboxdrv") == 0)
 > > +		add_taint(TAINT_CRAP);
 > 
 > Odds are we should have a list of these types of modules, as I think
 > there are a few others out there we should mark this way :)

possibly, though this is by far the leader (for us at least) today.

 > But anyway, I like this, and I think I'll go add it to the openSUSE
 > kernels so we can avoid the numerous bug reports we get there as well
 > for this crappy driver.
 > 
 > 	Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

I feel a bit dirty overloading TAINT_CRAP (even if the name is apropos).
Should I introduce a TAINT_OUT_OF_TREE perhaps instead ?

	Dave

Re: RFC: virtualbox tainting.

[Valdis.Kletnieks]

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

On Thu, 06 Oct 2011 15:50:07 EDT, Dave Jones said:

> I feel a bit dirty overloading TAINT_CRAP (even if the name is apropos).
> Should I introduce a TAINT_OUT_OF_TREE perhaps instead ?

That would fix my biggest issue with the patch - it's a slippery slope.  But if
we bite the bullet and go straight to the bottom of the slope with an
OUT_OF_TREE taint, I'd be happy with that.

Is there a good way to identify at modprobe time if a module is in-tree or
out-of-tree?


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

Re: RFC: virtualbox tainting.

[Dave Jones]

On Thu, Oct 06, 2011 at 03:53:42PM -0400, Valdis.Kletnieks@vt.edu wrote:
 > On Thu, 06 Oct 2011 15:50:07 EDT, Dave Jones said:
 > 
 > > I feel a bit dirty overloading TAINT_CRAP (even if the name is apropos).
 > > Should I introduce a TAINT_OUT_OF_TREE perhaps instead ?
 > 
 > That would fix my biggest issue with the patch - it's a slippery slope.  But if
 > we bite the bullet and go straight to the bottom of the slope with an
 > OUT_OF_TREE taint, I'd be happy with that.
 > 
 > Is there a good way to identify at modprobe time if a module is in-tree or
 > out-of-tree?

Not easily from within the kernel. (and doing it in userspace is pointless,
as out of tree modules could just add themselves to the list of 'clean' modules)

We actually used to do something in RHEL a few years ago where we gpg signed the
modules we were building, for pretty much this reason. (also useful for spotting when
for eg, an IHV replaces ext3.ko with their own version in their preloads).

A bit overkill though compared to just having a hardcoded list of offenders in the kernel.

	Dave

Re: RFC: virtualbox tainting.

[Greg KH]

On Thu, Oct 06, 2011 at 03:53:42PM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Thu, 06 Oct 2011 15:50:07 EDT, Dave Jones said:
> 
> > I feel a bit dirty overloading TAINT_CRAP (even if the name is apropos).
> > Should I introduce a TAINT_OUT_OF_TREE perhaps instead ?
> 
> That would fix my biggest issue with the patch - it's a slippery slope.  But if
> we bite the bullet and go straight to the bottom of the slope with an
> OUT_OF_TREE taint, I'd be happy with that.
> 
> Is there a good way to identify at modprobe time if a module is in-tree or
> out-of-tree?

See my previous email, yes, we can do this.

greg k-h

Re: RFC: virtualbox tainting.

[Greg KH]

On Thu, Oct 06, 2011 at 03:50:07PM -0400, Dave Jones wrote:
> On Thu, Oct 06, 2011 at 12:44:32PM -0700, Greg Kroah-Hartman wrote:
>  
>  > > diff --git a/kernel/module.c b/kernel/module.c
>  > > index 04379f92..d26c9a3 100644
>  > > --- a/kernel/module.c
>  > > +++ b/kernel/module.c
>  > > @@ -2653,6 +2653,10 @@ static int check_module_license_and_versions(struct module *mod)
>  > >  	if (strcmp(mod->name, "ndiswrapper") == 0)
>  > >  		add_taint(TAINT_PROPRIETARY_MODULE);
>  > >  
>  > > +	/* vbox is garbage. */
>  > > +	if (strcmp(mod->name, "vboxdrv") == 0)
>  > > +		add_taint(TAINT_CRAP);
>  > 
>  > Odds are we should have a list of these types of modules, as I think
>  > there are a few others out there we should mark this way :)
> 
> possibly, though this is by far the leader (for us at least) today.
> 
>  > But anyway, I like this, and I think I'll go add it to the openSUSE
>  > kernels so we can avoid the numerous bug reports we get there as well
>  > for this crappy driver.
>  > 
>  > 	Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> 
> I feel a bit dirty overloading TAINT_CRAP (even if the name is apropos).
> Should I introduce a TAINT_OUT_OF_TREE perhaps instead ?

We could do that in a "generic" way by setting a "in-tree" flag type
thing for everything that is built from within the kernel build, and
then taint if that flag is not found.

We have that type of logic already in the SUSE enterprise kernels, would
that be something that would make more sense to do here?  That way you
could just ignore any TAINT_OUT_OF_TREE messages which would cover all
of the other modules I was complaining about above?

If so, I could try to make up a patch doing this if you want.

greg k-h

Re: RFC: virtualbox tainting.

[Dave Jones]

On Thu, Oct 06, 2011 at 12:58:24PM -0700, Greg Kroah-Hartman wrote:

 > > I feel a bit dirty overloading TAINT_CRAP (even if the name is apropos).
 > > Should I introduce a TAINT_OUT_OF_TREE perhaps instead ?
 > 
 > We could do that in a "generic" way by setting a "in-tree" flag type
 > thing for everything that is built from within the kernel build, and
 > then taint if that flag is not found.

what stops an out of tree module from setting that flag ?
I guess I need to see the implementation to understand this. 

	Dave 

Re: RFC: virtualbox tainting.

[Greg KH]

On Thu, Oct 06, 2011 at 04:04:40PM -0400, Dave Jones wrote:
> On Thu, Oct 06, 2011 at 12:58:24PM -0700, Greg Kroah-Hartman wrote:
> 
>  > > I feel a bit dirty overloading TAINT_CRAP (even if the name is apropos).
>  > > Should I introduce a TAINT_OUT_OF_TREE perhaps instead ?
>  > 
>  > We could do that in a "generic" way by setting a "in-tree" flag type
>  > thing for everything that is built from within the kernel build, and
>  > then taint if that flag is not found.
> 
> what stops an out of tree module from setting that flag ?
> I guess I need to see the implementation to understand this.

If an out of tree module wants to override this, they can do so pretty
easily, it's not the gpg-sign type thing that RHEL did.

So as it is pretty easy to get around, I supposed all out-of-tree
modules would eventually just figure out how to set it because people
would complain.  But then again, virtual box could just rename their
module, which would do the same thing here.

I guess it all depends on how "hard" we want to try to enforce this.

greg k-h

Re: RFC: virtualbox tainting.

[Jiri Kosina]

On Thu, 6 Oct 2011, Dave Jones wrote:

>  > > I feel a bit dirty overloading TAINT_CRAP (even if the name is apropos).
>  > > Should I introduce a TAINT_OUT_OF_TREE perhaps instead ?
>  > 
>  > We could do that in a "generic" way by setting a "in-tree" flag type
>  > thing for everything that is built from within the kernel build, and
>  > then taint if that flag is not found.
> 
> what stops an out of tree module from setting that flag ?

What stops virtualbox developers from renaming the module in every 
release? That's the same story.

The only fool-proof way of doing is indeed is the GPG-signing thing.

-- 
Jiri Kosina
SUSE Labs

Re: RFC: virtualbox tainting.

[Alan Cox]

> What stops virtualbox developers from renaming the module in every 
> release? That's the same story.
> 
> The only fool-proof way of doing is indeed is the GPG-signing thing.

If you want to get into a sophisticated fight with someone over hiding
the presence of a module then that's a pointless exercise.

If you want to just make it easier to sort and detect out of tree modules
then fine but the only actual pressure you have controlling its
effectiveness is going to be the embarrasment of a vendor who gets caught
out. GPG is thus I think over-engineering it somewhat.

Alan

Re: RFC: virtualbox tainting.

[Valdis.Kletnieks]

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

On Fri, 07 Oct 2011 10:01:32 +0200, Jiri Kosina said:
> On Thu, 6 Oct 2011, Dave Jones wrote:

> > what stops an out of tree module from setting that flag ?
> 
> What stops virtualbox developers from renaming the module in every 
> release? That's the same story.
> 
> The only fool-proof way of doing is indeed is the GPG-signing thing.

It would also be a second in-tree user for the kernel MPI library code that
somebody posted recently... Just sayin' ;)

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

Re: RFC: virtualbox tainting.

[Greg KH]

On Fri, Oct 07, 2011 at 12:04:05PM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Fri, 07 Oct 2011 10:01:32 +0200, Jiri Kosina said:
> > On Thu, 6 Oct 2011, Dave Jones wrote:
> 
> > > what stops an out of tree module from setting that flag ?
> > 
> > What stops virtualbox developers from renaming the module in every 
> > release? That's the same story.
> > 
> > The only fool-proof way of doing is indeed is the GPG-signing thing.
> 
> It would also be a second in-tree user for the kernel MPI library code that
> somebody posted recently... Just sayin' ;)

Ah, that is a good idea as some systems do want to gpg sign kernel
modules, I would love it if someone dug up the Red Hat patches, ported
it to the in-kernel mpi library (which I think will be in 3.2) and got
it merged.

Actually, anyone have any pointers to the Red Hat patch, I might
consider doing this myself as I wrote the first gpg signed kernel code
years ago and I would like to see this finally merged.

greg k-h

Re: RFC: virtualbox tainting.

[Dave Jones]

On Fri, Oct 07, 2011 at 10:01:23AM -0700, Greg Kroah-Hartman wrote:
 
 > > > The only fool-proof way of doing is indeed is the GPG-signing thing.
 > > 
 > > It would also be a second in-tree user for the kernel MPI library code that
 > > somebody posted recently... Just sayin' ;)
 > 
 > Ah, that is a good idea as some systems do want to gpg sign kernel
 > modules, I would love it if someone dug up the Red Hat patches, ported
 > it to the in-kernel mpi library (which I think will be in 3.2) and got
 > it merged.
 > 
 > Actually, anyone have any pointers to the Red Hat patch, I might
 > consider doing this myself as I wrote the first gpg signed kernel code
 > years ago and I would like to see this finally merged.

Dave Howells did the last work on the Red Hat variant iirc.
(added to cc)

	Dave

Re: RFC: virtualbox tainting.

[Bastian Blank]

On Thu, Oct 06, 2011 at 12:58:24PM -0700, Greg KH wrote:
> On Thu, Oct 06, 2011 at 03:50:07PM -0400, Dave Jones wrote:
> > I feel a bit dirty overloading TAINT_CRAP (even if the name is apropos).
> > Should I introduce a TAINT_OUT_OF_TREE perhaps instead ?
> We could do that in a "generic" way by setting a "in-tree" flag type
> thing for everything that is built from within the kernel build, and
> then taint if that flag is not found.

Debian uses such a patch[1] since some time.

Bastian

[1]:
http://anonscm.debian.org/viewvc/kernel/dists/trunk/linux-2.6/debian/patches/debian/module-bug-Add-TAINT_OOT_MODULE-flag.patch?revision=16914&view=markup
-- 
Only a fool fights in a burning house.
		-- Kank the Klingon, "Day of the Dove", stardate unknown

Re: RFC: virtualbox tainting.

[Frank Mehnert]

[-- Attachment #1: Type: Text/Plain, Size: 2599 bytes --]

Dave,

On Thursday 06 October 2011 21:05:27 Dave Jones wrote:
> The number of bug reports we get from people with virtualbox loaded are
> truly astonishing.  It's GPL, but sadly that doesn't mean it's good.
> Nearly all of these bugs look like random corruption. (corrupt linked
> lists, corrupt page tables, and just plain 'weird' crashes).
> 
> This diff adds tainting to the module loader to treat it as we do with
> stuff from staging/ (crap). With this tainting in place, automatic bug
> filing tools can opt out of automatically filing kernel bugs, and inform
> the user to file bugs somewhere more appropriate.
> 
> Signed-off-by: Dave Jones <davej@redhat.com>
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 04379f92..d26c9a3 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2653,6 +2653,10 @@ static int check_module_license_and_versions(struct
> module *mod) if (strcmp(mod->name, "ndiswrapper") == 0)
>  		add_taint(TAINT_PROPRIETARY_MODULE);
> 
> +	/* vbox is garbage. */
> +	if (strcmp(mod->name, "vboxdrv") == 0)
> +		add_taint(TAINT_CRAP);
> +
>  	/* driverloader was caught wrongly pretending to be under GPL */
>  	if (strcmp(mod->name, "driverloader") == 0)
>  		add_taint_module(mod, TAINT_PROPRIETARY_MODULE);

I can understand that you would rather ignore bug reports from external
kernel modules. On the other hand I don't like the TAINT_CRAP flag as
you can probably imagine.

Why not just mark external modules like Bastian Blank suggested? I can
assure that we will not try circumvent a TAINT_OOT_MODULE flag.

Please also note that we always have good relations to the open source
community so feel free to point me an archive where all these kernel
panic reports arrive which you've got. We fixed some bugs in our kernel
modules in the past and it is even possible that some of the current
bug reports are from older versions of VirtualBox which might have been
fixed in the meantime.

And of course, helpful and constructive critism is always appreciated.

Thanks,

Frank
-- 
Dr.-Ing. Frank Mehnert
Senior Manager Software Development Desktop Virtualization, VirtualBox
ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | 71384 Weinstadt, Germany

Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Jürgen Kunz, Marcel van de Molen, Alexander van der Ven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: RFC: virtualbox tainting.

[Dave Jones]

On Fri, Oct 07, 2011 at 04:40:53PM +0200, Frank Mehnert wrote:
 
 > I can understand that you would rather ignore bug reports from external
 > kernel modules. On the other hand I don't like the TAINT_CRAP flag as
 > you can probably imagine.
 > 
 > Why not just mark external modules like Bastian Blank suggested? I can
 > assure that we will not try circumvent a TAINT_OOT_MODULE flag.

Sure. The exact mechanism used doesn't matter to me.

 > Please also note that we always have good relations to the open source
 > community so feel free to point me an archive where all these kernel
 > panic reports arrive which you've got.

Search through the fedora kernel bugs at bugzilla.redhat.com for 'vboxdrv'
or 'virtual box' (note to search for closed bugs as well as open ones).

 > We fixed some bugs in our kernel
 > modules in the past and it is even possible that some of the current
 > bug reports are from older versions of VirtualBox which might have been
 > fixed in the meantime.
 > 
 > And of course, helpful and constructive critism is always appreciated.

If you'd like to be cc'd on those reports, I can add you to the bugs
when we go through them if you create a bugzilla account. 

	Dave

Re: RFC: virtualbox tainting.

[Frank Mehnert]

[-- Attachment #1: Type: Text/Plain, Size: 2491 bytes --]

On Friday 07 October 2011 17:10:04 Dave Jones wrote:
> On Fri, Oct 07, 2011 at 04:40:53PM +0200, Frank Mehnert wrote:
>  > I can understand that you would rather ignore bug reports from external
>  > kernel modules. On the other hand I don't like the TAINT_CRAP flag as
>  > you can probably imagine.
>  > 
>  > Why not just mark external modules like Bastian Blank suggested? I can
>  > assure that we will not try circumvent a TAINT_OOT_MODULE flag.
> 
> Sure. The exact mechanism used doesn't matter to me.
> 
>  > Please also note that we always have good relations to the open source
>  > community so feel free to point me an archive where all these kernel
>  > panic reports arrive which you've got.
> 
> Search through the fedora kernel bugs at bugzilla.redhat.com for 'vboxdrv'
> or 'virtual box' (note to search for closed bugs as well as open ones).

I will have a closer look on this list but at a first glance I saw a
lot of bugs containing a kernel log where the vboxdrv module was loaded
but the problem was clearly not related to VirtualBox. With a normal
installation, the vboxdrv module is loaded during startup of the host
but stays inactive as long as no VM is started.

It would make our life easier if we could have a list with bugs where you
guys have a comprehensible assumption that a bug is clearly related to
VirtualBox.

>  > We fixed some bugs in our kernel
>  > modules in the past and it is even possible that some of the current
>  > bug reports are from older versions of VirtualBox which might have been
>  > fixed in the meantime.
>  > 
>  > And of course, helpful and constructive critism is always appreciated.
> 
> If you'd like to be cc'd on those reports, I can add you to the bugs
> when we go through them if you create a bugzilla account.

Thanks, I would indeed appreciate that. I have already a Redhat bugtracker
account, same E-mail address as this one.

Kind regards,

Frank
-- 
Dr.-Ing. Frank Mehnert
Senior Manager Software Development Desktop Virtualization, VirtualBox
ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | 71384 Weinstadt, Germany

Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Jürgen Kunz, Marcel van de Molen, Alexander van der Ven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: RFC: virtualbox tainting.

[Josh Boyer]

On Fri, Oct 7, 2011 at 10:40 AM, Frank Mehnert <frank.mehnert@oracle.com> wrote:
> Dave,
> Please also note that we always have good relations to the open source
> community so feel free to point me an archive where all these kernel
> panic reports arrive which you've got. We fixed some bugs in our kernel
> modules in the past and it is even possible that some of the current
> bug reports are from older versions of VirtualBox which might have been
> fixed in the meantime.
>
> And of course, helpful and constructive critism is always appreciated.

In all seriousness, is there a reason these modules haven't been
submitted to the staging tree?  Perhaps this was attempted in the past
and I'm just not finding references to it.

josh

Re: RFC: virtualbox tainting.

[Valdis.Kletnieks]

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

On Fri, 07 Oct 2011 11:16:13 EDT, Josh Boyer said:
> In all seriousness, is there a reason these modules haven't been
> submitted to the staging tree?  Perhaps this was attempted in the past
> and I'm just not finding references to it.

I haven't actually checked what the churn rate of the kernel side of VirtualBox is, but
there *is* some benefit if you have a package that includes both userspace and kernel
code, to keep them in sync.

Currently, VirtualBox is at 4.1.4.  Now, if they decide they need to change/fix/extend
the API for some reason, they can just ship a 4.1.6 and be happy.  If the code is
in-tree, then they are basically stuck with the API - they can't ship a 4.1.6 that can
get away with assuming that all the kernel API is there.  So you have to carry around
workarounds and test-for-features code and all that stuff.

I don't know - maybe the Oracle crew think the API is stable enough now that they can
afford to do that.  But Frank would probably be the one to speak to how in/out of tree
would affect their development/support methodology.


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

Re: RFC: virtualbox tainting.

[Josh Boyer]

On Fri, Oct 7, 2011 at 12:22 PM,  <Valdis.Kletnieks@vt.edu> wrote:
> On Fri, 07 Oct 2011 11:16:13 EDT, Josh Boyer said:
>> In all seriousness, is there a reason these modules haven't been
>> submitted to the staging tree?  Perhaps this was attempted in the past
>> and I'm just not finding references to it.
>
> I haven't actually checked what the churn rate of the kernel side of VirtualBox is, but
> there *is* some benefit if you have a package that includes both userspace and kernel
> code, to keep them in sync.

I haven't checked either, which is why I asked Frank directly since
I'm assuming he has ;).

> Currently, VirtualBox is at 4.1.4.  Now, if they decide they need to change/fix/extend
> the API for some reason, they can just ship a 4.1.6 and be happy.  If the code is
> in-tree, then they are basically stuck with the API - they can't ship a 4.1.6 that can
> get away with assuming that all the kernel API is there.  So you have to carry around
> workarounds and test-for-features code and all that stuff.

Yeah, I'm familiar with that line of reasoning.  It can be beneficial
to vbox itself, but then we have situations like this where users are
picking up that single-source package and trying to run it on distros
that change kernel versions rather frequently and that doesn't seem to
benefit anyone.  The users are left with a buggy machine/vbox
instance, the distributions are left with unhappy users, and vbox is
left with unhappy users AND unhappy distribution maintainers.
(Somehow the distributions get the brunt of the blame from the users,
but I'm not going to go down that path.  I'm trying to be productive.)

> I don't know - maybe the Oracle crew think the API is stable enough now that they can
> afford to do that.  But Frank would probably be the one to speak to how in/out of tree
> would affect their development/support methodology.

Yes.  Which is why I asked Frank :).

josh

Re: RFC: virtualbox tainting.

[Valdis.Kletnieks]

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

On Fri, 07 Oct 2011 14:10:18 EDT, Josh Boyer said:

> (Somehow the distributions get the brunt of the blame from the users,
> but I'm not going to go down that path.  I'm trying to be productive.)

The problem is that stuff like kerneloops and the RedHat 'abrtd' daemon tend to
forward to the distro, not the third-party vendor.  But I'm suspecting that if
Dave Jones and Frank have a nice discussion and work something out, we'll *all*
be better off. I'd not be surprised if Dave has bug reports for stuff Frank
hasn't seen, because the reports go to RedHat and Oracle never got a copy before.


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

Re: RFC: virtualbox tainting.

[Pekka Enberg]

Hi Frank,

On Fri, Oct 7, 2011 at 5:40 PM, Frank Mehnert <frank.mehnert@oracle.com> wrote:
> Please also note that we always have good relations to the open source
> community so feel free to point me an archive where all these kernel
> panic reports arrive which you've got. We fixed some bugs in our kernel
> modules in the past and it is even possible that some of the current
> bug reports are from older versions of VirtualBox which might have been
> fixed in the meantime.
>
> And of course, helpful and constructive critism is always appreciated.

What is holding you back from submitting the vbox modules to kernel
staging tree and starting to clean it up for proper inclusion?

                        Pekka

Re: RFC: virtualbox tainting.

[Dave Airlie]

> What is holding you back from submitting the vbox modules to kernel
> staging tree and starting to clean it up for proper inclusion?

They enjoy the ability to change the userspace ABI at will and ship
everything in one place.

Dave.

Re: RFC: virtualbox tainting.

[Greg KH]

On Mon, Oct 10, 2011 at 10:58:48AM +0300, Pekka Enberg wrote:
> Hi Frank,
> 
> On Fri, Oct 7, 2011 at 5:40 PM, Frank Mehnert <frank.mehnert@oracle.com> wrote:
> > Please also note that we always have good relations to the open source
> > community so feel free to point me an archive where all these kernel
> > panic reports arrive which you've got. We fixed some bugs in our kernel
> > modules in the past and it is even possible that some of the current
> > bug reports are from older versions of VirtualBox which might have been
> > fixed in the meantime.
> >
> > And of course, helpful and constructive critism is always appreciated.
> 
> What is holding you back from submitting the vbox modules to kernel
> staging tree and starting to clean it up for proper inclusion?

You haven't actually read the vbox kernel code before, have you :)

Re: RFC: virtualbox tainting.

[Pekka Enberg]

On Mon, Oct 10, 2011 at 5:24 PM, Greg KH <greg@kroah.com> wrote:
>> > And of course, helpful and constructive critism is always appreciated.
>>
>> What is holding you back from submitting the vbox modules to kernel
>> staging tree and starting to clean it up for proper inclusion?
>
> You haven't actually read the vbox kernel code before, have you :)

I actually have and had a rough patch that added the code to staging
directory in the kernel tree.

Admittedly I don't remember anything about the code but surely it
meets the standards of what we already have there? :-)

                        Pekka

Re: RFC: virtualbox tainting.

[Dave Airlie]

> I actually have and had a rough patch that added the code to staging
> directory in the kernel tree.
>
> Admittedly I don't remember anything about the code but surely it
> meets the standards of what we already have there? :-)

unless upstream commits to some sort of stable ABI policy there is no
point, you'd just have to ship a module that somehow magically exposes
different probably incompatible interfaces depending on what userspace
was running on it.

Dave.

Re: RFC: virtualbox tainting.

[Pekka Enberg]

Hi Dave,

On Mon, Oct 10, 2011 at 7:49 PM, Dave Airlie <airlied@gmail.com> wrote:
>> I actually have and had a rough patch that added the code to staging
>> directory in the kernel tree.
>>
>> Admittedly I don't remember anything about the code but surely it
>> meets the standards of what we already have there? :-)
>
> unless upstream commits to some sort of stable ABI policy there is no
> point, you'd just have to ship a module that somehow magically exposes
> different probably incompatible interfaces depending on what userspace
> was running on it.

Absolutely. No disagreement here.

                        Pekka

Re: RFC: virtualbox tainting.

[Maarten Lankhorst]

Hey Dave,

On 10/06/2011 09:05 PM, Dave Jones wrote:
> The number of bug reports we get from people with virtualbox loaded are
> truly astonishing.  It's GPL, but sadly that doesn't mean it's good.
> Nearly all of these bugs look like random corruption. (corrupt linked lists,
> corrupt page tables, and just plain 'weird' crashes).
>
> This diff adds tainting to the module loader to treat it as we do with stuff
> from staging/ (crap). With this tainting in place, automatic bug filing tools
> can opt out of automatically filing kernel bugs, and inform the user to file
> bugs somewhere more appropriate.
>
> Signed-off-by: Dave Jones <davej@redhat.com>
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 04379f92..d26c9a3 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2653,6 +2653,10 @@ static int check_module_license_and_versions(struct module *mod)
>  	if (strcmp(mod->name, "ndiswrapper") == 0)
>  		add_taint(TAINT_PROPRIETARY_MODULE);
>  
> +	/* vbox is garbage. */
> +	if (strcmp(mod->name, "vboxdrv") == 0)
> +		add_taint(TAINT_CRAP);
> +
>  	/* driverloader was caught wrongly pretending to be under GPL */
>  	if (strcmp(mod->name, "driverloader") == 0)
>  		add_taint_module(mod, TAINT_PROPRIETARY_MODULE);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
Patch vbox' kernel modules instead?
Seems to mark it as crap, all you have to do is add this to the driver source:
MODULE_INFO(staging, "Y");

Tested this theory by adding that line to snd-usb-audio, it immediately tainted kernel. :)

Cheers,
Maarten

Re: RFC: virtualbox tainting.

[Josh Boyer]

On Mon, Oct 10, 2011 at 4:41 AM, Maarten Lankhorst
<m.b.lankhorst@gmail.com> wrote:
> Patch vbox' kernel modules instead?
> Seems to mark it as crap, all you have to do is add this to the driver source:
> MODULE_INFO(staging, "Y");

The distributions can't patch the user's vbox modules.  Unless they're
patched in the upstream vbox source, this really isn't going to work.

josh

Re: RFC: virtualbox tainting.

[Parag Warudkar]

On Thu, Oct 6, 2011 at 3:05 PM, Dave Jones <davej@redhat.com> wrote:
> The number of bug reports we get from people with virtualbox loaded are
> truly astonishing.  It's GPL, but sadly that doesn't mean it's good.
> Nearly all of these bugs look like random corruption. (corrupt linked lists,
> corrupt page tables, and just plain 'weird' crashes).
>
> This diff adds tainting to the module loader to treat it as we do with stuff
> from staging/ (crap). With this tainting in place, automatic bug filing tools
> can opt out of automatically filing kernel bugs, and inform the user to file
> bugs somewhere more appropriate.
>

Why can't the automatic bug filing tools have a user space list of
offending modules that it can use to opt out of automatically filing
kernel bugs? May be it can keep that list as part of a file in a
package that can be updated as and when more out of tree modules are
found. This can also be used for all out of tree modules - if module
is not part of standard kernel modules then opt out. I think this can
and should all be done in user space.

If people still report bugs manually may be have bugzilla ask them for
list of modules loaded and run a batch job to scan and close the ones
with blacklisted modules.

Parag

Re: RFC: virtualbox tainting.

[Alex Riesen]

On Wed, Oct 12, 2011 at 02:57, Parag Warudkar <parag.lkml@gmail.com> wrote:
> Why can't the automatic bug filing tools have a user space list of
> offending modules that it can use to opt out of automatically filing
> kernel bugs? ...

Maybe because an oops is not always conveniently accompanied by enough
information
to opt-out.

Re: RFC: virtualbox tainting.

[Parag Warudkar]

On Wed, Oct 12, 2011 at 3:55 AM, Alex Riesen <raa.lkml@gmail.com> wrote:
> On Wed, Oct 12, 2011 at 02:57, Parag Warudkar <parag.lkml@gmail.com> wrote:
>> Why can't the automatic bug filing tools have a user space list of
>> offending modules that it can use to opt out of automatically filing
>> kernel bugs? ...
>
> Maybe because an oops is not always conveniently accompanied by enough
> information
> to opt-out.
>

In which case the reporting of the OOPS is useless anyways
irrespective of whether or not vboxdrv was loaded. So the tool  can
opt out based on the fact that it cannot find enough info in the OOPS
- missing module list for example. But I don't think that's a very
likely scenario - oops is in most cases accompanied by tainted flag
and module list - the possibility of OOPS printing only tainted but
not module list is not very real and can be ignored.

Parag

Re: RFC: virtualbox tainting.

[Alex Riesen]

On Wed, Oct 12, 2011 at 16:08, Parag Warudkar <parag.lkml@gmail.com> wrote:
> On Wed, Oct 12, 2011 at 3:55 AM, Alex Riesen <raa.lkml@gmail.com> wrote:
>> On Wed, Oct 12, 2011 at 02:57, Parag Warudkar <parag.lkml@gmail.com> wrote:
>>> Why can't the automatic bug filing tools have a user space list of
>>> offending modules that it can use to opt out of automatically filing
>>> kernel bugs? ...
>>
>> Maybe because an oops is not always conveniently accompanied by enough
>> information
>> to opt-out.
>>
>
> In which case the reporting of the OOPS is useless anyways
> irrespective of whether or not vboxdrv was loaded. So the tool  can

Of course not. There is a lot of insight in the instruction dump,
which may be still present, or in stack trace.

Re: RFC: virtualbox tainting.

[Parag Warudkar]

On Wed, Oct 12, 2011 at 10:18 AM, Alex Riesen <raa.lkml@gmail.com> wrote:
> On Wed, Oct 12, 2011 at 16:08, Parag Warudkar <parag.lkml@gmail.com> wrote:
>> On Wed, Oct 12, 2011 at 3:55 AM, Alex Riesen <raa.lkml@gmail.com> wrote:
>>> On Wed, Oct 12, 2011 at 02:57, Parag Warudkar <parag.lkml@gmail.com> wrote:
>>>> Why can't the automatic bug filing tools have a user space list of
>>>> offending modules that it can use to opt out of automatically filing
>>>> kernel bugs? ...
>>>
>>> Maybe because an oops is not always conveniently accompanied by enough
>>> information
>>> to opt-out.
>>>
>>
>> In which case the reporting of the OOPS is useless anyways
>> irrespective of whether or not vboxdrv was loaded. So the tool  can
>
> Of course not. There is a lot of insight in the instruction dump,
> which may be still present, or in stack trace.
>

How likely is an OOPS to just have the useful instruction dump but no
other info?
I think the likelihood is in the same ballpark as the likelihood of
vboxdrv being loaded but OOPS being caused by something else.

In both cases we end up losing a OOPS or two - not a big deal IMHO.

Generally the tainted flag is followed by module list, followed by the
IP, Regs, stack trace and then instruction dump.
I would think that if it got around to printing the instruction dump
it would have already printed the module list which is enough for user
space tool to opt out.

Parag

Re: RFC: virtualbox tainting.

[Zan Lynx]

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

On Tue, 2011-10-11 at 20:57 -0400, Parag Warudkar wrote:

> If people still report bugs manually may be have bugzilla ask them for
> list of modules loaded and run a batch job to scan and close the ones
> with blacklisted modules.

Still report bugs manually? Fedora 15's abrt tool has /never/
successfully filed a bug for me. It actually is supposed to work? I am
surprised.

-- 
Knowledge Is Power
Power Corrupts
Study Hard
Be Evil

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: RFC: virtualbox tainting.

[Parag Warudkar]

On Wed, Oct 12, 2011 at 3:55 PM, Zan Lynx <zlynx@acm.org> wrote:
> On Tue, 2011-10-11 at 20:57 -0400, Parag Warudkar wrote:
>
>> If people still report bugs manually may be have bugzilla ask them for
>> list of modules loaded and run a batch job to scan and close the ones
>> with blacklisted modules.
>
> Still report bugs manually? Fedora 15's abrt tool has /never/
> successfully filed a bug for me. It actually is supposed to work? I am
> surprised.

That's another reason not to have this patch in the kernel ;)

I don't know about Fedora, but Ubuntu's bug reporter got a lot better
with 11.10 betas.
The other day it detected i915 GPU hang, collected all files and
successfully reported the bug!

The only catch - it did not properly capure the i915_error_state (all
zeros), which in fairness, might not be the tool's fault.
That and it might need to be made a little more subtle / less annoying.

Parag