Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • D dynamorio
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 1,467
    • Issues 1,467
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 44
    • Merge requests 44
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • DynamoRIO
  • dynamorio
  • Issues
  • #3780
Closed
Open
Issue created Aug 12, 2019 by John F.X. Galea@johnfxgaleaContributor

Bug found in drreg_statelessly_restore_app_value

There is a bug in drreg's drreg_statelessly_restore_app_value function, particularly when the user attempts to restore flag values.

In order to restore flag values, the user is required to pass DR_REG_NULL as the value for the reg parameter. However, due to this value, DynamoRIO gets confused and badly restores the xchg register with garbage data. This occurs when the flags register is dead or has native values, and the xchg register, namely XAX, is not used (i.e., set to DR_REG_NULL).

The culprit code is shown below. Since we passed DR_REG_NULL to restore eflags, and pt->aflags.xchg is DR_REG_NULL since it is not used, we enter this branch. This branch should be entered only when reg != NULL.

#ifdef X86
    if (pt->aflags.xchg == reg) {
        pt->slot_use[AFLAGS_SLOT] = DR_REG_XAX; /* appease assert */
        restore_reg(drcontext, pt, DR_REG_XAX, AFLAGS_SLOT, ilist, where_respill, false);
        pt->slot_use[AFLAGS_SLOT] = DR_REG_NULL;
        if (respill_needed != NULL)
            *respill_needed = true;
    } else
#endif

The following instrumentation, retrieved via Dynamorio logs (-loglevel 3) highlights the issue clearly. Notice how at 0x4a1aa8f8, drreg incorrectly restores XAX.

 +0    m4 @0x4a1aa71c  9f                   lahf
 +1    m4 @0x4a1aa93c  0f 90 c0             seto   al
 +4    m4 @0x4a1aa82c  64 89 15 ac 00 00 00 mov    dword ptr [fs:0x000000ac], edx
 +11   m4 @0x4a1a86f0  64 8b 15 74 00 00 00 mov    edx, dword ptr [fs:0x00000074]
 +18   m4 @0x4a1aa300  66 81 aa 80 a8 00 00 sub    word ptr [edx+0x0000a880], 0x0001
                       01 00
 +27   m4 @0x4a1a85bc  0f 85 fa ff ff ff    jnz    @0x4a1a9340
 +33   m4 @0x4a1a9f60  64 c7 05 78 00 00 00 mov    dword ptr [fs:0x00000078], @0x4a1a8650
                       d0 5b 1a 4a
 +44   m4 @0x4a1a9d0c  64 c7 05 7c 00 00 00 mov    dword ptr [fs:0x0000007c], 0xb7fdaa20
                       20 aa fd b7
 +55   m4 @0x4a1a92a0  64 a3 a4 00 00 00    mov    dword ptr [fs:0x000000a4], eax
 +61   m4 @0x4a1aab5c  3c 81                cmp    al, 0x81
 +63   m4 @0x4a1aa9c4  9e                   sahf
 +64   m4 @0x4a1a9db8  64 8b 15 ac 00 00 00 mov    edx, dword ptr [fs:0x000000ac]
 +71   m4 @0x4a1a8650                       <label>
 +71   m4 @0x4a1aa8f8  64 a1 a4 00 00 00    mov    eax, dword ptr [fs:0x000000a4]
 +77   m4 @0x4a1a9340                       <label>

PS: I also think that the passing of DR_REG_NULL to indicate the restoration of flag values is in itself bad practice. Ideally, we should have another function specifically for flag restoration. But this is another issue..

Assignee
Assign to
Time tracking