Skip to content

Commit 17d5d6c

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

File tree

2 files changed

+56
-18
lines changed

2 files changed

+56
-18
lines changed

‎ext/snmp/snmp.c‎

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,35 @@ 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 *pptr = (zend_string *)(ptr - XtOffsetOf(zend_string, val));
631+
if (!ZSTR_IS_INTERNED(pptr)) {
632+
if (GC_REFCOUNT(pptr)) {
633+
zend_string_release(pptr);
634+
} else {
635+
efree(pptr);
636+
}
637+
}
638+
}
639+
640+
static void php_free_objid_query(struct objid_query *objid_query, zend_string* oid_str, zend_string *type_str, HashTable *type_ht, zend_string *value_str, HashTable *value_ht, int st) {
641+
if (!oid_str) {
642+
for (int i = 0; i < objid_query->count; i ++) {
643+
snmpobjarg *arg = &objid_query->vars[i];
644+
if (st & SNMP_CMD_SET) {
645+
if (!type_str && type_ht) {
646+
php_snmp_zend_string_release_from_char_pointer(&arg->type);
647+
}
648+
if (!value_str && value_ht) {
649+
php_snmp_zend_string_release_from_char_pointer(arg->value);
650+
}
651+
}
652+
php_snmp_zend_string_release_from_char_pointer(arg->oid);
653+
}
654+
}
655+
efree(objid_query->vars);
656+
}
657+
629658
/* {{{ php_snmp_parse_oid
630659
*
631660
* OID parser (and type, value for SNMP_SET command)
@@ -682,7 +711,6 @@ static bool php_snmp_parse_oid(
682711
return false;
683712
}
684713
objid_query->vars[objid_query->count].oid = ZSTR_VAL(tmp);
685-
zend_string_release(tmp);
686714
if (st & SNMP_CMD_SET) {
687715
if (type_str) {
688716
pptr = ZSTR_VAL(type_str);
@@ -706,18 +734,24 @@ static bool php_snmp_parse_oid(
706734
}
707735
}
708736
if (idx_type < type_ht->nNumUsed) {
709-
convert_to_string(tmp_type);
710-
if (Z_STRLEN_P(tmp_type) != 1) {
737+
zval new;
738+
ZVAL_COPY_VALUE(&new, tmp_type);
739+
if (!try_convert_to_string(&new)) {
740+
zend_value_error("conversion to string failed");
741+
php_free_objid_query(objid_query, oid_str, type_str, type_ht, value_str, value_ht, st);
742+
return false;
743+
}
744+
if (Z_STRLEN(new) != 1) {
711745
zend_value_error("Type must be a single character");
712-
efree(objid_query->vars);
746+
php_free_objid_query(objid_query, oid_str, type_str, type_ht, value_str, value_ht, st);
713747
return false;
714748
}
715-
pptr = Z_STRVAL_P(tmp_type);
749+
pptr = Z_STRVAL(new);
716750
objid_query->vars[objid_query->count].type = *pptr;
717751
idx_type++;
718752
} else {
719753
php_error_docref(NULL, E_WARNING, "'%s': no type set", Z_STRVAL_P(tmp_oid));
720-
efree(objid_query->vars);
754+
php_free_objid_query(objid_query, oid_str, type_str, type_ht, value_str, value_ht, st);
721755
return false;
722756
}
723757
}
@@ -743,12 +777,16 @@ static bool php_snmp_parse_oid(
743777
}
744778
}
745779
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);
780+
zend_string *tmp = zval_try_get_string(tmp_value);
781+
if (!tmp) {
782+
php_free_objid_query(objid_query, oid_str, type_str, type_ht, value_str, value_ht, st);
783+
return false;
784+
}
785+
objid_query->vars[objid_query->count].value = ZSTR_VAL(tmp);
748786
idx_value++;
749787
} else {
750788
php_error_docref(NULL, E_WARNING, "'%s': no value set", Z_STRVAL_P(tmp_oid));
751-
efree(objid_query->vars);
789+
php_free_objid_query(objid_query, oid_str, type_str, type_ht, value_str, value_ht, st);
752790
return false;
753791
}
754792
}
@@ -761,14 +799,14 @@ static bool php_snmp_parse_oid(
761799
if (st & SNMP_CMD_WALK) {
762800
if (objid_query->count > 1) {
763801
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Multi OID walks are not supported!");
764-
efree(objid_query->vars);
802+
php_free_objid_query(objid_query, oid_str, type_str, type_ht, value_str, value_ht, st);
765803
return false;
766804
}
767805
objid_query->vars[0].name_length = MAX_NAME_LEN;
768806
if (strlen(objid_query->vars[0].oid)) { /* on a walk, an empty string means top of tree - no error */
769807
if (!snmp_parse_oid(objid_query->vars[0].oid, objid_query->vars[0].name, &(objid_query->vars[0].name_length))) {
770808
php_snmp_error(object, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[0].oid);
771-
efree(objid_query->vars);
809+
php_free_objid_query(objid_query, oid_str, type_str, type_ht, value_str, value_ht, st);
772810
return false;
773811
}
774812
} else {
@@ -780,7 +818,7 @@ static bool php_snmp_parse_oid(
780818
objid_query->vars[objid_query->offset].name_length = MAX_OID_LEN;
781819
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))) {
782820
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);
821+
php_free_objid_query(objid_query, oid_str, type_str, type_ht, value_str, value_ht, st);
784822
return false;
785823
}
786824
}
@@ -1257,12 +1295,12 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12571295

