1688592 - Crash in [@ g_type_check_instance]

Status: RESOLVED WORKSFORME

(Core :: Widget: Gtk, defect)

Description

[Gabriele Svelto [:gsvelto] (PTO)]

Crash report: https://crash-stats.mozilla.org/report/index/5920e127-f2c9-4520-b050-694ef0210124

Reason: SIGSEGV /0x00000080

Top 10 frames of crashing thread:

0 libgobject-2.0.so.0 g_type_check_instance gobject/gtype.c:4134
1 libgobject-2.0.so.0 g_signal_emit_valist gobject/gsignal.c:3277
2 libgobject-2.0.so.0 g_signal_emit gobject/gsignal.c:3554
3 libgobject-2.0.so.0 g_closure_invoke gobject/gclosure.c:810
4 libgobject-2.0.so.0 signal_emit_unlocked_R gobject/gsignal.c:3742
5 libgobject-2.0.so.0 g_signal_emit_valist gobject/gsignal.c:3498
6 libgobject-2.0.so.0 g_signal_emit gobject/gsignal.c:3554
7 libibus-1.0.so.5 ibus_proxy_dispose src/src/ibusproxy.c:102
8 libgobject-2.0.so.0 g_object_run_dispose gobject/gobject.c:1226
9 libgobject-2.0.so.0 g_closure_invoke gobject/gclosure.c:810

Low volume, given the stack and comments this seems to happen when we lose a connection to dbus.

Comment 1

[pete.rolph]

I'm getting repeatedly hit by this, representative report: https://crash-stats.mozilla.org/report/index/448c5685-fb43-4112-80d9-0f4b00220114

I get a slightly different reason SIGSEGV / SI_KERNEL and a different stack trace.

The crash occurs whenever I attempt to leave a Google Meet call or close the tab containing it.

Comment 2

[Gabriele Svelto [:gsvelto] (PTO)]

Thanks for your comment and for the STR, that's precious information to figure this bug out. I'm making the bug private because I double-checked the crashes and these are use-after-frees so potentially security sensitive. Martin can you please have a look? This seems to be only happening on versions >= 95.

Comment 3

[pete.rolph]

You're very welcome! What is an STR?

Comment 4

[Gabriele Svelto [:gsvelto] (PTO)]

"Steps to reproduce"

Comment 5

[Martin Stránský [:stransky] (ni? me)]

I wonder if we call gtk_widget_disconnect_frame_clock() twice so the second call is called for already freed instance.

Comment 6

[Martin Stránský [:stransky] (ni? me)]

As a first step we should put MOZ_DIAGNOSTIC_ASSERT(IsMainThread()) to nsWindow::Destroy().

Comment 7

[Martin Stránský [:stransky] (ni? me)]

(In reply to Martin Stránský [:stransky] (ni? me) from comment #6)

As a first step we should put MOZ_DIAGNOSTIC_ASSERT(IsMainThread()) to nsWindow::Destroy().

Bug 1750513.

Comment 8

[Martin Stránský [:stransky] (ni? me)]

Let's see if we see any assertions from nsWindow::Destroy() when Bug 1750513 lands.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Hello Martin, do you see any improvements after bug 1750513 landed?
thanks

Comment 10

[Martin Stránský [:stransky] (ni? me)]

I haven't seen any related crashes recently so I think it's fixed.
Also there's a check we release it in correct thread (MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread()) so will clearly crash in the worst case.
I'd say it's not a security issue any more.

Comment 11

[Gabriele Svelto [:gsvelto] (PTO)]

There doesn't seem to be any crashes in 110 nor could I find crashes hitting the new assert so I'd say we can flag this fixed for 110.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Did something specifically land in 110 to address this? Do we need to do something for ESR still?

Comment 13

[Martin Stránský [:stransky] (ni? me)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #12)

Did something specifically land in 110 to address this? Do we need to do something for ESR still?

Possible crash was fixed by Bug 1802977 in 109.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

We're still seeing crash reports from 109, though:
https://crash-stats.mozilla.org/report/index/5a9524cf-028d-458b-8e31-515cd0230126

Comment 15

[Martin Stránský [:stransky] (ni? me)]

This one is promising:

https://crash-stats.mozilla.org/report/index/5e8e40c4-d588-4aef-bbc9-0ab370230129

But these reports are completely different bug, original one was about wrong release from a different thread AFAIK.

Comment 16

[Martin Stránský [:stransky] (ni? me)]

(In reply to Martin Stránský [:stransky] (ni? me) from comment #15)

This one is promising:

https://crash-stats.mozilla.org/report/index/5e8e40c4-d588-4aef-bbc9-0ab370230129

hm, that's actually 102 ESR which looks like the original one.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Do we need another bug then? The crashes from 109 still look like UAFs to me.

Comment 18

[Martin Stránský [:stransky] (ni? me)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #17)

Do we need another bug then? The crashes from 109 still look like UAFs to me.

Yes please.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

(In reply to Martin Stránský [:stransky] (ni? me) from comment #18)

Yes please.

Filed bug 1824634.