Closed Bug 679945 Opened 9 years ago Closed 2 years ago

Add DLL Blocklist telemetry

Categories

(Toolkit :: Blocklist Policy Requests, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED INVALID
mozilla9

People

(Reporter: khuey, Unassigned)

Details

Attachments

(1 file)

We should add telemetry for the DLL blocklist.  Specifically of interest is whether or not the hook installed properly and which DLLs if any were blocked from loading.
Attached patch PatchSplinter Review
This reports whether or not the blocklist hook was installed successfully.  Reporting DLLs blocked would be a lot trickier.
Assignee: nobody → khuey
Status: NEW → ASSIGNED
Attachment #555500 - Flags: review?(benjamin)
Attachment #555500 - Flags: review?(benjamin) → review+
http://hg.mozilla.org/mozilla-central/rev/1b2c92b52298
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
As mentioned on IRC the following may be required for comm-central:

    diff --git a/toolkit/xre/nsWindowsDllBlocklist.cpp b/toolkit/xre/nsWindowsDllBlocklist.cpp
    --- a/toolkit/xre/nsWindowsDllBlocklist.cpp
    +++ b/toolkit/xre/nsWindowsDllBlocklist.cpp
    @@ -221,14 +221,18 @@ WindowsDllInterceptor NtDllIntercept;
     
     void
     XRE_SetupDllBlocklist()
     {
       NtDllIntercept.Init("ntdll.dll");
     
       bool ok = NtDllIntercept.AddHook("LdrLoadDll", reinterpret_cast<intptr_t>(patched_LdrLoadDll), (void**) &stub_LdrLoadDll);
     
    +#ifdef XRE_WANT_DLL_BLOCKLIST
    +  XRE_TelemetryAccumulate(Telemetry::DLLBLOCKLIST_HOOK_INSTALLED, ok ? 1 : 0);
    +#else
       Telemetry::Accumulate(Telemetry::DLLBLOCKLIST_HOOK_INSTALLED, ok ? 1 : 0);
    +#endif
     #ifdef DEBUG
       if (!ok)
         printf_stderr ("LdrLoadDll hook failed, no dll blocklisting active\n");
     #endif
     }
(In reply to Mike Hommey [:glandium] from comment #4)
> As mentioned on IRC the following may be required for comm-central:

Thunderbird is up to date wrt this code now and I just did a compile with the patch and it succeeded.

Dunno about SeaMonkey (I suspect they are still blocking this, or you need the ifdef).
(In reply to Mark Banner (:standard8) from comment #6)
> (In reply to Mike Hommey [:glandium] from comment #4)
> > As mentioned on IRC the following may be required for comm-central:
> 
> Thunderbird is up to date wrt this code now and I just did a compile with
> the patch and it succeeded.
> 
> Dunno about SeaMonkey (I suspect they are still blocking this, or you need
> the ifdef).

That (c#4) would be required for SeaMonkey in the short term, failing that I'm ok with using the "introduced for seamonkey" define for this, or changing the define name.

(http://hg.mozilla.org/mozilla-central/rev/1985330ca298)

I am _hoping_ to get this code up to date for SeaMonkey in the Gecko 10 cycle.
Is Seamonkey ready to go?  I'd like to land this at some point.
(In reply to Justin Wood (:Callek) from comment #7)
> I am _hoping_ to get this code up to date for SeaMonkey in the Gecko 10
> cycle.

Unfortunately, I did not get SeaMonkey ready yet, it is on my priority list, but got sidetracked with critical releng machine failures and bringing them back.

I am ok with landing this with ifndef MOZ_SUITE around the telemetry stuff if you like, or we can do c#4's solution if it works.
We do want this; it'd be nice to get whatever issues are still outstanding sorted.  Patch will require some updating.
Assignee: khuey → nobody
Do we still want this and should move this to a more fitting component?
Or should we close it?
Flags: needinfo?(nfroyd)
(In reply to Georg Fritzsche [:gfritzsche] from comment #11)
> Do we still want this and should move this to a more fitting component?
> Or should we close it?

I think we still want this, and we should do the following:

1. Reland the original patch.  Thunderbird apparently works OK with this; SeaMonkey can submit patches to fix compilation problems.
2. Move the "report DLLs blocked" telemetry to a different bug.
3. Close this bug.
Flags: needinfo?(nfroyd)
This doesn't apply anymore as is.
This uses TelemetryHistograms.h, we now have to register histograms in Histograms.json [0].
This will also need an owner and data collection review [1].

Moving to the blocklisting component.

0: https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/Histograms.json
1: https://wiki.mozilla.org/Firefox/Data_Collection
Component: Telemetry → Blocklisting
Nathan, is there any interest in updating this patch? This bug hasn't had any activity for a very long time.
Flags: needinfo?(nfroyd)
Closing old blocklist requests that shouldn't be valid after the move to WebExtensions-only in Firefox 57. Please comment if you think this bug is still valid and should be reopened.
Status: REOPENED → RESOLVED
Closed: 9 years ago2 years ago
Resolution: --- → INVALID
Flags: needinfo?(nfroyd)
You need to log in before you can comment on or make changes to this bug.