Bug 1677202 Comment 5 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Magnus Melin [:mkmelin] from comment #4)
> https://searchfox.org/comm-central/rev/43a28c7a2eee7e5cd4ffecd6628450db2a4d9abe/mailnews/db/msgdb/src/nsMsgDatabase.cpp#88 well it's inside ifdef debug... so probably *very* hard to exploit ;)
> 

Right. I have noticed that by now. :-)

How does one clear the security flag?

> I suppose the if clause should be:
> 
> if (pMessageDB && pMessageDB->m_dbFile)

I found found out that things are not that simple.

I traced the values in #ifdef'ed code one by one.
It turns out pMessageDB points to an already freed heap area sometimes.
So pMessage->m_dbFile may not be null, but contains garbage.
This is timing-dependent. Sometimes the ASAN error is not triggered (!)
And that is why we can get crash SOMETIMES and not always, although in the mochitest flow, it seems always to happen at least on my PC.
However, if I run mochitest comm/mail/test/browser/keyboard/browser_spacehit.js by itself, the error does NOT happen(!).

So the issue is that there is a pointer to already freed structure in m_dbCache sometimes. Why?

I checked the code in  nsMsgDatabase::~nsMsgDatabase() 
and found that there is a following call in order to remove such entry in m_dbCache.
So this is probably not triggered all the time.

    static_cast<nsMsgDBService*>(serv.get())->RemoveFromCache(this);

Problem is that it is protected by if (serv), of course.
I have traced and indeed there are cases when the call to RemoveFromCache() does not happen due to 
|serv| is null.
I think this is because during shutdown phase, the XPCOM service has been shutdown and unavailable when we need it.

This is what I have figured out so far.

Maybe I can create a locally available code to clear such an entry in |m_dbCache|.
After all, we have routines to dump the entries in |m_dbCache| without going through XPCOM hoops. So maybe I can do it.
(In reply to Magnus Melin [:mkmelin] from comment #4)
> https://searchfox.org/comm-central/rev/43a28c7a2eee7e5cd4ffecd6628450db2a4d9abe/mailnews/db/msgdb/src/nsMsgDatabase.cpp#88 well it's inside ifdef debug... so probably *very* hard to exploit ;)
> 

Right. I have noticed that by now. :-)

How does one clear the security flag?

> I suppose the if clause should be:
> 
> if (pMessageDB && pMessageDB->m_dbFile)

I found out that things are not that simple.

I traced the values in #ifdef'ed code one by one.
It turns out pMessageDB points to an already freed heap area sometimes.
So pMessage->m_dbFile may not be null, but contains garbage.
This is timing-dependent. Sometimes the ASAN error is not triggered (!)
And that is why we can get crash SOMETIMES and not always, although in the mochitest flow, it seems always to happen at least on my PC.
However, if I run mochitest comm/mail/test/browser/keyboard/browser_spacehit.js by itself, the error does NOT happen(!).

So the issue is that there is a pointer to already freed structure in m_dbCache sometimes. Why?

I checked the code in  nsMsgDatabase::~nsMsgDatabase() 
and found that there is a following call in order to remove such entry in m_dbCache.
So this is probably not triggered all the time.

    static_cast<nsMsgDBService*>(serv.get())->RemoveFromCache(this);

Problem is that it is protected by if (serv), of course.
I have traced and indeed there are cases when the call to RemoveFromCache() does not happen due to 
|serv| is null.
I think this is because during shutdown phase, the XPCOM service has been shutdown and unavailable when we need it.

This is what I have figured out so far.

Maybe I can create a locally available code to clear such an entry in |m_dbCache|.
After all, we have routines to dump the entries in |m_dbCache| without going through XPCOM hoops. So maybe I can do it.

Back to Bug 1677202 Comment 5