Project

General

Profile

Feature #13245

Updated by shyouhei (Shyouhei Urabe) almost 5 years ago

Thread local and fiber local storages are by nature expected to be 
 visible from within the thread / fiber and not from anywhere else. OK? 

 That's a hoax. 

 The truth is they are visible from _anywhere_.    Thread local variable 
 for instance is just a set of key-value pair that is stored inside of 
 a thread instance variable.    No access control is ever attempted since 
 the beginning.    So you can touch any thread's any local storages at 
 will at any time without any synchronization. 

 This embarrasing "feature" has been there for a long time.    I thinnk 
 it's too late to remove the TLS accessor methods because they are used 
 everywhere.    BUT, even with that assumption, I strongly believe that 
 tampering other thread's local storage is something you must not. 

 Preventing such access should harm no one. 

 Signed-off-by: Urabe, Shyouhei <shyouhei@ruby-lang.org> 

 ```diff 
 --- 
  test/ruby/test_fiber.rb    | 7 +++++++ 
  test/ruby/test_thread.rb | 7 +++++++ 
  thread.c                   | 8 ++++++++ 
  3 files changed, 22 insertions(+) 

 diff --git a/test/ruby/test_fiber.rb b/test/ruby/test_fiber.rb 
 index d1d15828fc..319de9e7b2 100644 
 --- a/test/ruby/test_fiber.rb 
 +++ b/test/ruby/test_fiber.rb 
 @@ -169,6 +169,13 @@ def tvar(var, val) 
      assert_equal(nil, Thread.current[:v]); 
    end 
 
 +    def test_inter_thread_tls 
 +      t = Thread.new { } 
 +      assert_raise(ThreadError) do 
 +        t[:foo] = "bar" 
 +      end 
 +    end 
 + 
    def test_alive 
      fib = Fiber.new{Fiber.yield} 
      assert_equal(true, fib.alive?) 
 diff --git a/test/ruby/test_thread.rb b/test/ruby/test_thread.rb 
 index c82018dc8e..5ad663becf 100644 
 --- a/test/ruby/test_thread.rb 
 +++ b/test/ruby/test_thread.rb 
 @@ -95,6 +95,13 @@ def test_thread_variable_frozen 
      end 
    end 
 
 +    def test_inter_thread_variable 
 +      t = Thread.new { } 
 +      assert_raise(ThreadError) do 
 +        t.thread_variable_set(:foo, "bar") 
 +      end 
 +    end 
 + 
    def test_mutex_synchronize 
      m = Thread::Mutex.new 
      r = 0 
 diff --git a/thread.c b/thread.c 
 index 597fd293d6..03f7777aae 100644 
 --- a/thread.c 
 +++ b/thread.c 
 @@ -3154,6 +3154,10 @@ rb_thread_local_aset(VALUE thread, ID id, VALUE val) 
 	 rb_error_frozen("thread locals"); 
      } 
 
 +      if (th != GET_THREAD()) { 
 +          	 rb_raise(rb_eThreadError, "inter-thread access of TLS prohibited"); 
 +      } 
 + 
      return threadptr_local_aset(th, id, val); 
  } 
 
 @@ -3231,6 +3235,10 @@ rb_thread_variable_set(VALUE thread, VALUE id, VALUE val) 
 	 rb_error_frozen("thread locals"); 
      } 
 
 +      if (thread != GET_THREAD()->self) { 
 +          	 rb_raise(rb_eThreadError, "inter-thread access of TLS prohibited"); 
 +      } 
 + 
      locals = rb_ivar_get(thread, id_locals); 
      return rb_hash_aset(locals, rb_to_symbol(id), val); 
  } 
 --  
 2.11.1 
 ```

Back