Project

General

Profile

Actions

Bug #22259

closed

Arrays sharding a buffer segfault when concatenating with each other

Bug #22259: Arrays sharding a buffer segfault when concatenating with each other

Added by chucke (Tiago Cardoso) about 21 hours ago. Updated 8 minutes ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:126480]

Description

A succession of crashes was observed on a particular workload from our app, involving recursive set operations, after upgrading it to ruby 4.0.6 . LLM was used to build a reproduction (shared in the description of the PR linked below), after which we used it as a harness to get to the actual culprit and fix it.

The crash is due to a particular behaviour of how (btw, TIL, so correct my explanation if necessary) ruby arrays share internal buffers, for optimal memory usage. Array#dup creates a separate array object, but may share (under certain conditions, i.e. does not embed) the internal C buffer, where objects are stored, with it. Certain modification operations, like Array#dup, don't forcefully "copy on write", and instead bump/dec an index on the shared buffer, as so to determine the slice of the original shared buffer it points to.

The issue happens with how other operations work, such as Array#concat, which is, internally, under certain conditions, copying the full shared buffer to the destination array buffer, without regards of which slice it is about. This is easily demonstrable by the first reproduction in the PR linked below, where the last concat operation adds a "phantom" element to the destination array. that element is, at that point, gibberish, malloc-uninitialized data, which causes the VM to crash, depending on what it does with the address found there.

This bug has been there for a while, and can be reproduced (with different faulty outcomes) since ruby 3.3 at least (didn't try further back).

PR with fix here: https://github.com/ruby/ruby/pull/18472

Updated by mame (Yusuke Endoh) about 14 hours ago Actions #1 [ruby-core:126484]

Confirmed and reproduced. Thanks for the report.

While looking into it I found that the same line causes another bug.

a = [:A, :B, :C, :D, :E]
b = a[1, 4]      #=> [:B, :C, :D, :E]
a[0, 0] = b

p a  # expected: [:B, :C, :D, :E, :A, :B, :C, :D, :E]
     # actual:   [:B, :C, :D, :A, :A, :B, :C, :D, :E]

It silently returns a wrong element. It happens whenever the source starts after beg, and dates back to 2329d8b0de4.

I think the simple fix is to stop guessing from the pointer and let the callers say whether the source and the destination are the same array.

I opened a PR: https://github.com/ruby/ruby/pull/18481

Updated by chucke (Tiago Cardoso) about 9 hours ago 1Actions #2 [ruby-core:126486]

Thx for the patch, much cleaner than mine. Indeed I thought there must have been a cleaner fix, but I admit I was out of my depth.

Updated by mame (Yusuke Endoh) about 5 hours ago Actions #3

  • Status changed from Open to Closed

Applied in changeset git|46c287e2ca39da63d5164e2b7227308a41554c9a.


Fix splicing an array that shares its buffer with a longer array

Arrays sharing one buffer have the same data pointer, so rb_ary_splice()
took a in b = a.dup; b.pop; b.concat(a) for b itself and rebased
the source onto b's shorter storage, copying uninitialized memory.
Have the callers tell whether the replacement really is ary.

[Bug #22259]

Co-Authored-By: Claude Opus 5 (1M context)

Updated by byroot (Jean Boussier) 8 minutes ago Actions #4 [ruby-core:126491]

  • Backport changed from 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN to 3.3: WONTFIX, 3.4: REQUIRED, 4.0: REQUIRED

This bug seem to have been present all the way back since Ruby 2.5, so requesting a backport accordingly.

Actions

Also available in: PDF Atom