Closed
Bug 711505
Opened 13 years ago
Closed 13 years ago
Callback application should be locked when doing updates from the service
Categories
(Toolkit :: Application Update, defect)
Tracking
()
RESOLVED
FIXED
mozilla12
People
(Reporter: bbondy, Assigned: bbondy)
References
Details
Attachments
(1 file, 2 obsolete files)
5.42 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
When updates run through the maintenance service, we are no longer passing callback information to the maintenance service (as of a recent security bug). So when the maintenance service runs the updater elevated it does not have the callback information.
For that reason the callback application is not being locked during update, so a user could start it during an update. if this happens the user will get an error saying that they should download the complete MAR. It would be better to give the error we do without the service that the file is in use.
To do this;
> callbackFile = CreateFileW(argv[callbackIndex],
> DELETE | GENERIC_WRITE,
> // allow delete, rename, and write
> FILE_SHARE_DELETE | FILE_SHARE_WRITE,
> NULL, OPEN_EXISTING, 0, NULL);
This should be done from the unelevated callback though since the maintenance service does not have information about the callback application.
Assignee | ||
Comment 1•13 years ago
|
||
This makes it so you can't use firefox while the service is applying the update.
Attachment #582354 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 2•13 years ago
|
||
Alternate more safe way of doing locking that won't cause problems as per discussions.
Attachment #582354 -
Attachment is obsolete: true
Attachment #582354 -
Flags: review?(robert.bugzilla)
Attachment #582371 -
Flags: review?(robert.bugzilla)
Comment 3•13 years ago
|
||
Comment on attachment 582371 [details] [diff] [review]
Patch v2.
Yes! Some manual testing would be a good thing but I am as certain as can be this is the best way to do this.
Attachment #582371 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 4•13 years ago
|
||
Patch v2 was fine but it didn't handle when there was an error for launching the callback. In those cases the callback would be launched as session 0 in addition to the user session. I put the check inside the launch callback app function to catch all past, present and future calls to this function.
Attachment #582371 -
Attachment is obsolete: true
Attachment #582490 -
Flags: review?(robert.bugzilla)
Comment 5•13 years ago
|
||
Comment on attachment 582490 [details] [diff] [review]
Patch v3.
Makes sense. Would be good to go through these different code paths but better to do so when fully awake.
Attachment #582490 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 6•13 years ago
|
||
I tested one error condition which was how I seen a firefox in session 0 with the old patch, the new patch fixes it though. The other error conditions are all the same type of code with launching callback and early return
Assignee | ||
Updated•13 years ago
|
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Assignee | ||
Comment 7•13 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•