Skip to content

Commit 0ac4e4b

Browse files
committed
try out @nielsdos suggestion, delaying free object_id at the end of the call.
1 parent fc4ef43 commit 0ac4e4b

File tree

2 files changed

+38
-15
lines changed

2 files changed

+38
-15
lines changed

ext/snmp/snmp.c

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,22 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st,
626626
}
627627
/* }}} */
628628

629+
static void php_snmp_zend_string_release_from_char_pointer(char *ptr) {
630+
zend_string_release((zend_string*) (ptr - XtOffsetOf(zend_string, val)));
631+
}
632+
633+
static void php_free_objid_query(struct objid_query *objid_query, zend_string* oid_str, int st) {
634+
if (oid_str && (st & SNMP_CMD_SET)) {
635+
php_snmp_zend_string_release_from_char_pointer(objid_query->vars[0].value);
636+
} else {
637+
for (int i = 0; i < objid_query->count; i ++) {
638+
snmpobjarg *arg = &objid_query->vars[i];
639+
php_snmp_zend_string_release_from_char_pointer(arg->oid);
640+
}
641+
}
642+
efree(objid_query->vars);
643+
}
644+
629645
/* {{{ php_snmp_parse_oid
630646
*
631647
* OID parser (and type, value for SNMP_SET command)
@@ -682,7 +698,6 @@ static bool php_snmp_parse_oid(
682698
return false;
683699
}
684700
objid_query->vars[objid_query->count].oid = ZSTR_VAL(tmp);
685-
zend_string_release(tmp);
686701
if (st & SNMP_CMD_SET) {
687702
if (type_str) {
688703
pptr = ZSTR_VAL(type_str);
@@ -706,13 +721,17 @@ static bool php_snmp_parse_oid(
706721
}
707722
}
708723
if (idx_type < type_ht->nNumUsed) {
709-
convert_to_string(tmp_type);
710-
if (Z_STRLEN_P(tmp_type) != 1) {
724+
zend_string *tmp = zval_try_get_string(tmp_type);
725+
if (!tmp) {
726+
efree(objid_query->vars);
727+
return false;
728+
}
729+
if (ZSTR_LEN(tmp) != 1) {
711730
zend_value_error("Type must be a single character");
712731
efree(objid_query->vars);
713732
return false;
714733
}
715-
pptr = Z_STRVAL_P(tmp_type);
734+
pptr = ZSTR_VAL(tmp);
716735
objid_query->vars[objid_query->count].type = *pptr;
717736
idx_type++;
718737
} else {
@@ -743,8 +762,12 @@ static bool php_snmp_parse_oid(
743762
}
744763
}
745764
if (idx_value < value_ht->nNumUsed) {
746-
convert_to_string(tmp_value);
747-
objid_query->vars[objid_query->count].value = Z_STRVAL_P(tmp_value);
765+
zend_string *tmp = zval_try_get_string(tmp_value);
766+
if (!tmp) {
767+
efree(objid_query->vars);
768+
return false;
769+
}
770+
objid_query->vars[objid_query->count].value = ZSTR_VAL(tmp);
748771
idx_value++;
749772
} else {
750773
php_error_docref(NULL, E_WARNING, "'%s': no value set", Z_STRVAL_P(tmp_oid));
@@ -761,14 +784,14 @@ static bool php_snmp_parse_oid(
761784
if (st & SNMP_CMD_WALK) {
762785
if (objid_query->count > 1) {
763786
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Multi OID walks are not supported!");
764-
efree(objid_query->vars);
787+
php_free_objid_query(objid_query, oid_str, st);
765788
return false;
766789
}
767790
objid_query->vars[0].name_length = MAX_NAME_LEN;
768791
if (strlen(objid_query->vars[0].oid)) { /* on a walk, an empty string means top of tree - no error */
769792
if (!snmp_parse_oid(objid_query->vars[0].oid, objid_query->vars[0].name, &(objid_query->vars[0].name_length))) {
770793
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[0].oid);
771-
efree(objid_query->vars);
794+
php_free_objid_query(objid_query, oid_str, st);
772795
return false;
773796
}
774797
} else {
@@ -780,7 +803,7 @@ static bool php_snmp_parse_oid(
780803
objid_query->vars[objid_query->offset].name_length = MAX_OID_LEN;
781804
if (!snmp_parse_oid(objid_query->vars[objid_query->offset].oid, objid_query->vars[objid_query->offset].name, &(objid_query->vars[objid_query->offset].name_length))) {
782805
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[objid_query->offset].oid);
783-
efree(objid_query->vars);
806+
php_free_objid_query(objid_query, oid_str, st);
784807
return false;
785808
}
786809
}
@@ -1257,12 +1280,12 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12571280

12581281
if (session_less_mode) {
12591282
if (!netsnmp_session_init(&session, version, a1, a2, timeout, retries)) {
1260-
efree(objid_query.vars);
1283+
php_free_objid_query(&objid_query, oid_str, st);
12611284
netsnmp_session_free(&session);
12621285
RETURN_FALSE;
12631286
}
12641287
if (version == SNMP_VERSION_3 && !netsnmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL)) {
1265-
efree(objid_query.vars);
1288+
php_free_objid_query(&objid_query, oid_str, st);
12661289
netsnmp_session_free(&session);
12671290
/* Warning message sent already, just bail out */
12681291
RETURN_FALSE;
@@ -1273,7 +1296,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12731296
session = snmp_object->session;
12741297
if (!session) {
12751298
zend_throw_error(NULL, "Invalid or uninitialized SNMP object");
1276-
efree(objid_query.vars);
1299+
php_free_objid_query(&objid_query, oid_str, st);
12771300
RETURN_THROWS();
12781301
}
12791302

@@ -1299,15 +1322,15 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12991322

13001323
php_snmp_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU, st, session, &objid_query);
13011324

1302-
efree(objid_query.vars);
1303-
13041325
if (session_less_mode) {
13051326
netsnmp_session_free(&session);
13061327
} else {
13071328
netsnmp_ds_set_boolean(NETSNMP_DS_LIBRARY_ID, NETSNMP_DS_LIB_PRINT_NUMERIC_ENUM, glob_snmp_object.enum_print);
13081329
netsnmp_ds_set_boolean(NETSNMP_DS_LIBRARY_ID, NETSNMP_DS_LIB_QUICK_PRINT, glob_snmp_object.quick_print);
13091330
netsnmp_ds_set_int(NETSNMP_DS_LIBRARY_ID, NETSNMP_DS_LIB_OID_OUTPUT_FORMAT, glob_snmp_object.oid_output_format);
13101331
}
1332+
1333+
php_free_objid_query(&objid_query, oid_str, st);
13111334
}
13121335
/* }}} */
13131336

ext/snmp/tests/gh16959.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ array(4) {
3535
int(0)
3636
}
3737

38-
Warning: snmpget(): Invalid object identifier: -229 in %s on line %d
38+
Warning: snmpget(): Invalid object identifier: -54 in %s on line %d
3939
bool(true)
4040
array(4) {
4141
[63]=>

0 commit comments

Comments
 (0)