1482248 - folder name with special characters can cause coredump on startup before UI appears with thunderbird-60

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Richard PALO]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180808222917

Steps to reproduce:

Archlinux updated thunderbird from 52.9.1 to 60.0
when starting tb, I encountered coredump as shown in https://bugs.archlinux.org/task/59585

in my profiles Mail/Local Folders directory, there is a directory named:
'Ma'$'\356''tres d'\''Ouvrages.sbd' which should be "Maîtres d'Ouvrages.sbd"

I'm running french locale.


Actual results:

[calBackendLoader] Using Thunderbird's builtin libical backend
Erreur de segmentation (core dumped)



Expected results:

should probably ignore the file or at least signal a warning rather than coredumping.
tb 52.9.1 seemed to simply ignore the file.

deleting the directory permits tb to start up correctly.
Not sure if this is somewhat related to https://bugzilla.mozilla.org/show_bug.cgi?id=416888

Comment 1

[Wayne Mery (:wsmwk)]

Can you reproduce with download from https://www.thunderbird.net/en-US/ ?

Comment 2

[Wayne Mery (:wsmwk)]

Confirmed with karlmag on IRC - his crash is bp-bf530a4b-cced-4302-80da-ae2140180810 mozilla::detail::nsTStringRepr<T>::First

he says "I've experimented with moving away dirs containing special chars and when they are all gone thunderbird starts. If I put a random one back it crashes."

Top 10 frames of crashing thread:

0 libxul.so mozilla::detail::nsTStringRepr<char16_t>::First xpcom/string/nsTSubstring.cpp:848
1 libxul.so nsMsgLocalStoreUtils::nsShouldIgnoreFile comm/mailnews/local/src/nsMsgLocalStoreUtils.cpp:31
2 libxul.so nsMsgBrkMBoxStore::AddSubFolders comm/mailnews/local/src/nsMsgBrkMBoxStore.cpp:1063
3 libxul.so nsMsgBrkMBoxStore::DiscoverSubFolders comm/mailnews/local/src/nsMsgBrkMBoxStore.cpp:69
4 libxul.so nsMsgLocalMailFolder::GetSubFolders comm/mailnews/local/src/nsLocalMailFolder.cpp:226
5 libxul.so nsMsgDBFolder::ListFoldersWithFlags comm/mailnews/base/util/nsMsgDBFolder.cpp:4474
6 libxul.so nsMsgDBFolder::GetFoldersWithFlags comm/mailnews/base/util/nsMsgDBFolder.cpp:4462
7 libxul.so NS_InvokeByIndex 
8 libxul.so XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:1951
9 libxul.so XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:913

=============================================================

Possible match with topcrash  libxul.so@0xc71601 | libxul.so@0xb9dfef | libxul.so@0x4728d77 | libxul.so@0x4715935 | libxul.so@0x4715691 | libxul.so@0xb9c61c | libxul.so@0xa156cf | libxul.so@0x473d943 | libxul.so@0xcd50ec | libxul.so@0xa1567c | libxul.so@0x65d5a9f | libxul.so@0xb9c8...
where user of bp-8fc650cf-abb2-4b11-9907-6c1540180808 reports "Crash directly at startup. No window visible, instant crash."

Comment 3

[Magnus Melin [:mkmelin]]

Attachment: bug1482248_folder_name_empty.patch

It's pretty clear it crashes due to and empty name. Why that name is empty... I don't know.

Comment 4

[Jorg K (CEST = GMT+2)]

Comment on attachment 8999262 [details] [diff] [review]
bug1482248_folder_name_empty.patch

Maybe a fallout from bug 1440278?

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/80754eac3809
don't crash on empty file name in nsMsgLocalStoreUtils::nsShouldIgnoreFile. r=jorgk

Comment 6

[Masatoshi Kimura [:emk]]

That's bug 1340577:
https://hg.mozilla.org/mozilla-central/rev/3e552501cd1f#l7.15
https://hg.mozilla.org/mozilla-central/rev/3e552501cd1f#l8.14

Before bug 1340577, the assertion was debug-only and was not even fatal. This change just revealed the potential bug in c-c.
Bug 1340577 was fixed since mozilla54, so Tb 52.9.1 was not affected.

Comment 7

[Wayne Mery (:wsmwk)]

I'll let you guys duke out which bug is at fault.

