Skip to content

Commit 12b3eda

Browse files
Reranko05kou
andauthored
GH-45187: [Ruby] Ensure initializing all rb_memory_view_t members (#50234)
### Rationale for this change `rb_memory_view_get()` callers may pass a non-zero-initialized `rb_memory_view_t`. `primitive_array_get()` and `buffer_get()` did not initialize `item_desc.components` and `item_desc.length`, which could cause `rb_memory_view_release()` to attempt to free an invalid pointer and abort the process. ### What changes are included in this PR? This change initializes `item_desc.components` and `item_desc.length` in both `primitive_array_get()` and `buffer_get()`. It also adds regression tests that verify releasing a memory view with a non-zero-initialized `rb_memory_view_t` does not crash for both `Arrow::Int32Array` and `Arrow::Buffer`. ### Are these changes tested? Yes. Added regression tests in `test-memory-view.rb` that reproduced the crash before this change and pass after the fix. ### Are there any user-facing changes? No. **This PR contains a "Critical Fix".** This change fixes a crash that could occur when `rb_memory_view_release()` is called with a memory view whose `item_desc` members were not initialized. * GitHub Issue: #45187 Lead-authored-by: Aaditya Srinivasan <aadityasri03@gmail.com> Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
1 parent f3f65df commit 12b3eda

2 files changed

Lines changed: 56 additions & 6 deletions

File tree

ruby/red-arrow/ext/arrow/memory-view.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
# undef private
3232
#endif
3333

34+
#include <cstring>
3435
#include <sstream>
3536

3637
namespace red_arrow {
@@ -220,6 +221,7 @@ namespace red_arrow {
220221
return false;
221222
}
222223
auto view_ = reinterpret_cast<memory_view *>(view);
224+
memset(view_, 0, sizeof(memory_view));
223225
view_->obj = obj;
224226
view_->private_data = new PrivateData();
225227
auto array = GARROW_ARRAY(RVAL2GOBJ(obj));
@@ -231,9 +233,6 @@ namespace red_arrow {
231233
}
232234
view_->readonly = true;
233235
view_->ndim = 1;
234-
view_->shape = NULL;
235-
view_->strides = NULL;
236-
view_->sub_offsets = NULL;
237236
return true;
238237
}
239238

@@ -258,6 +257,7 @@ namespace red_arrow {
258257
return false;
259258
}
260259
auto view_ = reinterpret_cast<memory_view *>(view);
260+
memset(view_, 0, sizeof(memory_view));
261261
view_->obj = obj;
262262
auto buffer = GARROW_BUFFER(RVAL2GOBJ(obj));
263263
auto arrow_buffer = garrow_buffer_get_raw(buffer);
@@ -275,9 +275,6 @@ namespace red_arrow {
275275
view_->byte_size = arrow_buffer->size();
276276
view_->readonly = true;
277277
view_->ndim = 1;
278-
view_->shape = NULL;
279-
view_->strides = NULL;
280-
view_->sub_offsets = NULL;
281278
return true;
282279
}
283280

ruby/red-arrow/test/test-memory-view.rb

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,4 +431,57 @@ def little_endian?
431431
])
432432
end
433433
end
434+
435+
sub_test_case("uninitialized rb_memory_view_t") do
436+
def setup
437+
libruby = Fiddle.dlopen(nil)
438+
439+
@rb_memory_view_get = Fiddle::Function.new(
440+
libruby["rb_memory_view_get"],
441+
[
442+
Fiddle::TYPE_UINTPTR_T,
443+
Fiddle::TYPE_VOIDP,
444+
Fiddle::TYPE_INT,
445+
],
446+
Fiddle::TYPE_BOOL
447+
)
448+
449+
@rb_memory_view_release = Fiddle::Function.new(
450+
libruby["rb_memory_view_release"],
451+
[
452+
Fiddle::TYPE_VOIDP,
453+
],
454+
Fiddle::TYPE_BOOL
455+
)
456+
end
457+
458+
def assert_release(target)
459+
# We should use sizeof(rb_memory_view_t) but it isn't available from Ruby.
460+
# 256 must be larger than sizeof(rb_memory_view_t).
461+
size = 256
462+
Fiddle::Pointer.malloc(size, Fiddle::RUBY_FREE) do |view|
463+
size.times do |i|
464+
view[i] = 0xAA
465+
end
466+
467+
assert do
468+
@rb_memory_view_get.call(Fiddle.dlwrap(target), view, 0)
469+
end
470+
471+
assert do
472+
@rb_memory_view_release.call(view)
473+
end
474+
end
475+
end
476+
477+
test("Int32Array") do
478+
array = Arrow::Int32Array.new([1, 2, 3, 4, 5])
479+
assert_release(array)
480+
end
481+
482+
test("Buffer") do
483+
buffer = Arrow::Buffer.new("hello")
484+
assert_release(buffer)
485+
end
486+
end
434487
end

0 commit comments

Comments
 (0)