Closed Bug 755638 Opened 8 years ago Closed 7 years ago

Uncaught SecurityException when attempting to sync multiple Fennecs to the same Sync account.

Categories

(Firefox for Android :: Android Sync, defect, critical)

ARM
Android
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla17
Tracking Status
firefox15 + wontfix
firefox16 + verified

People

(Reporter: scoobidiver, Assigned: liuche)

References

(Depends on 1 open bug)

Details

(Keywords: crash, reproducible, topcrash, Whiteboard: [native-crash][14.0b1], sync)

Crash Data

Attachments

(2 files, 4 obsolete files)

Seen in the first days of 14.0b1: bp-07364ce0-79d2-4e7d-aea9-f1c182120515.

java.lang.SecurityException: caller uid 10164 is different than the authenticator's uid
	at android.os.Parcel.readException(Parcel.java:1322)
	at android.os.Parcel.readException(Parcel.java:1276)
	at android.accounts.IAccountManager$Stub$Proxy.getUserData(IAccountManager.java:440)
	at android.accounts.AccountManager.getUserData(AccountManager.java:333)
	at org.mozilla.gecko.sync.syncadapter.SyncAdapter.getClientsCount(SyncAdapter.java:495)
	at org.mozilla.gecko.sync.syncadapter.SyncAdapter.getSyncInterval(SyncAdapter.java:352)
	at org.mozilla.gecko.sync.syncadapter.SyncAdapter.onPerformSync(SyncAdapter.java:334)
	at android.content.AbstractThreadedSyncAdapter$SyncThread.run(AbstractThreadedSyncAdapter.java:164)

More reports at:
https://crash-stats.mozilla.com/query/query?product=FennecAndroid&version=ALL%3AALL&range_value=1&range_unit=weeks&query_search=signature&query_type=contains&query=authenticator&do_query=1
Whiteboard: [native-crash][startupcrash][14.0b1] → [native-crash][startupcrash][14.0b1], sync
This is the signature for users with Sync set up and more than one Firefox installed. It should occur once per user, then we'll disable the other Firefox for that Sync account to prevent it happening again.

