Infinite loop in LhInstallHook if no memory can be found. #17

Closed
spazzarama opened this Issue Aug 15, 2015 · 0 comments

Comments

Projects
None yet
1 participant
@spazzarama
Member

spazzarama commented Aug 15, 2015

Moved from CodePlex
Originally submitted by jgg99

I had an issue where a call to LhInstallHook in my application would never return, and after investigation, it was looping infinitely in LhAllocateMemory here:

    // we are trying to get memory as near as possible to relocate most RIP-relative addressings
    for(Base = (LONGLONG)InEntryPoint, Index = 0; ; Index += PAGE_SIZE)
    {
        if(Base + Index < iEnd)
        {
            if((Res = (UCHAR*)VirtualAlloc((void*)(Base + Index), PAGE_SIZE, MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE)) != NULL)
                break;
        }

        if(Base - Index > iStart)
        {
            if((Res = (BYTE*)VirtualAlloc((void*)(Base - Index), PAGE_SIZE, MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE)) != NULL)
                break;
        }
    }

If VirtualAlloc never returns a valid address, it will never exit this loop. A simple fix would be to break the loop if neither of the "if" branches is entered:

    // we are trying to get memory as near as possible to relocate most RIP-relative addressings
    for(Base = (LONGLONG)InEntryPoint, Index = 0; ; Index += PAGE_SIZE)
    {
        BOOLEAN end = TRUE;
        if(Base + Index < iEnd)
        {
            if((Res = (UCHAR*)VirtualAlloc((void*)(Base + Index), PAGE_SIZE, MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE)) != NULL)
                break;
            end = FALSE;
        }

        if(Base - Index > iStart)
        {
            if((Res = (BYTE*)VirtualAlloc((void*)(Base - Index), PAGE_SIZE, MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE)) != NULL)
                break;
            end = FALSE;
        }

        if (end)
            break;
    }

As a suggestion for improvement, this loop could also be optimized by using VirtualQuery to walk through contiguous regions of memory pages, e.g. to avoid examining each page of a large allocation.

Thanks!

@spazzarama spazzarama added the bug label Aug 15, 2015

@spazzarama spazzarama added this to the 2.7 Stable milestone Aug 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment