Bug #22335
closedStack corruption with >31 keyword args and refined Integer#==
Description
This is a messy one that originally surfaced as a flaky test in our Rails app, and required a lot of AI-assisted debugging to track down. This bug appears in the latest 4.0.7, though all of the original investigation was done against 3.3.11.
If you have:
- a method that takes only keyword args
- has >31 keyword args
- has two call-time-evaluated default values (e.g.
{}) at argument index >=31 - running in a Ruby process that has refined or redefined
Integer#==at any point
Then you get inconsistent stack corruption depending on your process's hash salt.
The mechanism is nasty:
- In a method with only keyword args,
vm_callee_setup_arg()andvm_call_iseq_setup_kwparm_kwarg()callargs_setup_kw_parameters()withklocals = argv + kw_param->bits_start - kw_param->num, i.e. abovecfp->sp, without first extendingcfp->spover those locals args_setup_kw_parameters()uses a full hash, keyed by integer, for arguments beyond index 31- When
args_setup_kw_parameters()sets a key in that hash, and that key collides with another in the ar_hint that small hashes use for a linear scan (not uncommon for a tiny hash, but still salt-dependent), it triggers an integer comparison - The refinement on
Integer#==prevents that comparison from taking the fast path and turns it into a real C frame - Which gets pushed onto the stack, when the stack pointer hasn't been extended
- Thus overwriting the other args and producing garbage
The net effect is that the called method sees garbage values for the overwritten arguments.
I've attached a reproducing script, which is unfortunately complex, as it needs to do some work to intentionally hit the correct hash collision depending on the process's salt. A simpler static version doesn't reproduce reliably enough to be useful. Even this version will occasionally get a hash salt with no collisions and fail to reproduce, though that seems pretty rare on my machine.
Files
Updated by eapache_opslevel (Evan Huus) 3 days ago
- Description updated (diff)
Updated by eapache_opslevel (Evan Huus) 3 days ago
- Description updated (diff)
Updated by eapache_opslevel (Evan Huus) 2 days ago
- Description updated (diff)
Updated by nobu (Nobuyoshi Nakada) 1 day ago
- Backport changed from 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN to 3.3: REQUIRED, 3.4: REQUIRED, 4.0: REQUIRED
Updated by nobu (Nobuyoshi Nakada) 1 day ago
- Status changed from Open to Closed
Applied in changeset git|d87ffb271a80fb108de3465d12ada829f3c130af.
[Bug #22335] Use identity hashes for keyword argument tracking
With more than 31 keyword arguments, tracking unevaluated defaults
uses an integer-keyed Hash whose collisions can invoke methods when
Integer#== is refined or redefined, corrupting argument locals above
cfp->sp. Use identity hashes to prevent these calls during argument
setup and checkkeyword execution.
Making rb_any_cmp compare special constants directly would also
bypass redefined eql? methods in ordinary hashes. Limit identity
comparison to the internal keyword tracking hashes to preserve that
behavior for now.