Earliest crash examples are 
20180104080056 58.0b3 bp-4295c8c6-1763-439b-87ca-923000180303
20180209193141 59.0b1 bp-00236ae6-9bb7-4a47-8bde-f9e5f0180307

Comment 8

[Masatoshi Kimura [:emk]]

So bug 1440278 (fixed since Tb 60) is ruled out.

Comment 9

[Masatoshi Kimura [:emk]]

nsMsgBrkMBoxStore::AddSubFolders calls nsIFile::GetLeafName.[1]
On Unix, GetLeafName will convert the file path using NS_CopyNativeToUnicode.[2][3]
On Unix, NS_CopyNativeToUnicode is just an alias of CopyUTF8toUTF16.[4]
CopyUTF8toUTF16 will fail if the input contains a byte sequence that is not valid as UTF-8.[5]
So it will end up with passing a empty name to nsShouldIgnoreFile.

We shoud check a return value from GetLeafName instead of wallpapering the problem in nsShouldIgnoreFile.

[1] https://dxr.mozilla.org/comm-central/rev/e5e1510b8d914bfa8439b21ba3f73e4f2e83e957/mailnews/local/src/nsMsgBrkMBoxStore.cpp#1058
[2] https://dxr.mozilla.org/comm-central/rev/fe17acc6e291e54463db3ea82697c714ae5a4b27/mozilla/xpcom/io/nsLocalFileUnix.cpp#2103
[3] https://dxr.mozilla.org/comm-central/rev/fe17acc6e291e54463db3ea82697c714ae5a4b27/mozilla/xpcom/io/nsLocalFileUnix.cpp#2072
[4] https://dxr.mozilla.org/comm-central/rev/fe17acc6e291e54463db3ea82697c714ae5a4b27/mozilla/xpcom/io/nsNativeCharsetUtils.cpp#19
[5] https://dxr.mozilla.org/mozilla-central/rev/4e56a2f51ad739ca52046723448f3129a58f1666/xpcom/string/nsReadableUtils.cpp#379-382

Comment 10

[Masatoshi Kimura [:emk]]

