DynamoRIO / dynamorio

Dynamic Instrumentation Tool Platform
Other
2.62k stars 557 forks source link

indirect branch lookup should have barriers for either both mask and table or neither? #6393

Open derekbruening opened 11 months ago

derekbruening commented 11 months ago

Looking at the arm and aarch64 emit_indirect_branch_lookup() they both have synchronization to handle a concurrently updated hash mask, but nothing when loading the lookuptable address. update_lookuptabletls() uses a store-release when updating the mask -- but not the lookuptable. I'm not understanding that. Since these tables are all private anyway (-shard{bb,trace}_ibt_tables are false by default) it doesn't matter in default runs?? But if they're private why don't we avoid the barrier for the mask under those options? Shouldn't we either have barriers for both the mask and table or for neither?

ksco commented 11 months ago

Should we put the barrier on the lookuptable instead of the mask? e.g. for AArch64:

# emit_indirect_branch_lookup

ldr xx, [x28, hash_mask]
ldar xx, [x28, hash_table]

# update_lookuptable_tls

stlr xx, [x28, hash_table]
str xx, [x28, hash_mask]