12581296
if (session_less_mode) {
12591297
if (!netsnmp_session_init(&session, version, a1, a2, timeout, retries)) {
1260-
efree(objid_query.vars);
1298+
php_free_objid_query(&objid_query, oid_str, type_str, type_ht, value_str, value_ht, st);
12611299
netsnmp_session_free(&session);
12621300
RETURN_FALSE;
12631301
}
12641302
if (version == SNMP_VERSION_3 && !netsnmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL)) {
1265-
efree(objid_query.vars);
1303+
php_free_objid_query(&objid_query, oid_str, type_str, type_ht, value_str, value_ht, st);
12661304
netsnmp_session_free(&session);
12671305
/* Warning message sent already, just bail out */
12681306
RETURN_FALSE;
@@ -1273,7 +1311,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12731311
session = snmp_object->session;
12741312
if (!session) {
12751313
zend_throw_error(NULL, "Invalid or uninitialized SNMP object");
1276-
efree(objid_query.vars);
1314+
php_free_objid_query(&objid_query, oid_str, type_str, type_ht, value_str, value_ht, st);
12771315
RETURN_THROWS();
12781316
}
12791317

@@ -1299,15 +1337,15 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12991337

13001338
php_snmp_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU, st, session, &objid_query);
13011339

1302-
efree(objid_query.vars);
1303-
13041340
if (session_less_mode) {
13051341
netsnmp_session_free(&session);
13061342
} else {
13071343
netsnmp_ds_set_boolean(NETSNMP_DS_LIBRARY_ID, NETSNMP_DS_LIB_PRINT_NUMERIC_ENUM, glob_snmp_object.enum_print);
13081344
netsnmp_ds_set_boolean(NETSNMP_DS_LIBRARY_ID, NETSNMP_DS_LIB_QUICK_PRINT, glob_snmp_object.quick_print);
13091345
netsnmp_ds_set_int(NETSNMP_DS_LIBRARY_ID, NETSNMP_DS_LIB_OID_OUTPUT_FORMAT, glob_snmp_object.oid_output_format);
13101346
}
1347+
1348+
php_free_objid_query(&objid_query, oid_str, type_str, type_ht, value_str, value_ht, st);
13111349
}
13121350
/* }}} */
13131351

‎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)