Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jl_calloc is subject to unchecked unsigned integer wrapping #42673

Closed
mkitti opened this issue Oct 16, 2021 · 1 comment · Fixed by #42761 · May be fixed by #42704
Closed

jl_calloc is subject to unchecked unsigned integer wrapping #42673

mkitti opened this issue Oct 16, 2021 · 1 comment · Fixed by #42761 · May be fixed by #42704

Comments

@mkitti
Copy link
Contributor

mkitti commented Oct 16, 2021

There is the opportunity for an unchecked unsigned integer wrapping
in the exported jl_calloc:

julia/src/gc.c

Lines 3464 to 3466 in 0ea61fa

JL_DLLEXPORT void *jl_calloc(size_t nm, size_t sz)
{
size_t nmsz = nm*sz;

julia> Libc.calloc(0xffffffffffffffff,  0xffffffffffffffff)
Ptr{Nothing} @0x0000000000000000

julia> ccall(:jl_calloc,Ptr{Nothing},(Csize_t, Csize_t), 0xffffffffffffffff, 0xffffffffffffffff)
Ptr{Nothing} @0x000000001ebd1180

My expectation is that jl_calloc should return C_NULL like Libc.calloc.

@mkitti mkitti changed the title jl_calloc is subject to overflow jl_calloc is subject to unchecked unsigned integer wrapping Oct 17, 2021
mkitti added a commit to mkitti/julia that referenced this issue Oct 18, 2021
Fix JuliaLang#42673 by checking for unsigned integer wrapping for jl_*calloc*
Reused former isaligned tagged bit for howtofree tagged bit
Retained all existing functions
jl_gc_*_aligned defaults to jl_* since macOS guarantees page alignment
offset alignment to hold original (void *) p0, and size_t align

commit 842bf06e785d3be97558083abe181505c6840549
Author: Mark Kittisopikul <[email protected]>
Date:   Sun Oct 17 02:20:11 2021 -0400

    Undo renaming of jl_calloc, jl_free, and jl_realloc

commit 9594400d207a9f8baac832af5dd0830165d1192e
Author: Daniel Matz <[email protected]>
Date:   Fri Nov 11 18:31:54 2016 -0600

    Remove the isaligned array flag

commit 4bb083a5f6b44766632dd394397257ebafc91726
Author: Daniel Matz <[email protected]>
Date:   Fri Nov 4 08:59:43 2016 -0500

    Disambiguate jl_malloc from jl_malloc_aligned

    Add gc counted, aligned malloc
mkitti added a commit to mkitti/julia that referenced this issue Oct 18, 2021
Fix JuliaLang#42673 by checking for unsigned integer wrapping for jl_*calloc*
Reused former isaligned tagged bit for howtofree tagged bit
Retained all existing functions
jl_gc_*_aligned defaults to jl_* since macOS guarantees page alignment
offset alignment to hold original (void *) p0, and size_t align

commit 842bf06e785d3be97558083abe181505c6840549
Author: Mark Kittisopikul <[email protected]>
Date:   Sun Oct 17 02:20:11 2021 -0400

    Undo renaming of jl_calloc, jl_free, and jl_realloc

commit 9594400d207a9f8baac832af5dd0830165d1192e
Author: Daniel Matz <[email protected]>
Date:   Fri Nov 11 18:31:54 2016 -0600

    Remove the isaligned array flag

commit 4bb083a5f6b44766632dd394397257ebafc91726
Author: Daniel Matz <[email protected]>
Date:   Fri Nov 4 08:59:43 2016 -0500

    Disambiguate jl_malloc from jl_malloc_aligned

    Add gc counted, aligned malloc
mkitti added a commit to mkitti/julia that referenced this issue Oct 18, 2021
Fix JuliaLang#42673 by checking for unsigned integer wrapping for jl_*calloc*
Reused former isaligned tagged bit for howtofree tagged bit
Retained all existing functions
jl_gc_*_aligned defaults to jl_* since macOS guarantees page alignment
offset alignment to hold original (void *) p0, and size_t align

commit 842bf06e785d3be97558083abe181505c6840549
Author: Mark Kittisopikul <[email protected]>
Date:   Sun Oct 17 02:20:11 2021 -0400

    Undo renaming of jl_calloc, jl_free, and jl_realloc

commit 9594400d207a9f8baac832af5dd0830165d1192e
Author: Daniel Matz <[email protected]>
Date:   Fri Nov 11 18:31:54 2016 -0600

    Remove the isaligned array flag

commit 4bb083a5f6b44766632dd394397257ebafc91726
Author: Daniel Matz <[email protected]>
Date:   Fri Nov 4 08:59:43 2016 -0500

    Disambiguate jl_malloc from jl_malloc_aligned

    Add gc counted, aligned malloc
mkitti added a commit to mkitti/julia that referenced this issue Oct 19, 2021
commit 927da5f067f51700c1d5ffb22304382710b1643b
Author: Mark Kittisopikul <[email protected]>
Date:   Tue Oct 19 05:52:50 2021 -0400

    Export jl_gc_*_aligned, jl_gc_managed_calloc

commit ce011a82cba044f88a962b6da24a02bd105fed3e
Author: Mark Kittisopikul <[email protected]>
Date:   Mon Oct 18 13:13:09 2021 -0400

    Simplify, reduce diff versus master

commit af4cf94520cc2c9fd5008b929c6e4ef2cd55bca3
Author: Mark Kittisopikul <[email protected]>
Date:   Mon Oct 18 03:40:07 2021 -0400

    Mixed and working

Fix JuliaLang#42673 by checking for unsigned integer wrapping for jl_*calloc*
Reused former isaligned tagged bit for howtofree tagged bit
Retained all existing functions
jl_gc_*_aligned defaults to jl_* since macOS guarantees page alignment
offset alignment to hold original (void *) p0, and size_t align

commit 842bf06e785d3be97558083abe181505c6840549
Author: Mark Kittisopikul <[email protected]>
Date:   Sun Oct 17 02:20:11 2021 -0400

    Undo renaming of jl_calloc, jl_free, and jl_realloc

commit 9594400d207a9f8baac832af5dd0830165d1192e
Author: Daniel Matz <[email protected]>
Date:   Fri Nov 11 18:31:54 2016 -0600

    Remove the isaligned array flag

commit 4bb083a5f6b44766632dd394397257ebafc91726
Author: Daniel Matz <[email protected]>
Date:   Fri Nov 4 08:59:43 2016 -0500

    Disambiguate jl_malloc from jl_malloc_aligned

    Add gc counted, aligned malloc
@mkitti
Copy link
Contributor Author

mkitti commented Oct 20, 2021

My proposed fix is

//_unchecked_calloc does not check for potential overflow of nm*sz
static inline void *_unchecked_calloc(size_t nm, size_t sz) {
    size_t nmsz = nm*sz;
    int64_t *p = (int64_t *)jl_gc_counted_calloc(nmsz + JL_SMALL_BYTE_ALIGNMENT, 1);
    if (p == NULL)
        return NULL;
    p[0] = nmsz;
    return (void *)(p + 2); // assumes JL_SMALL_BYTE_ALIGNMENT == 16
}

JL_DLLEXPORT void *jl_calloc(size_t nm, size_t sz)
{
    if (nm > SIZE_MAX/sz)
        return NULL;
    return _unchecked_calloc(nm, sz);
}

This is implemented in #42704, but we may want to apply a smaller patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant