Skip to contentSkip to conversation

RFC: virtualbox tainting.

PATCH Dave Jones 14 participants 37 messages Oct 6, 2011 – Oct 12, 2011

lore
Share & tools
lore

Dave Jones

author
#0 lore raw

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.

kernel/module.c +4 -0
index 04379f92..d26c9a3 100644
@@ -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);
Acked-by Greg Kroah-Hartman
#1 lore raw
Quoted 17 lines · 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.

kernel/module.c +0 -0
index 04379f92..d26c9a3 100644

kernel/module.c static int check_module_license_and_versions(struct module *mod) · line 2658

Quoted 6 lines
kernel/module.c +3 -0
@@ -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.

greg k-h

author
#2 lore raw
Quoted 8 lines · Greg Kroah-Hartman
Quoted 8 lines
kernel/module.c +0 -0
index 04379f92..d26c9a3 100644
@@ -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);
 
Quoted 6 lines

kernel/module.c static int check_module_license_and_versions(struct module *mod) · line 2658

Quoted 3 lines
kernel/module.c +3 -0
+	/* 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.

Quoted 5 lines

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.

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

#3 lore raw
Quoted 2 lines · Dave Jones

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?

author
#4 lore raw
Quoted 11 lines · On Thu, Oct 06, 2011 at 03:53:42PM -0400, [email protected] wrote:
Quoted 2 lines · Dave Jones

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

#5 lore raw
In reply to [email protected]
Quoted 11 lines · On Thu, Oct 06, 2011 at 03:53:42PM -0400, [email protected] wrote:
Quoted 2 lines · Dave Jones

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

#6 lore raw
In reply to Dave Jones
Quoted 19 lines · Dave Jones
Quoted 8 lines · Greg Kroah-Hartman
Quoted 8 lines
kernel/module.c +0 -0
index 04379f92..d26c9a3 100644
@@ -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);
 
Quoted 6 lines

kernel/module.c static int check_module_license_and_versions(struct module *mod) · line 2658

Quoted 3 lines
kernel/module.c +3 -0
+	/* 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.

Quoted 8 lines
Quoted 5 lines

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.

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

author
#7 lore raw
Quoted 6 lines · Greg Kroah-Hartman
Quoted 2 lines

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

#8 lore raw
Quoted 11 lines · Dave Jones
Quoted 6 lines · Greg Kroah-Hartman
Quoted 2 lines

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

#9 lore raw
In reply to Dave Jones
Quoted 8 lines · Dave Jones
Quoted 6 lines
Quoted 2 lines

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.

Signature · 2 lines
Jiri Kosina
SUSE Labs
#10 lore raw
Quoted 4 lines

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

#11 lore raw
In reply to Jiri Kosina
Quoted 8 lines · Jiri Kosina
Quoted 1 line · Dave Jones

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' ;)

#12 lore raw
Quoted 5 lines · On Fri, Oct 07, 2011 at 12:04:05PM -0400, [email protected] wrote:
Quoted 4 lines · Jiri Kosina
Quoted 1 line · Dave Jones

what stops an out of tree module from setting that flag ?

Quoted 7 lines
Quoted 4 lines

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

author
#13 lore raw
Quoted 5 lines · Greg Kroah-Hartman
Quoted 4 lines
Quoted 1 line

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' ;)

Quoted 8 lines

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

#14 lore raw
In reply to Greg KH
Quoted 6 lines · Greg KH
Quoted 2 lines · Dave Jones

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

Signature · 2 lines
Only a fool fights in a burning house.
		-- Kank the Klingon, "Day of the Dove", stardate unknown
#15 lore raw
In reply to Dave Jones

Dave,

Quoted 17 lines · 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.

kernel/module.c +0 -0
index 04379f92..d26c9a3 100644

kernel/module.c static int check_module_license_and_versions(struct · line 2662

Quoted 10 lines
kernel/module.c +4 -0
@@ -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

Signature · 11 lines
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
author
#16 lore raw
Quoted 6 lines · Frank Mehnert

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.

Quoted 3 lines

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

Quoted 6 lines

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

#17 lore raw
Quoted 8 lines · On Friday 07 October 2011 17:10:04 Dave Jones wrote:
Quoted 6 lines · Frank Mehnert

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.

Quoted 8 lines

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

Quoted 3 lines

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.

Quoted 9 lines
Quoted 6 lines

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

Signature · 11 lines
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
#18 lore raw
In reply to Frank Mehnert
Quoted 9 lines · Frank Mehnert

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

#19 lore raw
Quoted 3 lines · Josh Boyer

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.

#20 lore raw
Quoted 8 lines · On Fri, Oct 7, 2011 at 12:22 PM, <[email protected]> wrote:
Quoted 3 lines · Josh Boyer

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

Quoted 5 lines

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

Quoted 3 lines

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

#21 lore raw
Quoted 2 lines · Josh Boyer

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

#22 lore raw
In reply to Frank Mehnert

Hi Frank,

Quoted 8 lines · Frank Mehnert

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

#23 lore raw
Quoted 2 lines

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.

#24 lore raw
In reply to Pekka Enberg
Quoted 10 lines · Pekka Enberg

Hi Frank,

Quoted 7 lines · Frank Mehnert

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.

Quoted 4 lines
Quoted 1 line

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

#25 lore raw
Quoted 6 lines · Greg KH
Quoted 4 lines
Quoted 1 line

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

#26 lore raw
Quoted 5 lines

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.

#27 lore raw

Hi Dave,

Quoted 10 lines · Dave Airlie
Quoted 5 lines

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

#28 lore raw
In reply to Dave Jones

Hey Dave,

Quoted 28 lines · 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.

kernel/module.c +4 -1
index 04379f92..d26c9a3 100644
@@ -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);
--
Quoted 4 lines

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
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

#29 lore raw
Quoted 3 lines · Maarten Lankhorst

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

#30 lore raw
In reply to Dave Jones
Quoted 10 lines · 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.

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

#31 lore raw
Quoted 3 lines · Parag Warudkar

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.

#32 lore raw
Quoted 9 lines · Alex Riesen
Quoted 3 lines · Parag Warudkar

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

#33 lore raw
Quoted 6 lines · Parag Warudkar
Quoted 5 lines · Alex Riesen
Quoted 3 lines · Parag Warudkar

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

Quoted 7 lines
Quoted 4 lines

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.

#34 lore raw
Quoted 12 lines · Alex Riesen
Quoted 11 lines · Parag Warudkar
Quoted 9 lines · Alex Riesen
Quoted 3 lines · Parag Warudkar

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.

Quoted 6 lines
Quoted 2 lines

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

#35 lore raw
In reply to Parag Warudkar
Quoted 3 lines · Parag Warudkar

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.

Signature · 4 lines
Knowledge Is Power
Power Corrupts
Study Hard
Be Evil
#36 lore raw
Quoted 9 lines · Zan Lynx
Quoted 3 lines · Parag Warudkar

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