Closed Bug 1573270 Opened 5 years ago Closed 4 years ago

Add unit tests and comments for the read-only shared memory fix in bug 1565744

Categories

(Core :: IPC, task, P2)

Unspecified
Windows
task

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr68 --- wontfix
firefox71 --- wontfix
firefox72 --- fixed

People

(Reporter: jld, Assigned: jld)

References

Details

(Keywords: sec-other, Whiteboard: [adv-main72-][no-nag][post-critsmash-triage])

Attachments

(1 file)

I'd like to commit a unit test for the failure mode fixed in bug 1565744, and comments explaining the code (which deals with some… not-well-documented cases of the Windows security model; note that Chrome had the same bug for years until it was reported by an external researcher).

However, for the usual reasons, this should probably wait at least until the fix has reached all the affected release channels (+ padding for people to actually update).

Bug 1479960 is going to move the code in question, and already has unit tests for other cases, but not this one. I have a patch that adds the tests and comments.

Type: enhancement → task
Keywords: sec-other

Bug 1565744 was fixed in 69 and 68.1, released on 2019-09-03. Has it been long enough that I could land the comments and unit test for that fix, or should I wait longer? (There's background info on the bug/patch in the sec-approval request: bug 1565744 comment #8.)

Flags: needinfo?(dveditz)

Absolutely. We could land tests for Fx70 bugs now, too: it's been a month most people have upgraded who were going to.

Flags: needinfo?(dveditz)

https://hg.mozilla.org/integration/autoland/rev/9d28ee24c377

(Landing during the soft freeze because this is mostly-NPOTB: a unit test, which passes on try, and comments.)

Backed out changeset 9d28ee24c377 (bug 1573270) for xpcshell failures at toolkit/modules/tests/xpcshell/test_firstStartup.js

Backout: https://hg.mozilla.org/integration/autoland/rev/fc01d4367291cbc38244097b939653bfd1496ac1

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=9d28ee24c3772c25c4995d202bc23e4484854dc0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=278105742&repo=autoland&lineNumber=5925

task 2019-11-25T23:47:32.812Z] 23:47:32 INFO - TEST-PASS | toolkit/modules/tests/xpcshell/test_firstStartup.js | test_timeout - [test_timeout : 46] 2 == 2
[task 2019-11-25T23:47:32.812Z] 23:47:32 INFO - (xpcshell/head.js) | test run_next_test 1 finished (2)
[task 2019-11-25T23:47:32.812Z] 23:47:32 INFO - (xpcshell/head.js) | test run_next_test 2 pending (2)
[task 2019-11-25T23:47:32.812Z] 23:47:32 INFO - (xpcshell/head.js) | test test_timeout finished (2)
[task 2019-11-25T23:47:32.812Z] 23:47:32 INFO - "CONSOLE_MESSAGE: (warn) [JavaScript Warning: "firstStartup.elapsed - Truncating float/double number."]"
[task 2019-11-25T23:47:32.812Z] 23:47:32 INFO - PID 7356 | [7356, Main Thread] WARNING: Ignoring duplicate observer.: file z:/build/build/src/modules/libpref/Preferences.cpp, line 2696
[task 2019-11-25T23:47:32.813Z] 23:47:32 INFO - (xpcshell/head.js) | test run_next_test 2 finished (1)
[task 2019-11-25T23:47:32.813Z] 23:47:32 INFO - exiting test
[task 2019-11-25T23:47:32.813Z] 23:47:32 INFO - PID 7356 | [7356, Main Thread] WARNING: Ignoring duplicate observer.: file z:/build/build/src/modules/libpref/Preferences.cpp, line 2696
[task 2019-11-25T23:47:32.813Z] 23:47:32 INFO - PID 7356 | [7356, Main Thread] WARNING: Ignoring duplicate observer.: file z:/build/build/src/modules/libpref/Preferences.cpp, line 2696
[task 2019-11-25T23:47:32.813Z] 23:47:32 INFO - PID 7356 | [7356, Main Thread] WARNING: Ignoring duplicate observer.: file z:/build/build/src/modules/libpref/Preferences.cpp, line 2696
[task 2019-11-25T23:47:32.813Z] 23:47:32 INFO - PID 7356 | [7356, Main Thread] WARNING: Ignoring duplicate observer.: file z:/build/build/src/modules/libpref/Preferences.cpp, line 2696
[task 2019-11-25T23:47:32.813Z] 23:47:32 INFO - PID 7356 | [7356, Main Thread] WARNING: This method is lossy. Use GetCanonicalPath !: file z:/build/build/src/xpcom/io/nsLocalFileWin.cpp, line 3140
[task 2019-11-25T23:47:32.813Z] 23:47:32 INFO - PID 7356 | [7356, Main Thread] WARNING: This method is lossy. Use GetCanonicalPath !: file z:/build/build/src/xpcom/io/nsLocalFileWin.cpp, line 3140
[task 2019-11-25T23:47:32.813Z] 23:47:32 INFO - PID 7356 | [7356, Main Thread] WARNING: This method is lossy. Use GetCanonicalPath !: file z:/build/build/src/xpcom/io/nsLocalFileWin.cpp, line 3140
[task 2019-11-25T23:47:32.813Z] 23:47:32 INFO - "CONSOLE_MESSAGE: (error) [JavaScript Error: "NetworkError when attempting to fetch resource." {file: "resource://services-settings/Utils.jsm" line: 109}]
[task 2019-11-25T23:47:32.813Z] 23:47:32 INFO - fetchLatestChanges@resource://services-settings/Utils.jsm:109:28
[task 2019-11-25T23:47:32.813Z] 23:47:32 INFO - sync@resource://services-settings/RemoteSettingsClient.jsm:401:37
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - get@resource://services-settings/RemoteSettingsClient.jsm:354:22
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - asyncloadRecipes@resource://normandy/lib/RecipeRunner.jsm:344:51
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - run@resource://normandy/lib/RecipeRunner.jsm:296:35
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - async
init@resource://normandy/lib/RecipeRunner.jsm:120:18
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - asyncfinishInit@resource://normandy/Normandy.jsm:129:24
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - async
init/<@resource://normandy/Normandy.jsm:65:54
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - init@resource://gre/modules/FirstStartup.jsm:59:19
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - test_success@Z:/task_1574722640/build/tests/xpcshell/tests/toolkit/modules/tests/xpcshell/test_firstStartup.js:14:16
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - run_next_test/_run_next_test/<@Z:\task_1574722640\build\tests\xpcshell\head.js:1567:22
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - _run_next_test@Z:\task_1574722640\build\tests\xpcshell\head.js:1567:38
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - run@Z:\task_1574722640\build\tests\xpcshell\head.js:735:9
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - _do_main@Z:\task_1574722640\build\tests\xpcshell\head.js:246:6
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - _execute_test@Z:\task_1574722640\build\tests\xpcshell\head.js:573:5
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - @-e:1:1
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - "
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - PID 7356 | [7356, Main Thread] WARNING: Workers don't support the 'mem.mem.' preference!: file z:/build/build/src/dom/workers/RuntimeService.cpp, line 547
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - "CONSOLE_MESSAGE: (error) [JavaScript Error: "IndexedDB UnknownErr: ActorsParent.cpp:12712"]"
[task 2019-11-25T23:47:32.814Z] 23:47:32 INFO - "CONSOLE_MESSAGE: (error) [JavaScript Error: "IndexedDB UnknownErr: ActorsParent.cpp:12712"]"
[task 2019-11-25T23:47:32.815Z] 23:47:32 WARNING - TEST-UNEXPECTED-FAIL | toolkit/modules/tests/xpcshell/test_firstStartup.js | test_timeout - [test_timeout : 265] A promise chain failed to handle a rejection: A request was aborted, for example through a call to IDBTransaction.abort. - stack: Transaction/this._completionPromise</transaction.onabort@resource://gre/modules/IndexedDB.jsm:254:15
[task 2019-11-25T23:47:32.815Z] 23:47:32 INFO - _execute_test@Z:\task_1574722640\build\tests\xpcshell\head.js:661:19
[task 2019-11-25T23:47:32.815Z] 23:47:32 INFO - @-e:1:1
[task 2019-11-25T23:47:32.815Z] 23:47:32 INFO - Rejection date: Mon Nov 25 2019 23:47:32 GMT+0000 (Greenwich Mean Time) - false == true
[task 2019-11-25T23:47:32.815Z] 23:47:32 INFO - resource://testing-common/PromiseTestUtils.jsm:assertNoUncaughtRejections:265
[task 2019-11-25T23:47:32.815Z] 23:47:32 INFO - Z:\task_1574722640\build\tests\xpcshell\head.js:_execute_test:668
[task 2019-11-25T23:47:32.815Z] 23:47:32 INFO - -e:null:1
[task 2019-11-25T23:47:32.815Z] 23:47:32 INFO - exiting test

Flags: needinfo?(jld)

The patch only added a gtest (not xpcshell test) and some comments, so it shouldn't have been able to cause that failure. I've retriggered the tests on a few of the earlier revisions to try to get more information.

I notice that the failure log mentions Normandy (in the stack below NetworkError), and there were some pushes just before mine that touched that code, in particular b4da4b5e07816cb653efe702d3f1819999b2fc08 / bug 1594035; maybe that's the real cause?

You are right, sorry about that. I've just re-landed your patch. Thank you

Flags: needinfo?(jld)
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Flags: in-testsuite+
Whiteboard: [no-nag] → [adv-main72-][no-nag]
Flags: qe-verify-
Whiteboard: [adv-main72-][no-nag] → [adv-main72-][no-nag][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: