Skip to content

Commit ac4e3cd

Browse files
committed
Merge branch 'PHP-8.5'
* PHP-8.5: Fix GH-18139: Memory leak when overriding some settings via readline_info()
2 parents 8c4f806 + d9b02e4 commit ac4e3cd

File tree

2 files changed

+41
-21
lines changed

2 files changed

+41
-21
lines changed

ext/readline/readline.c

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ static zval _prepped_callback;
4747
static zval _readline_completion;
4848
static zval _readline_array;
4949

50+
ZEND_TLS char *php_readline_custom_readline_name = NULL;
51+
#if defined(PHP_WIN32) || defined(HAVE_LIBEDIT)
52+
ZEND_TLS char *php_readline_custom_line_buffer = NULL;
53+
#endif
54+
5055
PHP_MINIT_FUNCTION(readline);
5156
PHP_MSHUTDOWN_FUNCTION(readline);
5257
PHP_RSHUTDOWN_FUNCTION(readline);
@@ -146,7 +151,6 @@ PHP_FUNCTION(readline_info)
146151
zend_string *what = NULL;
147152
zval *value = NULL;
148153
size_t oldval;
149-
char *oldstr;
150154

151155
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|S!z!", &what, &value) == FAILURE) {
152156
RETURN_THROWS();
@@ -181,35 +185,29 @@ PHP_FUNCTION(readline_info)
181185
add_assoc_long(return_value,"attempted_completion_over",rl_attempted_completion_over);
182186
} else {
183187
if (zend_string_equals_literal_ci(what,"line_buffer")) {
184-
oldstr = strdup(rl_line_buffer ? rl_line_buffer : "");
188+
RETVAL_STRING(SAFE_STRING(rl_line_buffer));
185189
if (value) {
186190
if (!try_convert_to_string(value)) {
187191
RETURN_THROWS();
188192
}
193+
/* XXX: These stores would need to be atomic ideally or use a memory barrier */
189194
#if !defined(PHP_WIN32) && !defined(HAVE_LIBEDIT)
190-
if (!rl_line_buffer) {
191-
rl_line_buffer = malloc(Z_STRLEN_P(value) + 1);
192-
} else if (strlen(oldstr) < Z_STRLEN_P(value)) {
193-
rl_extend_line_buffer(Z_STRLEN_P(value) + 1);
194-
free(oldstr);
195-
oldstr = strdup(rl_line_buffer ? rl_line_buffer : "");
195+
rl_extend_line_buffer(Z_STRLEN_P(value) + 1);
196+
if (EXPECTED(rl_line_buffer)) {
197+
memcpy(rl_line_buffer, Z_STRVAL_P(value), Z_STRLEN_P(value) + 1);
196198
}
197-
memcpy(rl_line_buffer, Z_STRVAL_P(value), Z_STRLEN_P(value) + 1);
198199
#else
199-
char *tmp = strdup(Z_STRVAL_P(value));
200-
if (tmp) {
201-
if (rl_line_buffer) {
202-
free(rl_line_buffer);
203-
}
204-
rl_line_buffer = tmp;
200+
char *copy = strdup(Z_STRVAL_P(value));
201+
rl_line_buffer = copy;
202+
if (php_readline_custom_line_buffer) {
203+
free(php_readline_custom_line_buffer);
205204
}
205+
php_readline_custom_line_buffer = copy;
206206
#endif
207207
#if !defined(PHP_WIN32)
208208
rl_end = Z_STRLEN_P(value);
209209
#endif
210210
}
211-
RETVAL_STRING(SAFE_STRING(oldstr));
212-
free(oldstr);
213211
} else if (zend_string_equals_literal_ci(what, "point")) {
214212
RETVAL_LONG(rl_point);
215213
#ifndef PHP_WIN32
@@ -268,15 +266,19 @@ PHP_FUNCTION(readline_info)
268266
RETVAL_STRING((char *)SAFE_STRING(rl_library_version));
269267
#endif
270268
} else if (zend_string_equals_literal_ci(what, "readline_name")) {
271-
oldstr = (char*)rl_readline_name;
269+
RETVAL_STRING(SAFE_STRING(rl_readline_name));
272270
if (value) {
273-
/* XXX if (rl_readline_name) free(rl_readline_name); */
274271
if (!try_convert_to_string(value)) {
275272
RETURN_THROWS();
276273
}
277-
rl_readline_name = strdup(Z_STRVAL_P(value));
274+
char *copy = strdup(Z_STRVAL_P(value));
275+
/* XXX: This store would need to be atomic ideally or use a memory barrier */
276+
rl_readline_name = copy;
277+
if (php_readline_custom_readline_name) {
278+
free(php_readline_custom_readline_name);
279+
}
280+
php_readline_custom_readline_name = copy;
278281
}
279-
RETVAL_STRING(SAFE_STRING(oldstr));
280282
} else if (zend_string_equals_literal_ci(what, "attempted_completion_over")) {
281283
oldval = rl_attempted_completion_over;
282284
if (value) {

ext/readline/tests/gh18139.phpt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
GH-18139 (Memory leak when overriding some settings via readline_info())
3+
--EXTENSIONS--
4+
readline
5+
--FILE--
6+
<?php
7+
8+
var_dump(readline_info('readline_name', 'first'));
9+
var_dump(readline_info('readline_name', 'second'));
10+
var_dump(readline_info('line_buffer', 'third'));
11+
var_dump(readline_info('line_buffer', 'fourth'));
12+
13+
?>
14+
--EXPECTF--
15+
string(%d) "%S"
16+
string(5) "first"
17+
string(%d) "%S"
18+
string(5) "third"

0 commit comments

Comments
 (0)