Last Comment Bug 826171 - strndup implementation in memory/build/mozmemory_wrap.c is broken
: strndup implementation in memory/build/mozmemory_wrap.c is broken
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Memory Allocator (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla20
Assigned To: Mike Hommey [:glandium]
:
Mentors:
Depends on:
Blocks: 559263
  Show dependency treegraph
 
Reported: 2013-01-03 00:53 PST by Mike Hommey [:glandium]
Modified: 2013-03-19 08:24 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
affected
affected
fixed
affected
affected
affected
affected


Attachments
Fix strndup in memory/build/mozmemory_wrap.c (879 bytes, patch)
2013-01-03 00:55 PST, Mike Hommey [:glandium]
justin.lebar+bug: review+
Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2013-01-03 00:53:58 PST
That strndup definition, inherited from bug 559263, doesn't handle a partial copy of another string properly.
Comment 1 Mike Hommey [:glandium] 2013-01-03 00:55:20 PST
Created attachment 697358 [details] [diff] [review]
Fix strndup in memory/build/mozmemory_wrap.c
Comment 2 Justin Lebar (not reading bugmail) 2013-01-03 06:49:42 PST
Comment on attachment 697358 [details] [diff] [review]
Fix strndup in memory/build/mozmemory_wrap.c

o.O
Comment 4 Ed Morley [:emorley] 2013-01-04 10:02:04 PST
https://hg.mozilla.org/mozilla-central/rev/facbbe435906
Comment 5 Lukas Blakk [:lsblakk] use ?needinfo 2013-02-13 11:29:47 PST
Batch edit: Bugs marked status-b2g18: affected after 2/13 branching of v1.0.1 are now also status-b2g18-v1.0.1: affected
Comment 6 Kai Engert (:kaie) 2013-03-18 11:02:32 PDT
This sounds like the kind of bug that should get fixed everywhere. Should we request approval on the patch for most branches?
Comment 7 Stef Walter 2013-03-18 11:30:22 PDT
For reference, this affects modules loaded into a firefox process, which expect the libc implementation:

https://bugzilla.redhat.com/show_bug.cgi?id=922904
Comment 8 Justin Lebar (not reading bugmail) 2013-03-18 18:04:45 PDT
(In reply to Kai Engert (:kaie) from comment #6)
> This sounds like the kind of bug that should get fixed everywhere. Should we
> request approval on the patch for most branches?

I think we didn't take it on branches because we had no reason to believe that this fix had any effect on users, and that's a criterion that's usually applied to uplift requests.

If that belief is not true, I'd certainly be OK uplifting this to branches.
Comment 9 Stef Walter 2013-03-18 21:24:36 PDT
(In reply to Justin Lebar [:jlebar] from comment #8)
> (In reply to Kai Engert (:kaie) from comment #6)
> > This sounds like the kind of bug that should get fixed everywhere. Should we
> > request approval on the patch for most branches?
> 
> I think we didn't take it on branches because we had no reason to believe
> that this fix had any effect on users, and that's a criterion that's usually
> applied to uplift requests.
> 
> If that belief is not true, I'd certainly be OK uplifting this to branches.

Firefox exports these functions into the global symbol namespace. I'm not sure what the reasoning is for doing that. 

Because these functions override glibc functions, bugs in them have effects on any module loaded by firefox (imagine NPAPI plugin, PKCS#11 module, or binary add-on) making use of the given function. In this case the bug was seen when using the p11-kit-trust.so PKCS#11 module:

https://bugzilla.redhat.com/show_bug.cgi?id=922904
Comment 10 Kai Engert (:kaie) 2013-03-19 05:19:07 PDT
Comment on attachment 697358 [details] [diff] [review]
Fix strndup in memory/build/mozmemory_wrap.c

Proposing to fix on all active branches, in order to prevent random misbehaviour.
Comment 11 Scoobidiver (away) 2013-03-19 07:50:56 PDT
It's already in 20.0 Beta and 21.0a2.
Comment 12 Mike Hommey [:glandium] 2013-03-19 08:24:25 PDT
Comment on attachment 697358 [details] [diff] [review]
Fix strndup in memory/build/mozmemory_wrap.c

It's already on aurora, beta, and doesn't affect esr17 on linux, I'm pretty sure it doesn't affect esr17 on mac, and it probably doesn't affect esr17 on windows.

Note You need to log in before you can comment on or make changes to this bug.