I'd call this routine.
Severity: critical → normal
Whiteboard: [native-crash][startupcrash][14.0b1], sync → [native-crash][14.0b1], sync
Crash Signature: [@ java.lang.SecurityException: caller uid 10164 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] → [@ java.lang.SecurityException: caller uid 10164 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10216 is different than the authenticator''s uid at android.os.Parcel.…
Depends on: 757121
False dependency; timing coincidence.
No longer depends on: 757121
(In reply to Richard Newman [:rnewman] from comment #2)
> False dependency; timing coincidence.
What are the difference between this generic bug and a bug with STR that makes you think they are unrelated although they share their signature and stack?
(In reply to Scoobidiver from comment #3)
> (In reply to Richard Newman [:rnewman] from comment #2)
> > False dependency; timing coincidence.
> What are the difference between this generic bug and a bug with STR that
> makes you think they are unrelated although they share their signature and
> stack?

The other bug is "crash on page". There is no causality: the page has nothing to do with it, so the dependency is misleading.

A background thread initiated by Android is dying because the user has more than one Fennec installed.
(In reply to Richard Newman [:rnewman] from comment #4)

> The other bug is "crash on page". There is no causality: the page has
> nothing to do with it, so the dependency is misleading.

Note that I'm not saying the crash signature differs... if you like, you can dupe the other bug to this one. Just that there is no dependency relationship here.
I run into this crash: https://crash-stats.mozilla.com/report/index/bp-91ae0d10-9424-4e24-ad1e-e4f452120607 when I had Beta build installed, sync running and after that I installed nightly and tried to sync. 

Build: Firefox 16.0a1 (2012-06-07)
Device: LG Optimus 2X (Android 2.2.2)
(In reply to Andreea Pod from comment #6)
> I run into this crash:
> https://crash-stats.mozilla.com/report/index/bp-91ae0d10-9424-4e24-ad1e-
> e4f452120607 when I had Beta build installed, sync running and after that I
> installed nightly and tried to sync. 
> 
> Build: Firefox 16.0a1 (2012-06-07)
> Device: LG Optimus 2X (Android 2.2.2)

Yup, expected.
Blocks: 761206
Crash Signature: android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10166 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] → android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10166 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10080 is different than…
1 	http://en.m.wikipedia.org/wiki/Standard_RAID_levels
1 	http://1cooks.co.uk/
1 	http://jsbin.com/3/ipelar.js
Crash Signature: than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] → than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10133 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: c…
Crash Signature: [@ java.lang.SecurityException: caller uid 10164 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10216 is different than the authenticator''s uid at android.os.Parcel.… → [@ java.lang.SecurityException: caller uid 10099 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10164 is different than the authenticator''s uid at android.os.Parcel.…
1. place nightly on the device
2. move nightly to SDcard
3. setup sync
4. reboot device
5. go back to sync and select nightly to sync
6. launch nightly

Expected: no crash
Actual: crash.

related to bug 768102
Keywords: reproducible
Turns out that I had multiple firefox installed.  Scratch comment 9

STR:
1. place firefox release on device, place nightly on device
2. set up sync accounts in the sync section
3. set nightly to the version to sync
4. launch nightly

Expected, no crash
Actual: crash.
Crash Signature: than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10232 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] → than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10232 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: c…
Crash Signature: caller uid 10109 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] → caller uid 10109 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10136 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)]
Crash Signature: caller uid 10109 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10136 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] → caller uid 10109 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10136 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ …
No longer blocks: 761206
Depends on: 761206
Duplicate of this bug: 778722
I get this after every 20 seconds of uptime on my release build from the market. I have sync setup between multiple phones and desktop.
(In reply to Benoit Girard (:BenWa) from comment #12)
> I get this after every 20 seconds of uptime on my release build from the
> market. I have sync setup between multiple phones and desktop.

Do you have more than one channel installed on your phone? (Not "installed with Sync set up", just installed is enough.)
I have 'Firefox' (release) installed with sync, and 'Fennec bgirard' (dev build) with sync. I don't currently have any other channel installed but it's likely I did at one point. I'm running jellybean.
(In reply to Benoit Girard (:BenWa) from comment #14)
> I have 'Firefox' (release) installed with sync, and 'Fennec bgirard' (dev
> build) with sync. I don't currently have any other channel installed but
> it's likely I did at one point. I'm running jellybean.

If your custom Fennec build is earlier than <https://hg.mozilla.org/mozilla-central/rev/28106e179603>, then this is your problem.

(Even if your build is after that, you shouldn't be able to have two different Sync accounts set up.)
(In reply to Richard Newman [:rnewman] from comment #15)
> If your custom Fennec build is earlier than
> <https://hg.mozilla.org/mozilla-central/rev/28106e179603>, then this is your
> problem.
There are no crashes after 17.0a1/20120726, so it's fixed by bug 772645.
Status: NEW → RESOLVED
Closed: 7 years ago
Depends on: 772645
Keywords: topcrash
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
To clarify the tracking flags, with combined signatures, it's #23 top crasher in 15.0b2 and #3 in 16.0a2.
(In reply to Scoobidiver from comment #16)

> There are no crashes after 17.0a1/20120726, so it's fixed by bug 772645.

Coincidence; see Bug 772645 comment 16.

My hypothesis is that after uplift to Aurora (train or not), this will reoccur. It would also reoccur if a user had two of {Aurora, Beta, Release} installed and syncing.
Status: RESOLVED → REOPENED
Depends on: 777800
Resolution: FIXED → ---
Target Milestone: Firefox 17 → ---
Crash Signature: java.lang.SecurityException: caller uid 10147 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] → java.lang.SecurityException: caller uid 10147 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10038 is different than the authenticator''s uid at android.os.Parcel.rea…
Crash Signature: android.os.Parcel.readException(Parcel.java)] → android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10338 is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java)] [@ java.lang.SecurityException: caller uid 10168 is different than…
Assigning to Richard, if I'm reading this bug correctly there's a once-per-user crash when they have a second Firefox installed on their phones (and using Sync).  This is a poor experience and at a time where we're trying to grow our rating on the Play Store, it seems like trying to get rid of a crash we can reproduce should be a priority. What are the options here for clearing this up?
Assignee: nobody → rnewman
(In reply to Lukas Blakk [:lsblakk] from comment #19)
> What are the options here for clearing this up?
If I understood the explanation (see bug 772645#c16), it will be fixed by bug 777800.
(In reply to Scoobidiver from comment #20)

> If I understood the explanation (see bug 772645#c16), it will be fixed by
> bug 777800.

Correct, in the general case, and some of the instances will be avoided by the partitioning introduced in Bug 772645.

(See the "Depends On" field.)

We can attempt to catch this exception at our first point of interaction with the Android accounts system, but I'd be hesitant to pile on too many Band-Aids in this code path.
Status: REOPENED → NEW
Chenxia, will you have time to attend to this today?

If we can, I'd like to see if we can get a patch against mozilla-beta and mozilla-aurora that prevents any user-visible crash here, perhaps simply by catching SecurityException in performSync.

This is a travel week for me, so I'm hesitant to say I can fit this in myself, but I'll try if you can't.
Assignee: rnewman → nobody
Component: General → Android Sync
Product: Firefox for Android → Mozilla Services
Version: Firefox 14 → unspecified
Testing and building the patches now, Richard. I'll need flags for landing on beta and aurora.

This should be a small fix, just adding handling for SecurityException (and uncaught exceptions) in SyncAdapter where there was none before.
Assignee: nobody → liuche
Summary: java.lang.SecurityException: caller uid <n> is different than the authenticator''s uid at android.os.Parcel.readException(Parcel.java) → Uncaught SecurityException when attempting to sync multiple Fennecs to the same Sync account.
Attached patch mozilla-aurora patch v1 (obsolete) — Splinter Review
Catches and handles SecurityException. Unchecks sync checkbox automatically.
Attachment #651512 - Flags: review?(rnewman)
Attached patch mozilla-beta patch v1 (obsolete) — Splinter Review
Catches and handles SecurityException. Unchecks sync checkbox automatically.
Attachment #651513 - Flags: review?(rnewman)
Comment on attachment 651513 [details] [diff] [review]
mozilla-beta patch v1

Review of attachment 651513 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/sync/syncadapter/SyncAdapter.java
@@ +154,3 @@
>        syncResult.stats.numIoExceptions++;
> +    } catch (Exception ex) {
> +      Logger.error(LOG_TAG, "Unknown exception. Aborting sync.", ex);

This doesn't need to be here. This method is called with an Exception, and the clauses don't take any action that would throw. The `finally` clause is just there so that each clause can `return`.

@@ +279,5 @@
> +        // Uncheck Sync checkbox because we cannot sync this instance.
> +        ThreadPool.run(new Runnable() {
> +          @Override
> +          public void run() {
> +            SyncAccounts.setSyncAutomatically(account, false);

How 'bout

  class SyncAccounts {
    public void backgroundSetSyncAutomatically(…) {
      …
    }
    …
  }

? That'll make this clearer.
Attachment #651513 - Flags: review?(rnewman) → feedback+
Comment on attachment 651512 [details] [diff] [review]
mozilla-aurora patch v1

Review of attachment 651512 [details] [diff] [review]:
-----------------------------------------------------------------

Same comment as for beta.
Attachment #651512 - Flags: review?(rnewman)
Severity: normal → critical
Attached patch mozilla-beta patch v2 (obsolete) — Splinter Review
Addressed comments, pulled off-main-thread Account handling out into SyncAccounts.
Attachment #651513 - Attachment is obsolete: true
Attachment #651527 - Flags: review?(rnewman)
Attachment #651527 - Flags: approval-mozilla-aurora?
Attached patch mozilla-aurora patch v2 (obsolete) — Splinter Review
Addressed comments, pulled off-main-thread Account handling out into SyncAccounts.
Attachment #651512 - Attachment is obsolete: true
Attachment #651528 - Flags: review?(rnewman)
Changes from Chenxia's beta patch:

* Request beta, not aurora, approval;
* Moved syncAutomatically change into exception handler, not duplicated.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Old.

User impact if declined: 
  Subset of users with two Fennecs installed can see a crash on first sync of second version.

Testing completed (on m-c, etc.):
  None yet. Will port patch to inbound momentarily.

Risk to taking this patch (and alternatives if risky):
  Should only affect the exception path; slim risk.

String or UUID changes made by this patch: 
  None.
Attachment #651527 - Attachment is obsolete: true
Attachment #651527 - Flags: review?(rnewman)
Attachment #651527 - Flags: approval-mozilla-aurora?
Attachment #651630 - Flags: review+
Attachment #651630 - Flags: approval-mozilla-beta?
Same, but for Aurora.

Chenxia, back over to you. Please double-check my review changes, backport this to Git and m-c, verify, and so forth.

Much obliged!
Attachment #651528 - Attachment is obsolete: true
Attachment #651528 - Flags: review?(rnewman)
Attachment #651632 - Flags: review+
Attachment #651632 - Flags: approval-mozilla-aurora?
rnewman's beta and aurora patches both test out fine on my device.

Backported to git, tested, and pushed to develop, but waiting to for tree to open again.
(In reply to Chenxia Liu [:liuche] from comment #32)
> rnewman's beta and aurora patches both test out fine on my device.

Upload builds for QA? Tight on beta.
Status: NEW → ASSIGNED
Beta and Aurora builds: http://people.mozilla.org/~liuche/bug-755638/

STR:
1. Install release (or beta) from marketplace. Set up sync.
2. Install test apk.
3. Go to Settings > Accounts > (Sync Account). Both versions of FF should show up in the Data & Synchronization, with release checked, and test version unchecked.
4. Click on unchecked test version. It should immediately uncheck, and no crash should occur.

Over to QA. I'll be on PTO but have internet access, so if no one else is around, I will land it when this gets beta and aurora approval.
Comment on attachment 651632 [details] [diff] [review]
Aurora patch, updated.

Approving for Aurora, not sure need to take this on Beta at this point in the cycle if a user has to have two different Firefox products installed - that's an unlikely use case for our GA so this can ride the trains.
Attachment #651632 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #651630 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
ttps://hg.mozilla.org/releases/mozilla-aurora/rev/1ec304d94205
https://hg.mozilla.org/mozilla-central/rev/2759cfa65dc5
Status: ASSIGNED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
updating status-15 to wontfix.
Duplicate of this bug: 784426
Status: RESOLVED → VERIFIED
Product: Mozilla Services → Android Background Services
Product: Android Background Services → Firefox for Android
You need to log in before you can comment on or make changes to this bug.