From 713e99cec284a7bfdfdc6b598102028e0f844b8f Mon Sep 17 00:00:00 2001 From: mame Date: Wed, 17 Feb 2010 17:19:53 +0000 Subject: * io.c (io_fread, io_getpartial, io_read, io_sysread): by using lock, prohibit modification of buffer string during read (which had caused EFAULT or SEGV). [ruby-dev:40437] * test/ruby/test_io.rb: rewrite tests for the old behavior. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@26704 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 8 ++++++++ io.c | 18 +++++++----------- test/ruby/test_io.rb | 12 ++++++------ 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/ChangeLog b/ChangeLog index 932976b16a..90e6a5ed02 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +Thu Feb 18 02:14:26 2010 Yusuke Endoh + + * io.c (io_fread, io_getpartial, io_read, io_sysread): by using lock, + prohibit modification of buffer string during read (which had caused + EFAULT or SEGV). [ruby-dev:40437] + + * test/ruby/test_io.rb: rewrite tests for the old behavior. + Wed Feb 17 21:34:01 2010 Yusuke Endoh * regcomp.c (setup_tree, onig_compile): optimize .* at last by diff --git a/io.c b/io.c index 9983e17801..8375838836 100644 --- a/io.c +++ b/io.c @@ -1487,6 +1487,7 @@ io_fread(VALUE str, long offset, rb_io_t *fptr) long n = len; long c; + rb_str_locktmp(str); if (READ_DATA_PENDING(fptr) == 0) { while (n > 0) { again: @@ -1504,6 +1505,7 @@ io_fread(VALUE str, long offset, rb_io_t *fptr) if ((n -= c) <= 0) break; rb_thread_wait_fd(fptr->fd); } + rb_str_unlocktmp(str); return len - n; } @@ -1519,6 +1521,7 @@ io_fread(VALUE str, long offset, rb_io_t *fptr) break; } } + rb_str_unlocktmp(str); return len - n; } @@ -1810,18 +1813,15 @@ io_getpartial(int argc, VALUE *argv, VALUE io, int nonblock) if (!nonblock) READ_CHECK(fptr); - if (RSTRING_LEN(str) != len) { - modified: - rb_raise(rb_eRuntimeError, "buffer string modified"); - } n = read_buffered_data(RSTRING_PTR(str), len, fptr); if (n <= 0) { again: - if (RSTRING_LEN(str) != len) goto modified; if (nonblock) { rb_io_set_nonblock(fptr); } + rb_str_locktmp(str); n = rb_read_internal(fptr->fd, RSTRING_PTR(str), len); + rb_str_unlocktmp(str); if (n < 0) { if (!nonblock && rb_io_wait_readable(fptr->fd)) goto again; @@ -2136,9 +2136,6 @@ io_read(int argc, VALUE *argv, VALUE io) if (len == 0) return str; READ_CHECK(fptr); - if (RSTRING_LEN(str) != len) { - rb_raise(rb_eRuntimeError, "buffer string modified"); - } n = io_fread(str, 0, fptr); if (n == 0) { if (fptr->fd < 0) return Qnil; @@ -3841,11 +3838,10 @@ rb_io_sysread(int argc, VALUE *argv, VALUE io) n = fptr->fd; rb_thread_wait_fd(fptr->fd); rb_io_check_closed(fptr); - if (RSTRING_LEN(str) != ilen) { - rb_raise(rb_eRuntimeError, "buffer string modified"); - } + rb_str_locktmp(str); n = rb_read_internal(fptr->fd, RSTRING_PTR(str), ilen); + rb_str_unlocktmp(str); if (n == -1) { rb_sys_fail_path(fptr->pathv); diff --git a/test/ruby/test_io.rb b/test/ruby/test_io.rb index 6d7f94eaab..5fc3b133dd 100644 --- a/test/ruby/test_io.rb +++ b/test/ruby/test_io.rb @@ -823,15 +823,15 @@ class TestIO < Test::Unit::TestCase end) end - def test_readpartial_error + def test_readpartial_lock with_pipe do |r, w| s = "" t = Thread.new { r.readpartial(5, s) } 0 until s.size == 5 - s.clear + assert_raise(RuntimeError) { s.clear } w.write "foobarbaz" w.close - assert_raise(RuntimeError) { t.join } + assert_equal("fooba", t.value) end end @@ -858,15 +858,15 @@ class TestIO < Test::Unit::TestCase end) end - def test_read_error + def test_read_lock with_pipe do |r, w| s = "" t = Thread.new { r.read(5, s) } 0 until s.size == 5 - s.clear + assert_raise(RuntimeError) { s.clear } w.write "foobarbaz" w.close - assert_raise(RuntimeError) { t.join } + assert_equal("fooba", t.value) end end -- cgit v1.2.3