There's this code at http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/toolkit/components/resistfingerprinting/nsRFPService.cpp#151 : > if (!mInitialTZValue.IsEmpty()) { > nsAutoCString tzValue = NS_LITERAL_CSTRING("TZ=") + mInitialTZValue; > PR_SetEnv(tzValue.get()); > } else { The value string that's passed to PR_SetEnv goes out of scope immediately, so the pointer that PR_SetEnv now holds on to is no longer valid. We have other callers of PR_SetEnv with dynamic strings in the tree, and those use ToNewCString in order to copy and leak the string, e.g. here: http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/toolkit/crashreporter/nsExceptionHandler.cpp#2504 And then there are a few valgrind annotations for these intentional leaks at http://searchfox.org/mozilla-central/source/build/valgrind/cross-architecture.sup#8 .

Thanks for looking into this. I will fix this.

Comment on attachment 8888653 [details] Bug 1382840 - Making the nsRFPService::UpdatePref() to copy the string which been passed to PR_SetEnv(). https://reviewboard.mozilla.org/r/159686/#review165252 ::: toolkit/components/resistfingerprinting/nsRFPService.cpp:157 (Diff revision 1) > + > + // If the tz has been set before, we free it first since it will be allocated > + // a new value later. > + if (tz) { > + free(tz); > + tz = nullptr; No point in nulling this out here.

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/80b3b7fa1e23 Making the nsRFPService::UpdatePref() to copy the string which been passed to PR_SetEnv(). r=Ehsan

Please request Beta approval on this.

Comment on attachment 8888653 [details] Bug 1382840 - Making the nsRFPService::UpdatePref() to copy the string which been passed to PR_SetEnv(). Approval Request Comment [Feature/Bug causing the regression]: 1330890 [User impact if declined]: Users might get wrong timezone when they flip off the pref 'privacy.resistFingerprinting' if they have set the TZ environment value by themselves. [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: No [Is the change risky?]: Low risky, because of this pref is set off by default, so the normal users will not encounter this issue. [Why is the change risky/not risky?]: [String changes made/needed]:

Comment on attachment 8888653 [details] Bug 1382840 - Making the nsRFPService::UpdatePref() to copy the string which been passed to PR_SetEnv(). makes sense, seems worth getting in 55.0b13

(In reply to Tim Huang[:timhuang] from comment #8) > [Is this code covered by automated tests?]: No > [Has the fix been verified in Nightly?]: Yes > [Needs manual test from QE? If yes, steps to reproduce]: No Setting qe-verify- based on Tim's assessment on manual testing needs.