io_buffer.c: Allow copies between overlapping buffers with #copy and #set_string (#11640)

The current implementation of `IO::Buffer#copy` and `#set_string` has
an undefined behavior when the source and destination memory overlaps,
due to the underlying use of the `memcpy` C function.

This patch guarantees the methods to be safe even when copying between
overlapping buffers by replacing `memcpy` with `memmove`,

Fixes: [Bug #20745]
This commit is contained in:
Kasumi Hanazuki 2024-11-07 05:55:39 +09:00 committed by GitHub
parent 56ecc243e2
commit 35bf660337
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
Notes: git 2024-11-06 20:55:58 +00:00
Merged-By: ioquatix <samuel@codeotaku.com>
2 changed files with 70 additions and 5 deletions

View file

@ -2328,7 +2328,7 @@ io_buffer_set_values(VALUE self, VALUE buffer_types, VALUE _offset, VALUE values
}
static void
io_buffer_memcpy(struct rb_io_buffer *buffer, size_t offset, const void *source_base, size_t source_offset, size_t source_size, size_t length)
io_buffer_memmove(struct rb_io_buffer *buffer, size_t offset, const void *source_base, size_t source_offset, size_t source_size, size_t length)
{
void *base;
size_t size;
@ -2340,7 +2340,9 @@ io_buffer_memcpy(struct rb_io_buffer *buffer, size_t offset, const void *source_
rb_raise(rb_eArgError, "The computed source range exceeds the size of the source buffer!");
}
memcpy((unsigned char*)base+offset, (unsigned char*)source_base+source_offset, length);
if (length != 0) {
memmove((unsigned char*)base+offset, (const unsigned char*)source_base+source_offset, length);
}
}
// (offset, length, source_offset) -> length
@ -2377,7 +2379,7 @@ io_buffer_copy_from(struct rb_io_buffer *buffer, const void *source_base, size_t
length = source_size - source_offset;
}
io_buffer_memcpy(buffer, offset, source_base, source_offset, source_size, length);
io_buffer_memmove(buffer, offset, source_base, source_offset, source_size, length);
return SIZET2NUM(length);
}
@ -2420,7 +2422,7 @@ rb_io_buffer_initialize_copy(VALUE self, VALUE source)
* copy(source, [offset, [length, [source_offset]]]) -> size
*
* Efficiently copy from a source IO::Buffer into the buffer, at +offset+
* using +memcpy+. For copying String instances, see #set_string.
* using +memmove+. For copying String instances, see #set_string.
*
* buffer = IO::Buffer.new(32)
* # =>
@ -2469,6 +2471,19 @@ rb_io_buffer_initialize_copy(VALUE self, VALUE source)
* buffer = IO::Buffer.new(2)
* buffer.copy(IO::Buffer.for('test'), 0)
* # in `copy': Specified offset+length is bigger than the buffer size! (ArgumentError)
*
* It is safe to copy between memory regions that overlaps each other.
* In such case, the data is copied as if the data was first copied from the source buffer to
* a temporary buffer, and then copied from the temporary buffer to the destination buffer.
*
* buffer = IO::Buffer.new(10)
* buffer.set_string("0123456789")
* buffer.copy(buffer, 3, 7)
* # => 7
* buffer
* # =>
* # #<IO::Buffer 0x000056494f8ce440+10 INTERNAL>
* # 0x00000000 30 31 32 30 31 32 33 34 35 36 0120123456
*/
static VALUE
io_buffer_copy(int argc, VALUE *argv, VALUE self)
@ -2530,7 +2545,7 @@ io_buffer_get_string(int argc, VALUE *argv, VALUE self)
* call-seq: set_string(string, [offset, [length, [source_offset]]]) -> size
*
* Efficiently copy from a source String into the buffer, at +offset+ using
* +memcpy+.
* +memmove+.
*
* buf = IO::Buffer.new(8)
* # =>

View file

@ -633,4 +633,54 @@ class TestIOBuffer < Test::Unit::TestCase
end
end
end
def test_copy_overlapped_fwd
buf = IO::Buffer.for('0123456789').dup
buf.copy(buf, 3, 7)
assert_equal '0120123456', buf.get_string
end
def test_copy_overlapped_bwd
buf = IO::Buffer.for('0123456789').dup
buf.copy(buf, 0, 7, 3)
assert_equal '3456789789', buf.get_string
end
def test_copy_null_destination
buf = IO::Buffer.new(0)
assert_predicate buf, :null?
buf.copy(IO::Buffer.for('a'), 0, 0)
assert_predicate buf, :empty?
end
def test_copy_null_source
buf = IO::Buffer.for('a').dup
src = IO::Buffer.new(0)
assert_predicate src, :null?
buf.copy(src, 0, 0)
assert_equal 'a', buf.get_string
end
def test_set_string_overlapped_fwd
str = +'0123456789'
IO::Buffer.for(str) do |buf|
buf.set_string(str, 3, 7)
end
assert_equal '0120123456', str
end
def test_set_string_overlapped_bwd
str = +'0123456789'
IO::Buffer.for(str) do |buf|
buf.set_string(str, 0, 7, 3)
end
assert_equal '3456789789', str
end
def test_set_string_null_destination
buf = IO::Buffer.new(0)
assert_predicate buf, :null?
buf.set_string('a', 0, 0)
assert_predicate buf, :empty?
end
end