@RyanVM: Yup, that rationale seems fine. I think checking if the new update mitigates the fragmentation should be pretty easy: we can run a simpler version of the two tests listed in Bug 1739761 > Test 1. Without the patch. For example bg requires about 23.26mb in the sandbox to perform spellchecking. > - When allocating a 16mb sandbox, spellchecking uses 11.67mb out of 16mb used and then crashes with OOM > - When allocating a 32mb sandbox, spellchecking uses 22.53mb out of 32mb used and then crashes with OOM > - When allocating a 64mb sandbox, spellchecking uses 23.26mb out of 64mb used and succeeds > > Test 2. With the patch > - This reduces the number of wasm pages allocated from 64 to 47 on en_us builds, corresponding to about 1 MB of reduced memory usage. Documenting this in case it comes up in the future 1. Add the following after the `sandbox->create`, i.e., after https://searchfox.org/firefox-main/source/extensions/spellcheck/hunspell/glue/RLBoxHunspell.cpp#68 ``` std::cout << "!!!!!!!!!!!Hunspell RLBox for locale: " << dpath.get() << "\n"; ``` 2. Add the following at the bottom of `RLBoxHunspell::spell`, i.e., after https://searchfox.org/firefox-main/source/extensions/spellcheck/hunspell/glue/RLBoxHunspell.cpp#184 ``` size_t sz = mSandbox->get_total_memory(); std::cout << "!!!!!!!!!!!Hunspell RLBox Size: " << sz << " bytes\n"; ``` 3. Open a webpage like below with a textbox and right click to enable check spelling first, and choose the language under "Languages" with the English language then with the Bulgarian language. (you may need to install the required language dictionary extensions from the Mozilla store) ``` <!DOCTYPE html> <html> <head> <meta charset='utf-8'> <meta http-equiv='X-UA-Compatible' content='IE=edge'> <title>Hunspell test</title> <meta name='viewport' content='width=device-width, initial-scale=1'> </head> <body> <input type="text" value="Hello world!" /> </body> </html> ``` 4. Update hunspell to the new version without the patch from Bug 1739761 and repeat steps 1 to 3 5. If you see that the numbers look similar, then the upstream version no longer needs the memory arena patch 6. If there is a difference, we may need to spend time to figure out what to do
Bug 2037089 Comment 1 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
@RyanVM: Yup, that rationale seems fine. I think checking if the new update mitigates the fragmentation should be pretty easy: we can run a simpler version of the two tests listed in Bug 1739761 > Test 1. Without the patch. For example bg requires about 23.26mb in the sandbox to perform spellchecking. > - When allocating a 16mb sandbox, spellchecking uses 11.67mb out of 16mb used and then crashes with OOM > - When allocating a 32mb sandbox, spellchecking uses 22.53mb out of 32mb used and then crashes with OOM > - When allocating a 64mb sandbox, spellchecking uses 23.26mb out of 64mb used and succeeds > > Test 2. With the patch > - This reduces the number of wasm pages allocated from 64 to 47 on en_us builds, corresponding to about 1 MB of reduced memory usage. Documenting this in case it comes up in the future 1. Add the following after the `sandbox->create`, i.e., after https://searchfox.org/firefox-main/source/extensions/spellcheck/hunspell/glue/RLBoxHunspell.cpp#68 ``` std::cout << "!!!!!!!!!!!Hunspell RLBox for locale: " << dpath.get() << "\n"; ``` 2. Add the following at the bottom of `RLBoxHunspell::spell`, i.e., after https://searchfox.org/firefox-main/source/extensions/spellcheck/hunspell/glue/RLBoxHunspell.cpp#184 ``` size_t sz = mSandbox->get_total_memory(); std::cout << "!!!!!!!!!!!Hunspell RLBox Size: " << sz << " bytes, " << sz / (64 * 1024) << " Wasm pages\n"; ``` 3. Open a webpage like below with a textbox and right click to enable check spelling first, and choose the language under "Languages" with the English language then with the Bulgarian language. (you may need to install the required language dictionary extensions from the Mozilla store) ``` <!DOCTYPE html> <html> <head> <meta charset='utf-8'> <meta http-equiv='X-UA-Compatible' content='IE=edge'> <title>Hunspell test</title> <meta name='viewport' content='width=device-width, initial-scale=1'> </head> <body> <input type="text" value="Hello world!" /> </body> </html> ``` 4. Update hunspell to the new version without the patch from Bug 1739761 and repeat steps 1 to 3 5. If you see that the numbers look similar, then the upstream version no longer needs the memory arena patch 6. If there is a difference, we may need to spend time to figure out what to do
@RyanVM: Yup, that rationale seems fine. The rlbox sandbox will mitigate the vulnerabilities but we should try to update asap. I think checking if the new update mitigates the fragmentation should be pretty easy: we can run a simpler version of the two tests listed in Bug 1739761 > Test 1. Without the patch. For example bg requires about 23.26mb in the sandbox to perform spellchecking. > - When allocating a 16mb sandbox, spellchecking uses 11.67mb out of 16mb used and then crashes with OOM > - When allocating a 32mb sandbox, spellchecking uses 22.53mb out of 32mb used and then crashes with OOM > - When allocating a 64mb sandbox, spellchecking uses 23.26mb out of 64mb used and succeeds > > Test 2. With the patch > - This reduces the number of wasm pages allocated from 64 to 47 on en_us builds, corresponding to about 1 MB of reduced memory usage. Documenting this in case it comes up in the future 1. Add the following after the `sandbox->create`, i.e., after https://searchfox.org/firefox-main/source/extensions/spellcheck/hunspell/glue/RLBoxHunspell.cpp#68 ``` std::cout << "!!!!!!!!!!!Hunspell RLBox for locale: " << dpath.get() << "\n"; ``` 2. Add the following at the bottom of `RLBoxHunspell::spell`, i.e., after https://searchfox.org/firefox-main/source/extensions/spellcheck/hunspell/glue/RLBoxHunspell.cpp#184 ``` size_t sz = mSandbox->get_total_memory(); std::cout << "!!!!!!!!!!!Hunspell RLBox Size: " << sz << " bytes, " << sz / (64 * 1024) << " Wasm pages\n"; ``` 3. Open a webpage like below with a textbox and right click to enable check spelling first, and choose the language under "Languages" with the English language then with the Bulgarian language. (you may need to install the required language dictionary extensions from the Mozilla store) ``` <!DOCTYPE html> <html> <head> <meta charset='utf-8'> <meta http-equiv='X-UA-Compatible' content='IE=edge'> <title>Hunspell test</title> <meta name='viewport' content='width=device-width, initial-scale=1'> </head> <body> <input type="text" value="Hello world!" /> </body> </html> ``` 4. Update hunspell to the new version without the patch from Bug 1739761 and repeat steps 1 to 3 5. If you see that the numbers look similar, then the upstream version no longer needs the memory arena patch 6. If there is a difference, we may need to spend time to figure out what to do
@RyanVM: Yup, that rationale seems fine. The rlbox sandbox will mitigate the vulnerabilities but we should try to update asap. I think checking if the new update mitigates the fragmentation should be pretty easy: we can run a simpler version of the two tests listed in Bug 1739761 > Test 1. Without the patch. For example bg requires about 23.26mb in the sandbox to perform spellchecking. > - When allocating a 16mb sandbox, spellchecking uses 11.67mb out of 16mb used and then crashes with OOM > - When allocating a 32mb sandbox, spellchecking uses 22.53mb out of 32mb used and then crashes with OOM > - When allocating a 64mb sandbox, spellchecking uses 23.26mb out of 64mb used and succeeds > > Test 2. With the patch > - This reduces the number of wasm pages allocated from 64 to 47 on en_us builds, corresponding to about 1 MB of reduced memory usage. Documenting this in case it comes up in the future 1. Add the following after the `sandbox->create`, i.e., after https://searchfox.org/firefox-main/source/extensions/spellcheck/hunspell/glue/RLBoxHunspell.cpp#68 ``` std::cout << "!!!!!!!!!!!Hunspell RLBox for locale: " << dpath.get() << "\n"; ``` 2. Add the following at the bottom of `RLBoxHunspell::spell`, i.e., after https://searchfox.org/firefox-main/source/extensions/spellcheck/hunspell/glue/RLBoxHunspell.cpp#184 ``` size_t sz = mSandbox->get_total_memory(); std::cout << "!!!!!!!!!!!Hunspell RLBox Size: " << sz << " bytes, " << sz / (64 * 1024) << " Wasm pages\n"; ``` 3. Open a webpage like below with a textbox and right click to enable check spelling first, and choose the language under "Languages" with the English language then with the Bulgarian language. (you may need to install the required language dictionary extensions from the Mozilla store) ``` <!DOCTYPE html> <html> <head> <meta charset='utf-8'> <meta http-equiv='X-UA-Compatible' content='IE=edge'> <title>Hunspell test</title> <meta name='viewport' content='width=device-width, initial-scale=1'> </head> <body> <input type="text" value="Hello world!" /> </body> </html> ``` 4. Update hunspell to the new version without the patch from Bug 1739761 and repeat steps 1 to 3 5. If you see that the numbers look similar, then the upstream version no longer needs the memory arena patch. If there is a difference, we may need to spend time to figure out what to do 6. Note we usually collect the "number of page" values from the opt build. The Wasm code when compiled without optimizations is much slower and heavier than the optimized versions.