Closed Bug 1389052 Opened 7 years ago Closed 7 years ago

aligned_alloc emulation loses track of original allocation address

Categories

(NSS :: Libraries, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ueno, Assigned: ueno)

Details

Attachments

(1 file)

In the following commits:

https://hg.mozilla.org/projects/nss/rev/e45bc5e967aa#l2.28
https://hg.mozilla.org/projects/nss/rev/cd068f7ce6ae#l7.990

aligned_alloc is emulated with the following code:

    /* aligned_alloc is C11 so we have to do it the old way. */
    ghash = PORT_ZAlloc(sizeof(gcmHashContext) + 15);
    if (ghash == NULL) {
        PORT_SetError(SEC_ERROR_NO_MEMORY);
        goto loser;
    }
    ghash->mem = ghash; /* (1) */
    ghash = (gcmHashContext *)(((uintptr_t)ghash + 15) & ~(uintptr_t)0x0F);

On a platform where pointer alignment is smaller than 8 (for example, on i686, it is 4), ghash can point to a middle of the allocated block and ghash->mem set on (1) will be cleared.  That results in memory leaks, as reported by Kamil Dudka in:
https://bugzilla.redhat.com/show_bug.cgi?id=1479881

The attached patch fixes it by setting ->mem after moving the pointer to the desired alignment boundary.
Attachment #8895786 - Flags: review?(franziskuskiefer)
Comment on attachment 8895786 [details] [diff] [review]
nss-aligned-alloc.patch

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

I didn't know 686 still existed... Thanks for the fix.
Attachment #8895786 - Flags: review?(franziskuskiefer) → review+
Assignee: nobody → dueno
Target Milestone: --- → 3.33
https://hg.mozilla.org/projects/nss/rev/8019abedc833059e7333aadb269a2412fe6a14f5
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Thank both of you for fixing it!
Franziskus, I wonder if we shouldn't create a PORT_ZNewAligned(type, alignment, memptr) macro and function.  It might be paired with a PORT_ZFreeAligned(var, memptr) macro.
That's a good idea. I filed bug 1390129 to do this.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: