1895081 - (CVE-2024-6603) Free on address which was not realloced on pthread_attr_destroy (after realloc failure on pthread_getattr_np)

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Irvan Kurniawan [:sourc7]]

Attachment: asan-free-which-not-realloced-pthread_attr_destroy.txt

When running Firefox to run new Worker repeatedly subsequently with fillMemory to fill the virtual memory, after a while it able to crash at pthread_attr_destroy -> free

I observe the crash occur after realloc failure on nsThread::InitCommon() at pthread_getattr_np line. From libc code, the realloc failure occur on following __pthread_attr_setaffinity_np code:

=> void *newp = realloc (iattr->extension->cpuset, cpusetsize);
if (newp == NULL)
  return ENOMEM;

On nsThread::InitCommon() after pthread_getattr_np(pthread_self(), &attr); is called, after some code, it then run pthread_attr_destroy(&attr); which run free (iattr->extension->cpuset); which was not realloced.

Fix suggestion is to check pthread_getattr_np(pthread_self(), &attr); for non-zero return code or ENOMEM error.

Comment 1

[Irvan Kurniawan [:sourc7]]

https://searchfox.org/mozilla-central/source/xpcom/threads/nsThread.cpp#459

Comment 2

[Daniel Veditz [:dveditz]]

Is this crashing in ASAN code itself? (because no memory, presumably). What happens in a release or debug build?

Comment 3

[Irvan Kurniawan [:sourc7]]

(In reply to Daniel Veditz [:dveditz] from comment #2)

Is this crashing in ASAN code itself? (because no memory, presumably).

Sorry Dan for the delay, the atomic_compare_exchange_strong is often indicate "attempting free on address which was not malloc()-ed", e.g. on bug 1893270 I also encounter atomic_compare_exchange_strong on ASAN logs, as [:dholbert] also encounter on comment 6.

On bug 1893270 when try reproduce that on ASan 32-bit, surprisingly it show the correct report, which is "attempting free on address which was not malloc()-ed", that's reason I attached ASan 32-bit log on that bug.

Tried to do the same on ASan 32-bit, but still no luck for this bug. However on Valgrind build, it show the "Invalid read of size 8" and "Invalid free() / delete / delete[] / realloc()" which points to pthread_attr_destroy.

What happens in a release or debug build?

The crash at pthread_attr_destroy is hard to trigger, I able to trigger the crash on official Nightly 32-bit build, but very rarely, the CPU register show interesting address, but unfortunately I only managed to capture SEGV crash on 0x00000000.

To reliably trigger the crash, I have to modify glibc code pthread_attr_setaffinity.c void *newp to return NULL at runCount more than 300 (e.g. 400, 500) to make sure it doesn't hit Firefox startup code. After installed the modified glibc to chroot or VM, then on the modified glibc Firefox Nightly 32-bit crash stack look similar, interestingly the SEGV crash address is able to hit multiple (changing address) e.g. 0x05864605, 0x53746567, 0x6572726f, 0x706c6568, 0x8b240489, and more.

On modified glibc I also captured SEGV crash on 0x00000000, which similar to what I capture earlier, made me think that it still possible to capture SEGV on interesting crash address (e.g. 0x6572726f, 0x706c6568, 0x8b240489) on original glibc, but very difficult to reproduce naturally. I'll attach the captured stack in a moment.

Comment 4

[Irvan Kurniawan [:sourc7]]

Attachment: minidump1.txt

Comment 5

[Irvan Kurniawan [:sourc7]]

Attachment: minidump2.txt

Comment 6

[Irvan Kurniawan [:sourc7]]

Attachment: modified_glibc_minidump1.txt

Comment 7

[Irvan Kurniawan [:sourc7]]

Attachment: modified_glibc_minidump2.txt

Comment 8

[Irvan Kurniawan [:sourc7]]

Attachment: modified_glibc_minidump3.txt

Comment 9

[Irvan Kurniawan [:sourc7]]

Attachment: modified_glibc_minidump_hitzero1.txt

Comment 10

[Irvan Kurniawan [:sourc7]]

Attachment: modified_glibc_minidump_hitzero2.txt

Comment 11

[Irvan Kurniawan [:sourc7]]

Attachment: modified_glibc_valgrind1.txt

Comment 12

[Irvan Kurniawan [:sourc7]]

Attachment: modified_glibc_valgrind2.txt

Comment 13

[Irvan Kurniawan [:sourc7]]

Attachment: modified-glibc-pthread.patch

After applied the patch, I followed Arch Linux glibc official PKGBUILD build command flags https://gitlab.archlinux.org/archlinux/packaging/packages/glibc/-/blob/main/PKGBUILD?ref_type=heads to compile glibc 64-bit and 32-bit, then run make DESTDIR=/var/chroot1 install to install the modified glibc to destination chroot or VM root folder.

After modified glibc was installed, I visit attached workerinterval.test.html, after a seconds Firefox tab is crashed at pthread_attr_destroy as attached.

Comment 14

[Irvan Kurniawan [:sourc7]]

Attachment: workerinterval.test.html

Comment 15

[Nika Layzell [:nika] (ni? for response)]

Seems like we should add some checks which do a MOZ_CRASH if any of the pthread_* calls in nsThread::InitCommon fail (https://searchfox.org/mozilla-central/rev/a18a7c526cf3c531f2fc24db4f0dffbc16290a7e/xpcom/threads/nsThread.cpp#457-459,462,480,507). Not sure it's worth trying to recover in a situation where these are OOM-ing.

Comment 16

[Irvan Kurniawan [:sourc7]]

Attachment: log_minidump_0x2c617461.txt

(In reply to Irvan Kurniawan (:sourc7) from comment #3)

made me think that it still possible to capture SEGV on interesting crash address (e.g. 0x6572726f, 0x706c6568, 0x8b240489) on original glibc

Yes, it's possible. I able to capture SIGSEGV / SEGV_ACCERR at interesting address 0x2c617461 on Firefox Nightly 32-bit Official buildId 20240517215359 on original glibc.

Comment 17

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1895081 - Add extra assertions to nsThread initialization, r=#xpcom-reviewers!

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c24bd37e841
Add extra assertions to nsThread initialization, r=xpcom-reviewers,jstutte

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7c24bd37e841

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for ESR115 approval when you get a chance.

Comment 21

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 9405088 [details]
Bug 1895081 - Add extra assertions to nsThread initialization, r=#xpcom-reviewers!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: low risk sec bug
• User impact if declined: potential crash/uaf if specific OOM conditions are met
• Fix Landed on Version: 128
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Adding release-mode assertions re: pthread call failure

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9405088 [details]
Bug 1895081 - Add extra assertions to nsThread initialization, r=#xpcom-reviewers!

Approved for 115.13esr.

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/65c457647e34

Comment 24

[Simon Friedberger [:simonf]]

Attachment: advisory.txt