(In reply to Masatoshi Kimura [:emk] from comment #9)
> We shoud check a return value from GetLeafName instead of wallpapering the
> problem in nsShouldIgnoreFile.

Sorry, this is wrong. Since NS_CopyNativeToUnicode will never fail, the error check will not work. Maybe it is an an m-c bug? So the empty name check would be fine for given that we will need uplifting.

Comment 11

[Jorg K (CEST = GMT+2)]

Masatoshi-san, thanks for the analysis. We recognise that the patch papers over a deeper problem, but it's a stopgap for now.

I guess to find the source of the problem, someone on Linux would have to do some experiments to see where the empty string comes from. Magnus?

If I read nsMsgLocalStoreUtils::nsShouldIgnoreFile() correctly, an empty string would even have been *accepted* since it doesn't match any of the conditions.

Comment 12

[Masatoshi Kimura [:emk]]

As I said, the empty string comes from the failed GetLeafName call. Although NS_CopyNativeToUnicode will never fail, it means it just swallow any errors on failure even though the result is trucated. So the fundamental fix would be either

1. Fix NS_CopyNativeToUnicode so that it returns failures on error.
2. Check the return value from GetLeafName.

or

Use GetNativeLeafName on Unix-like systems.

Comment 13

[Masatoshi Kimura [:emk]]

(In reply to Jorg K (GMT+2) from comment #11)
> If I read nsMsgLocalStoreUtils::nsShouldIgnoreFile() correctly, an empty
> string would even have been *accepted* since it doesn't match any of the
> conditions.

It is purely by accident. Because .First() would have lead an undefined behavior if the string is empty. Bug 1340577 made the error more explicit.

Comment 14

[Magnus Melin [:mkmelin]]

> 1. Fix NS_CopyNativeToUnicode so that it returns failures on error.

Sounds like the real fix - but that's a large change.

Comment 15

[Jorg K (CEST = GMT+2)]

Comment on attachment 8999262 [details] [diff] [review]
bug1482248_folder_name_empty.patch

I still think we should follow up in a new bug. Checking the status of GetLeafName() or using GetNativeLeafName() on Linux doesn't appear to be hard.

Comment 16

[Jorg K (CEST = GMT+2)]

TB 60.1 ESR:
https://hg.mozilla.org/releases/comm-esr60/rev/7802e1dbc3f26a0ddcb5efa14dfce919f3b7ea21

Comment 17

[Richard PALO]

I'm waiting to test this latest patchset on arch (hopefully thunderbird-60.0-4, but with thunderbird-60.0-2 I still have issues...

Namely, where -2 finally works on x86_64 locally, it cores running nfs4 on aarch64 with:
           PID: 5271 (thunderbird)
           UID: 1001 (richard)
           GID: 100 (users)
        Signal: 11 (SEGV)
     Timestamp: Mon 2018-08-20 11:44:38 CEST (2h 37min ago)
  Command Line: /usr/lib/thunderbird/thunderbird
    Executable: /usr/lib/thunderbird/thunderbird
 Control Group: /user.slice/user-1001.slice/session-c2.scope
          Unit: session-c2.scope
         Slice: user-1001.slice
       Session: c2
     Owner UID: 1001 (richard)
       Boot ID: 8bfb9b23e36d468e8e6d32a089adb36d
    Machine ID: a3cf6c7e75e14060a929d42784885d41
      Hostname: odroid-001e06336dd6
       Storage: /var/lib/systemd/coredump/core.thunderbird.1001.8bfb9b23e36d468e8e6d32a089adb36d.5271.1534758278000000.lz4
       Message: Process 5271 (thunderbird) of user 1001 dumped core.
                
                Stack trace of thread 5271:
                #0  0x0000ffffada876a4 raise (libpthread.so.0)
                #1  0x0000ffffa9b90970 n/a (libxul.so)
                #2  0x0000ffffaa0877b0 n/a (libxul.so)
                #3  0x0000ffffadb0164c n/a (linux-vdso.so.1)
                #4  0x0000ffffadb0164c n/a (linux-vdso.so.1)
                #5  0x0000ffffa9c41ae0 n/a (libxul.so)
                #6  0x0000000000000003 n/a (n/a)
                #7  0x0000000000000003 n/a (n/a)
                #8  0x0000aaaacdc6e8ac _start (thunderbird)

So I ask specifically to test on nfs filesystem (best with both nfs3 & nfs4) as well as on at least {x86, x86_64 and aarch64} clients.

Comment 18

[Richard PALO]

seems I'm able to run thunderbird-60.0-4 on aarch64 with --safe-mode to avoid dumping core.

Is there environment variable I can set to trace where things foul up without --safe-mode
at least in order to see if it's specifically related to this bug or something else?

Comment 19

[Richard PALO]

take that back, still dumped even with --safe-mode (but after I got the main window, which is somewhat different):
           PID: 2055 (thunderbird)
           UID: 1001 (richard)
           GID: 100 (users)
        Signal: 11 (SEGV)
     Timestamp: Mon 2018-08-20 17:37:40 CEST (9min ago)
  Command Line: /usr/lib/thunderbird/thunderbird --new-instance --no-remote --safe-mode
    Executable: /usr/lib/thunderbird/thunderbird
 Control Group: /user.slice/user-1001.slice/session-c3.scope
          Unit: session-c3.scope
         Slice: user-1001.slice
       Session: c3
     Owner UID: 1001 (richard)
       Boot ID: 8e5a30ffcf5046c89f7c6a983c3c4f76
    Machine ID: a3cf6c7e75e14060a929d42784885d41
      Hostname: odroid-001e06336dd6
       Storage: /var/lib/systemd/coredump/core.thunderbird.1001.8e5a30ffcf5046c89f7c6a983c3c4f76.2055.1534779460000000.lz4
       Message: Process 2055 (thunderbird) of user 1001 dumped core.
                
                Stack trace of thread 2055:
                #0  0x0000ffff84f526a4 raise (libpthread.so.0)
                #1  0x0000ffff8105d6b0 n/a (libxul.so)
                #2  0x0000ffff8155c330 n/a (libxul.so)
                #3  0x0000ffff84fcc64c n/a (linux-vdso.so.1)
                #4  0x0000ffff84fcc64c n/a (linux-vdso.so.1)
                #5  0x0000ffff815b90ec n/a (libxul.so)
                #6  0x0000ffffca810a18 n/a (n/a)
                #7  0x0000ffffca810a18 n/a (n/a)