From: "ioquatix (Samuel Williams) via ruby-core" <ruby-core@...>
Date: 2024-11-07T20:27:40+00:00
Subject: [ruby-core:119821] [Ruby master Bug#20863] `zlib.c` calls `rb_str_set_len` and `rb_str_modify_expand`(and others) without holding the GVL.

Issue #20863 has been updated by ioquatix (Samuel Williams).


I think the issue is, those methods from a public interface POV, are not allowed to be called without the GVL.

Even if today the implementation follows a "safe" code path, in the future, it may not.

Adding these annotations will help to clarify that "this method is not safe to call without the GVL" - a form of internal and run-time documentation.

----------------------------------------
Bug #20863: `zlib.c` calls `rb_str_set_len` and `rb_str_modify_expand`(and others) without holding the GVL.
https://bugs.ruby-lang.org/issues/20863#change-110509

* Author: ioquatix (Samuel Williams)
* Status: Open
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
## Background

I was working on https://bugs.ruby-lang.org/issues/20876 and was investigating some problems with `zlib.c` and GVL, and noticed that `zstream_run_func` is executed without the GVL, but can invoke various `rb_` string functions. Those functions in turn can invoke `rb_raise` and generally look problematic. However, maybe by luck, such code path does not appear to be invoked in typical usage.

However, even so, it is possible to cause `zstream_run_func` to segfault by a carefully crafted program which causes the internal buffer to be resized while the GVL is released: https://github.com/ruby/zlib/pull/88#issuecomment-2455772054

## Proposal

I would like to modify `zlib.c` to only release the GVL around the CPU intensive compression/decompression operation: https://github.com/ruby/zlib/pull/88

In addition, I identified several more improvements to prevent segfaults and other related failures:

- Use `rb_str_locktemp` to prevent the `z->buf` changing size while in use by the `rb_nogvl` code.
- Expand the mutex to protect `#deflate` and `#inflate` completely, not just the internal operation.

In order to catch these issues earlier and find other bugs like this, I recommend we introduce additional checks: https://bugs.ruby-lang.org/issues/20877



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/