1510738 - Null check return value parameter in pthread_join redirection

Status: RESOLVED FIXED

(Core Graveyard :: Web Replay, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

pthread_join has a return value parameter which is allowed to be NULL, but its redirection writes to this parameter without any checks.  This is responsible for the crash below.

11/16 https://crash-stats.mozilla.com/report/index/f2dcff81-98b1-4f0b-b358-64bff0181116

Comment 1

[Logan Smyth [:loganfsmyth]]

Comment on attachment 9028414 [details] [diff] [review]
patch

Review of attachment 9028414 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/recordreplay/ProcessRedirectDarwin.cpp
@@ -780,1 @@
>    aArguments->Rval<ssize_t>() = 0;

The headers imply that these return values are `int` types. Is the `ssize_t` here right?

Comment 2

[Brian Hackett [Laid off!]]

(In reply to Logan Smyth [:loganfsmyth] from comment #1)
> Comment on attachment 9028414 [details] [diff] [review]
> patch
> 
> Review of attachment 9028414 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/recordreplay/ProcessRedirectDarwin.cpp
> @@ -780,1 @@
> >    aArguments->Rval<ssize_t>() = 0;
> 
> The headers imply that these return values are `int` types. Is the `ssize_t`
> here right?

The aArguments accessors require that when arguments or return values are interpreted as being of some type, that type must use the full eight bytes allocated for the argument/return value.  This ensures that writes to these arguments/return values will completely fill them in.  Doing so isn't actually necessary here, as the 'int' return type means that only four bytes of the return value will be used.  Filling in all eight bytes won't hurt anything, however.

Comment 3

[Logan Smyth [:loganfsmyth]]

Cool, thanks for the info. Hope you don't mind the questions every once in a while.

Comment 4

[Brian Hackett [Laid off!]]

(In reply to Logan Smyth [:loganfsmyth] from comment #3)
> Cool, thanks for the info. Hope you don't mind the questions every once in a
> while.

No worries, I'm happy to answer any questions you have.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa55c733298c
Null check return value parameter in pthread_join redirection, r=lsmyth.

Comment 6

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/aa55c733298c