Only use two last numbers in NVIDIA driver version numbers

NEW
Assigned to

Status

()

8 years ago
2 years ago

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

(Blocks: 1 bug)

unspecified
x86
Windows 7
Points:
---

Firefox Tracking Flags

(blocking2.0 .x+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

A user with an old NVIDIA Quadro driver on Windows XP is reporting he has driver version 8.3.1.3, so he's getting unwillingly whitelisted because on XP we require 6.14.12.5721 (while on vista/7 we require 8.17.12.5721).

Let's only use the two last numbers here (12.5721).
(Assignee)

Comment 1

8 years ago
Created attachment 517010 [details] [diff] [review]
don't get confused by NVIDIA driver versions

Tryserver build:
http://tbpl.mozilla.org/?tree=MozillaTry&rev=ebc52969d789

Not suggesting that we absolutely need this in Firefox 4.0.0.0.0. This patch is not 100% risk-free. I'm testing it as much as I can by spoofing, but my advice is to take that in the first bugfix update.
Attachment #517010 - Flags: review?(joe)
(Assignee)

Comment 2

8 years ago
I have now tested this patch a lot using spoofing, and it seems to be running well.

Also, I have extracted some stats from crash reports, and they hint that we should really use this patch:

$ grep AdapterVendorID 20110306-pub-crashdata.csv | wc -l
94367

That tells us that 94.4K reports received yesterday were Firefox 4 (since Firefox 3 reports don't have AdapterVendorID)

$ grep AdapterVendorID\:\ 10de 20110306-pub-crashdata.csv | wc -l
29621

That tells us that out of these 94.4K reports, at least 29.6K are using Nvidia cards.

$ egrep AdapterDriverVersion\:\ [0-9]\\.[0-9]\\.[0-9]\\.[0-9] 20110306-pub-cras
hdata.csv  | grep AdapterVendorID\:\ 10de | wc -l
5506

That tells us that out of these 29.6K Nvidia reports, 5.5K have these weird version numbers. That's 19% of all reports using Nvidia cards.

$ egrep AdapterDriverVersion\:\ [7-9]\\.[0-9]\\.[0-9]\\.[0-9] 20110306-pub-cras
hdata.csv  | grep AdapterVendorID\:\ 10de | wc -l
1735

That tells us that out of these 5.5K reports, 1.7K are on machines getting incorrectly whitelisted (assuming they're all on WindowsXP), like the version '8.3.1.3' initially reported, because 8.3.1.3 >= 6.14.12.5721.

1735/29621 = 6% of Nvidia users
1735/94621 = 1.8% of all Firefox 4 reports

So I think that's quite serious and we should land the patch.
blocking2.0: --- → ?
(Assignee)

Comment 3

8 years ago
Created attachment 517446 [details] [diff] [review]
Don't get confused by NVIDIA driver versions, updated

There actually was a bug there that would have made the current entry in the downloaded blocklist, using 8.17.x.x numbers, not take effect. Scary! patch updated.
Attachment #517010 - Attachment is obsolete: true
Attachment #517010 - Flags: review?(joe)
Attachment #517446 - Flags: review?(bjacob)
(Assignee)

Updated

8 years ago
Attachment #517446 - Flags: review?(bjacob) → review?(joe)
(Assignee)

Comment 4

8 years ago
After discussion with Joe: since:
 * this is important enough that we want it in Firefox 4.0 if possible, but
 * this is probably not important enough to delay Firefox 4 release by itself,
this looks like a good 'RC ride-along' candidate.
Comment on attachment 517446 [details] [diff] [review]
Don't get confused by NVIDIA driver versions, updated

As follow-ups for this, we should a) insert a test that makes sure 8.1.1.0-style stuff is blocked on XP, and that the first two numbers also don't impact the blacklisting; b) contact NVIDIA to figure out what their long-term driver version style is going to be, so we can code for that.
Attachment #517446 - Flags: review?(joe) → review+
Er, actually, maybe we should remove that const_cast<>, and use a temporary instead. Not sure whether we can rely on the ability to modify const globals.
You can see the tests I wrote in bug 625160, and use them as a template.
blocking2.0: ? → .x+
Assignee: nobody → bjacob
Blocks: 1285569
You need to log in before you can comment on or make changes to this bug.