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

liballoc_system's unix __rust_reallocate doesn't handle allocation failures for over-aligned types #32993

Closed
froydnj opened this issue Apr 15, 2016 · 2 comments

Comments

@froydnj
Copy link
Contributor

froydnj commented Apr 15, 2016

The Unix implementation of __rust_reallocate in liballoc_system boils down to:

    pub unsafe fn reallocate(ptr: *mut u8, old_size: usize, size: usize, align: usize) -> *mut u8 {
        if align <= MIN_ALIGN {
            libc::realloc(ptr as *mut libc::c_void, size as libc::size_t) as *mut u8
        } else {
            let new_ptr = allocate(size, align);
            ptr::copy(ptr, new_ptr, cmp::min(size, old_size));
            deallocate(ptr, old_size, align);
            new_ptr
        }
    }

The else branch of the conditional doesn't properly handle the case where allocate fails. For comparison's sake, the Windows implementation says:

    pub unsafe fn reallocate(ptr: *mut u8, _old_size: usize, size: usize, align: usize) -> *mut u8 {
        if align <= MIN_ALIGN {
            HeapReAlloc(GetProcessHeap(), 0, ptr as LPVOID, size as SIZE_T) as *mut u8
        } else {
            let header = get_header(ptr);
            let new = HeapReAlloc(GetProcessHeap(),
                                  0,
                                  header.0 as LPVOID,
                                  (size + align) as SIZE_T) as *mut u8;
            if new.is_null() {
                return new;
            }
            align_ptr(new, align)
        }
    }
alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 15, 2016
The Unix implementation was incorrectly handling failure for reallocation of
over-aligned types by not checking for NULL.

Closes rust-lang#32993
@alexcrichton
Copy link
Member

Oh dear, seems bad!

Out of curiosity, was this discovered from reading or from a bug? We could consider a backport to beta if that's necessary

@froydnj
Copy link
Contributor Author

froydnj commented Apr 15, 2016

This was discovered when reading through liballoc* to write a custom memory allocator for Firefox. I suspect it's hard to make it trigger due to overcommit policies and the general lack of usage of over-aligned types.

Manishearth added a commit to Manishearth/rust that referenced this issue Apr 15, 2016
…id-this-land, r=nagisa

alloc_system: Handle failure properly

The Unix implementation was incorrectly handling failure for reallocation of
over-aligned types by not checking for NULL.

Closes rust-lang#32993
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants