Null check return value parameter in pthread_join redirection

RESOLVED FIXED in Firefox 65

Status

()

defect
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: bhackett, Assigned: bhackett)

Tracking

(Blocks 1 bug)

unspecified
mozilla65
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(1 attachment)

Posted patch patchSplinter Review
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
Attachment #9028414 - Flags: review?(lsmyth)
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?
Attachment #9028414 - Flags: review?(lsmyth) → review+
(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.
Cool, thanks for the info. Hope you don't mind the questions every once in a while.
(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.
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa55c733298c
Null check return value parameter in pthread_join redirection, r=lsmyth.
https://hg.mozilla.org/mozilla-central/rev/aa55c733298c
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.