Last Comment Bug 792541 - DLL block request: sprotector.dll
: DLL block request: sprotector.dll
Status: RESOLVED FIXED
[dll][Win8]
:
Product: Toolkit
Classification: Components
Component: Blocklisting (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Benjamin Smedberg [:bsmedberg]
:
Mentors:
Depends on:
Blocks: 785940
  Show dependency treegraph
 
Reported: 2012-09-19 12:59 PDT by Alex Keybl [:akeybl]
Modified: 2016-03-07 15:30 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
fixed
fixed


Attachments
Add a flag for "win8 and up only" and block sprotector.dll, rev. 1 (3.07 KB, patch)
2012-09-21 07:23 PDT, Benjamin Smedberg [:bsmedberg]
ehsan: review+
Details | Diff | Review
Add a flag for "win8 and up only" and block sprotector.dll, rev. 1.1 (3.12 KB, patch)
2012-09-21 09:54 PDT, Benjamin Smedberg [:bsmedberg]
no flags Details | Diff | Review
Add a flag for "win8 and up only" and block sprotector.dll, rev. 1.2 (3.05 KB, patch)
2012-09-21 09:55 PDT, Benjamin Smedberg [:bsmedberg]
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Review

Description Alex Keybl [:akeybl] 2012-09-19 12:59:36 PDT
DLL name: sprotector.dll
DLL versions to block: (unversioned)
Applications, versions, and platforms affected: Windows 8

Reasons:
Safend let us know that "SProtcetor.dll existed in our legacy versions that will not work on Windows 8" and this is a top startup crasher on Win8 (bug 785940).
Comment 1 Benjamin Smedberg [:bsmedberg] 2012-09-19 13:20:51 PDT
So is this supposed to be blocked on Windows 8 *only*? The DLL blocklist currently does not have the capacity to block only on certain versions of Windows.
Comment 2 Alex Keybl [:akeybl] 2012-09-20 10:04:08 PDT
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> So is this supposed to be blocked on Windows 8 *only*? The DLL blocklist
> currently does not have the capacity to block only on certain versions of
> Windows.

Would it be difficult to conditionalize based upon WinNT version? Out of scope for FF16 beta 5 and up? I'm not confident that we should be blocklisting for all versions of Windows, which means this would go unfixed prior to Win8's release.
Comment 3 Benjamin Smedberg [:bsmedberg] 2012-09-21 07:23:04 PDT
Created attachment 663388 [details] [diff] [review]
Add a flag for "win8 and up only" and block sprotector.dll, rev. 1
Comment 4 :Ehsan Akhgari (busy, don't ask for review please) 2012-09-21 07:55:23 PDT
Comment on attachment 663388 [details] [diff] [review]
Add a flag for "win8 and up only" and block sprotector.dll, rev. 1

Review of attachment 663388 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/xre/nsWindowsDllBlocklist.cpp
@@ +57,5 @@
> +
> +  // Flags
> +  uint32_t flags;
> +
> +  static const uint32_t kBLOCK_WIN8PLUS_ONLY = 0x1;

Nit: please make this an enum with a default value of 0.

@@ +288,5 @@
> +  OSVERSIONINFOW osInfo;
> +  osInfo.dwOSVersionInfoSize = sizeof(OSVERSIONINFOW);
> +  GetVersionExW(&osInfo);
> +  return osInfo.dwMajorVersion > 6 || 
> +         osInfo.dwMajorVersion >= 6 && osInfo.dwMinorVersion >= 2;

Nit: please use parenthesis to make the precedence clear.

@@ +378,5 @@
>  #ifdef DEBUG_very_verbose
>      printf_stderr("LdrLoadDll: info->name: '%s'\n", info->name);
>  #endif
>  
> +    if (info->flags & DllBlockInfo::kBLOCK_WIN8PLUS_ONLY &&

Parentheses here too please.
Comment 5 Benjamin Smedberg [:bsmedberg] 2012-09-21 09:54:10 PDT
Created attachment 663446 [details] [diff] [review]
Add a flag for "win8 and up only" and block sprotector.dll, rev. 1.1
Comment 6 Benjamin Smedberg [:bsmedberg] 2012-09-21 09:55:31 PDT
Created attachment 663449 [details] [diff] [review]
Add a flag for "win8 and up only" and block sprotector.dll, rev. 1.2
Comment 7 Benjamin Smedberg [:bsmedberg] 2012-09-26 07:08:35 PDT
https://hg.mozilla.org/mozilla-central/rev/ca4af4af5334

Got approval from akeybl to push this up to aurora/beta immediately, I'll do that in a few hours.
Comment 9 Brian R. Bondy [:bbondy] 2012-09-27 12:55:23 PDT
Comment on attachment 663449 [details] [diff] [review]
Add a flag for "win8 and up only" and block sprotector.dll, rev. 1.2

Review of attachment 663449 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/xre/nsWindowsDllBlocklist.cpp
@@ +282,5 @@
>    return full_fname;
>  }
>  
> +static bool
> +IsWin8OrLater()

To avoid code duplication all over the tree for things like this, this probably would have been better in xpcom/base/nsWindowsHelpers.h where IsVistaOrLater already exists inside an unnamed namespace

Note You need to log in before you can comment on or make changes to this bug.