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.
Created attachment 582354 [details] [diff] [review] Patch v1. This makes it so you can't use firefox while the service is applying the update.
Created attachment 582371 [details] [diff] [review] Patch v2. Alternate more safe way of doing locking that won't cause problems as per discussions.
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+
Created attachment 582490 [details] [diff] [review] Patch v3. 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.
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+
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
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in before you can comment on or make changes to this bug.