Skip to content

PHPC-1274: Do not allow forked children to affect parent resources #1059

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 62 additions & 3 deletions php_phongo.c
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ void phongo_session_init(zval* return_value, mongoc_client_session_t* client_ses

session = Z_SESSION_OBJ_P(return_value);
session->client_session = client_session;
session->client = mongoc_client_session_get_client(client_session);
}
/* }}} */

Expand Down Expand Up @@ -2623,8 +2624,8 @@ static void php_phongo_persist_client(const char* hash, size_t hash_len, mongoc_
{
php_phongo_pclient_t* pclient = (php_phongo_pclient_t*) pecalloc(1, sizeof(php_phongo_pclient_t), 1);

pclient->pid = (int) getpid();
pclient->client = client;
pclient->created_by_pid = (int) getpid();
pclient->client = client;

#if PHP_VERSION_ID >= 70000
zend_hash_str_update_ptr(&MONGODB_G(pclients), hash, hash_len, pclient);
Expand Down Expand Up @@ -2879,13 +2880,71 @@ PHP_INI_BEGIN()
PHP_INI_END()
/* }}} */

static void phongo_pclient_reset_once(php_phongo_pclient_t* pclient, int pid)
{
if (pclient->last_reset_by_pid != pid) {
mongoc_client_reset(pclient->client);
pclient->last_reset_by_pid = pid;
}
}

/* Resets the libmongoc client if it has not already been reset for the current
* PID (based on information in the hash table of persisted libmongoc clients).
* This ensures that we do not reset a client multiple times from the same child
* process. */
void php_phongo_client_reset_once(mongoc_client_t* client, int pid)
{
HashTable* pclients;

TSRMLS_FETCH();
pclients = &MONGODB_G(pclients);

#if PHP_VERSION_ID >= 70000
{
zval* z_ptr;
php_phongo_pclient_t* pclient;

ZEND_HASH_FOREACH_VAL(pclients, z_ptr)
{
if ((Z_TYPE_P(z_ptr) != IS_PTR)) {
continue;
}

pclient = (php_phongo_pclient_t*) Z_PTR_P(z_ptr);

if (pclient->client == client) {
phongo_pclient_reset_once(pclient, pid);
return;
}
}
ZEND_HASH_FOREACH_END();
}
#else
{
HashPosition pos;
php_phongo_pclient_t** pclient;

for (
zend_hash_internal_pointer_reset_ex(pclients, &pos);
zend_hash_get_current_data_ex(pclients, (void**) &pclient, &pos) == SUCCESS;
zend_hash_move_forward_ex(pclients, &pos)) {

if ((*pclient)->client == client) {
phongo_pclient_reset_once((*pclient), pid);
return;
}
}
}
#endif
}

static inline void php_phongo_pclient_destroy(php_phongo_pclient_t* pclient)
{
/* Do not destroy mongoc_client_t objects created by other processes. This
* ensures that we do not shutdown sockets that may still be in use by our
* parent process (see: CDRIVER-2049). While this is a leak, we are already
* in MSHUTDOWN at this point. */
if (pclient->pid == getpid()) {
if (pclient->created_by_pid == getpid()) {
mongoc_client_destroy(pclient->client);
}

Expand Down
18 changes: 17 additions & 1 deletion php_phongo.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ extern zend_module_entry mongodb_module_entry;
* forking). We avoid using pid_t for Windows compatibility. */
typedef struct {
mongoc_client_t* client;
int pid;
int created_by_pid;
int last_reset_by_pid;
} php_phongo_pclient_t;

ZEND_BEGIN_MODULE_GLOBALS(mongodb)
Expand Down Expand Up @@ -168,6 +169,8 @@ bool php_phongo_parse_int64(int64_t* retval, const char* data, phongo_zpp_char_l
zend_bool phongo_writeerror_init(zval* return_value, bson_t* bson TSRMLS_DC);
zend_bool phongo_writeconcernerror_init(zval* return_value, bson_t* bson TSRMLS_DC);

void php_phongo_client_reset_once(mongoc_client_t* client, int pid);

#if PHP_VERSION_ID >= 70000
#define PHONGO_CE_FINAL(ce) \
do { \
Expand Down Expand Up @@ -217,6 +220,19 @@ zend_bool phongo_writeconcernerror_init(zval* return_value, bson_t* bson TSRMLS_
#define PHONGO_ZVAL_EXCEPTION_NAME(e) (ZSTR_VAL(Z_OBJCE_P(e)->name))
#endif

#define PHONGO_SET_CREATED_BY_PID(intern) \
do { \
(intern)->created_by_pid = (int) getpid(); \
} while (0);

#define PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern) \
do { \
int pid = (int) getpid(); \
if ((intern)->created_by_pid != pid) { \
php_phongo_client_reset_once((intern)->client, pid); \
} \
} while (0);

#endif /* PHONGO_H */

/*
Expand Down
5 changes: 5 additions & 0 deletions php_phongo_structs.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ typedef struct {
PHONGO_ZEND_OBJECT_PRE
mongoc_cursor_t* cursor;
mongoc_client_t* client;
int created_by_pid;
uint32_t server_id;
bool advanced;
php_phongo_bson_state visitor_data;
Expand All @@ -78,6 +79,7 @@ typedef struct {
typedef struct {
PHONGO_ZEND_OBJECT_PRE
mongoc_client_t* client;
int created_by_pid;
PHONGO_ZEND_OBJECT_POST
} php_phongo_manager_t;

Expand Down Expand Up @@ -107,13 +109,16 @@ typedef struct {
typedef struct {
PHONGO_ZEND_OBJECT_PRE
mongoc_client_t* client;
int created_by_pid;
uint32_t server_id;
PHONGO_ZEND_OBJECT_POST
} php_phongo_server_t;

typedef struct {
PHONGO_ZEND_OBJECT_PRE
mongoc_client_session_t* client_session;
mongoc_client_t* client;
int created_by_pid;
PHONGO_ZEND_OBJECT_POST
} php_phongo_session_t;

Expand Down
7 changes: 7 additions & 0 deletions src/MongoDB/Cursor.c
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,11 @@ static void php_phongo_cursor_free_object(phongo_free_object_arg* object TSRMLS_

zend_object_std_dtor(&intern->std TSRMLS_CC);

/* If this Cursor was created in a different process, reset the client so
* that mongoc_cursor_destroy does not issue a killCursors command for an
* active cursor owned by a parent process. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

if (intern->cursor) {
mongoc_cursor_destroy(intern->cursor);
}
Expand Down Expand Up @@ -451,6 +456,8 @@ static phongo_create_object_retval php_phongo_cursor_create_object(zend_class_en
zend_object_std_init(&intern->std, class_type TSRMLS_CC);
object_properties_init(&intern->std, class_type);

PHONGO_SET_CREATED_BY_PID(intern);

#if PHP_VERSION_ID >= 70000
intern->std.handlers = &php_phongo_handler_cursor;

Expand Down
27 changes: 27 additions & 0 deletions src/MongoDB/Manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,10 @@ static PHP_METHOD(Manager, executeCommand)
goto cleanup;
}

/* If the Manager was created in a different process, reset the client so
* that cursors created by this process can be differentiated. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

phongo_execute_command(intern->client, PHONGO_COMMAND_RAW, db, command, options, server_id, return_value, return_value_used TSRMLS_CC);

cleanup:
Expand Down Expand Up @@ -430,6 +434,10 @@ static PHP_METHOD(Manager, executeReadCommand)
return;
}

/* If the Manager was created in a different process, reset the client so
* that cursors created by this process can be differentiated. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

phongo_execute_command(intern->client, PHONGO_COMMAND_READ, db, command, options, server_id, return_value, return_value_used TSRMLS_CC);
} /* }}} */

Expand Down Expand Up @@ -462,6 +470,10 @@ static PHP_METHOD(Manager, executeWriteCommand)
return;
}

/* If the Manager was created in a different process, reset the client so
* that cursors created by this process can be differentiated. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

phongo_execute_command(intern->client, PHONGO_COMMAND_WRITE, db, command, options, server_id, return_value, return_value_used TSRMLS_CC);
} /* }}} */

Expand Down Expand Up @@ -494,6 +506,10 @@ static PHP_METHOD(Manager, executeReadWriteCommand)
return;
}

/* If the Manager was created in a different process, reset the client so
* that cursors created by this process can be differentiated. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

phongo_execute_command(intern->client, PHONGO_COMMAND_READ_WRITE, db, command, options, server_id, return_value, return_value_used TSRMLS_CC);
} /* }}} */

Expand Down Expand Up @@ -535,6 +551,10 @@ static PHP_METHOD(Manager, executeQuery)
goto cleanup;
}

/* If the Manager was created in a different process, reset the client so
* that cursors created by this process can be differentiated. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

phongo_execute_query(intern->client, namespace, query, options, server_id, return_value, return_value_used TSRMLS_CC);

cleanup:
Expand Down Expand Up @@ -750,6 +770,11 @@ static PHP_METHOD(Manager, startSession)
}
}

/* If the Manager was created in a different process, reset the client so
* that its session pool is cleared. This will ensure that we do not re-use
* a server session (i.e. LSID) created by a parent process. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

cs = mongoc_client_start_session(intern->client, cs_opts, &error);

if (cs) {
Expand Down Expand Up @@ -855,6 +880,8 @@ static phongo_create_object_retval php_phongo_manager_create_object(zend_class_e
zend_object_std_init(&intern->std, class_type TSRMLS_CC);
object_properties_init(&intern->std, class_type);

PHONGO_SET_CREATED_BY_PID(intern);

#if PHP_VERSION_ID >= 70000
intern->std.handlers = &php_phongo_handler_manager;

Expand Down
22 changes: 22 additions & 0 deletions src/MongoDB/Server.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ static PHP_METHOD(Server, executeCommand)

options = php_phongo_prep_legacy_option(options, "readPreference", &free_options TSRMLS_CC);

/* If the Server was created in a different process, reset the client so
* that cursors created by this process can be differentiated. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

phongo_execute_command(intern->client, PHONGO_COMMAND_RAW, db, command, options, intern->server_id, return_value, return_value_used TSRMLS_CC);

if (free_options) {
Expand All @@ -71,6 +75,10 @@ static PHP_METHOD(Server, executeReadCommand)
return;
}

/* If the Server was created in a different process, reset the client so
* that cursors created by this process can be differentiated. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

phongo_execute_command(intern->client, PHONGO_COMMAND_READ, db, command, options, intern->server_id, return_value, return_value_used TSRMLS_CC);
} /* }}} */

Expand All @@ -91,6 +99,10 @@ static PHP_METHOD(Server, executeWriteCommand)
return;
}

/* If the Server was created in a different process, reset the client so
* that cursors created by this process can be differentiated. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

phongo_execute_command(intern->client, PHONGO_COMMAND_WRITE, db, command, options, intern->server_id, return_value, return_value_used TSRMLS_CC);
} /* }}} */

Expand All @@ -111,6 +123,10 @@ static PHP_METHOD(Server, executeReadWriteCommand)
return;
}

/* If the Server was created in a different process, reset the client so
* that cursors created by this process can be differentiated. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

phongo_execute_command(intern->client, PHONGO_COMMAND_READ_WRITE, db, command, options, intern->server_id, return_value, return_value_used TSRMLS_CC);
} /* }}} */

Expand All @@ -134,6 +150,10 @@ static PHP_METHOD(Server, executeQuery)

options = php_phongo_prep_legacy_option(options, "readPreference", &free_options TSRMLS_CC);

/* If the Server was created in a different process, reset the client so
* that cursors created by this process can be differentiated. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

phongo_execute_query(intern->client, namespace, query, options, intern->server_id, return_value, return_value_used TSRMLS_CC);

if (free_options) {
Expand Down Expand Up @@ -568,6 +588,8 @@ static phongo_create_object_retval php_phongo_server_create_object(zend_class_en
zend_object_std_init(&intern->std, class_type TSRMLS_CC);
object_properties_init(&intern->std, class_type);

PHONGO_SET_CREATED_BY_PID(intern);

#if PHP_VERSION_ID >= 70000
intern->std.handlers = &php_phongo_handler_server;

Expand Down
15 changes: 12 additions & 3 deletions src/MongoDB/Session.c
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ static PHP_METHOD(Session, getServer)
RETURN_NULL();
}

phongo_server_init(return_value, mongoc_client_session_get_client(intern->client_session), server_id TSRMLS_CC);
phongo_server_init(return_value, intern->client, server_id TSRMLS_CC);
} /* }}} */

/* {{{ proto array|null MongoDB\Driver\Session::getTransactionOptions()
Expand Down Expand Up @@ -639,6 +639,13 @@ static void php_phongo_session_free_object(phongo_free_object_arg* object TSRMLS

zend_object_std_dtor(&intern->std TSRMLS_CC);

/* If this Session was created in a different process, reset the client so
* that its session pool is cleared and mongoc_client_session_destroy will
* destroy the corresponding server session rather than return it to the
* now-empty pool. This will ensure that we do not re-use a server session
* (i.e. LSID) created by a parent process. */
PHONGO_RESET_CLIENT_IF_PID_DIFFERS(intern);

if (intern->client_session) {
mongoc_client_session_destroy(intern->client_session);
}
Expand All @@ -657,6 +664,8 @@ static phongo_create_object_retval php_phongo_session_create_object(zend_class_e
zend_object_std_init(&intern->std, class_type TSRMLS_CC);
object_properties_init(&intern->std, class_type);

PHONGO_SET_CREATED_BY_PID(intern);

#if PHP_VERSION_ID >= 70000
intern->std.handlers = &php_phongo_handler_session;

Expand Down Expand Up @@ -772,13 +781,13 @@ static HashTable* php_phongo_session_get_debug_info(zval* object, int* is_temp T
#if PHP_VERSION_ID >= 70000
zval server;

phongo_server_init(&server, mongoc_client_session_get_client(intern->client_session), server_id TSRMLS_CC);
phongo_server_init(&server, intern->client, server_id TSRMLS_CC);
ADD_ASSOC_ZVAL_EX(&retval, "server", &server);
#else
zval* server = NULL;

MAKE_STD_ZVAL(server);
phongo_server_init(server, mongoc_client_session_get_client(intern->client_session), server_id TSRMLS_CC);
phongo_server_init(server, intern->client, server_id TSRMLS_CC);
ADD_ASSOC_ZVAL_EX(&retval, "server", server);
#endif
} else {
Expand Down
Loading