826171 - strndup implementation in memory/build/mozmemory_wrap.c is broken

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect)

Description

[Mike Hommey [:glandium]]

That strndup definition, inherited from bug 559263, doesn't handle a partial copy of another string properly.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Fix strndup in memory/build/mozmemory_wrap.c

Comment 2

[Justin Lebar (not reading bugmail)]

Comment on attachment 697358 [details] [diff] [review]
Fix strndup in memory/build/mozmemory_wrap.c

o.O

Comment 3

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/facbbe435906

Comment 4

[Ed Morley [:emorley]]

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

Comment 5

[Lukas Blakk [:lsblakk] use ?needinfo]

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:)]

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]

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)]

(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]

(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:)]

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)]

It's already in 20.0 Beta and 21.0a2.

Comment 12

[Mike Hommey [:glandium]]

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.