From 4082ca2220b5ba910b546afddf7780fc4a51f75a Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Sat, 19 Oct 2024 02:47:04 +0900 Subject: [PATCH 1/2] asn1_der_decoding2: optimize _asn1_find_up call with node cache If we are parsing a sequence or set and the current node is a direct child of it, there is no need to traverse the list back to the leftmost one as we have a node cache. Signed-off-by: Daiki Ueno Signed-off-by: Simon Josefsson --- lib/decoding.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/decoding.c b/lib/decoding.c index d2f6dea..1e0fcb3 100644 --- a/lib/decoding.c +++ b/lib/decoding.c @@ -1570,7 +1570,14 @@ asn1_der_decoding2 (asn1_node *element, const void *ider, int *max_ider_len, move = UP; } if (move == UP) - p = _asn1_find_up (p); + { + /* If we are parsing a sequence or set and p is a direct + child of it, no need to traverse the list back to the leftmost node. */ + if (tcache.tail == p) + p = tcache.head; + else + p = _asn1_find_up (p); + } } _asn1_delete_not_used (*element); -- 2.47.1 From 869a97aa259dffa2620dabcad84e1c22545ffc3d Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Fri, 8 Nov 2024 16:05:32 +0900 Subject: [PATCH 2/2] asn1_find_node: optimize "?NUMBER" node lookup with indexing To avoid linear search of named nodes, this adds a array of child nodes to their parent nodes as a cache. Signed-off-by: Daiki Ueno Signed-off-by: Simon Josefsson --- lib/element.c | 64 +++++++++++++++++++++++++++++++++++++++--------- lib/element.h | 10 ++++++++ lib/int.h | 8 ++++++ lib/parser_aux.c | 10 ++++++++ lib/structure.c | 13 ++++++++++ 5 files changed, 93 insertions(+), 12 deletions(-) diff --git a/lib/element.c b/lib/element.c index 850bef4..528df41 100644 --- a/lib/element.c +++ b/lib/element.c @@ -33,6 +33,8 @@ #include "structure.h" #include "c-ctype.h" #include "element.h" +#include +#include "intprops.h" void _asn1_hierarchical_name (asn1_node_const node, char *name, int name_size) @@ -129,6 +131,41 @@ _asn1_convert_integer (const unsigned char *value, unsigned char *value_out, return ASN1_SUCCESS; } +int +_asn1_node_array_set (struct asn1_node_array_st *array, size_t position, + asn1_node node) +{ + if (position >= array->size) + { + size_t new_size = position, i; + asn1_node *new_nodes; + + if (INT_MULTIPLY_OVERFLOW (new_size, 2)) + return ASN1_GENERIC_ERROR; + new_size *= 2; + + if (INT_ADD_OVERFLOW (new_size, 1)) + return ASN1_GENERIC_ERROR; + new_size += 1; + + if (INT_MULTIPLY_OVERFLOW (new_size, sizeof (*new_nodes))) + return ASN1_GENERIC_ERROR; + + new_nodes = realloc (array->nodes, new_size * sizeof (*new_nodes)); + if (!new_nodes) + return ASN1_MEM_ALLOC_ERROR; + + for (i = array->size; i < new_size; i++) + new_nodes[i] = NULL; + + array->nodes = new_nodes; + array->size = new_size; + } + + array->nodes[position] = node; + return ASN1_SUCCESS; +} + /* Appends a new element into the sequence (or set) defined by this * node. The new element will have a name of '?number', where number * is a monotonically increased serial number. @@ -145,6 +182,7 @@ _asn1_append_sequence_set (asn1_node node, struct node_tail_cache_st *pcache) asn1_node p, p2; char temp[LTOSTR_MAX_SIZE+1]; long n; + int result; if (!node || !(node->down)) return ASN1_GENERIC_ERROR; @@ -176,22 +214,24 @@ _asn1_append_sequence_set (asn1_node node, struct node_tail_cache_st *pcache) pcache->tail = p2; } - if (p->name[0] == 0) - _asn1_str_cpy (temp, sizeof (temp), "?1"); - else + n = 0; + if (p->name[0] != 0) { - n = strtol (p->name + 1, NULL, 0); - n++; - temp[0] = '?'; - if (n < 0) - return ASN1_GENERIC_ERROR; - /* assuming non-negative n, we have enough space in buffer */ - _asn1_ltostr (n, temp + 1); - if (strlen(temp) >= LTOSTR_MAX_SIZE) - return ASN1_GENERIC_ERROR; + n = strtol (p->name + 1, NULL, 10); + if (n <= 0 || n >= LONG_MAX - 1) + return ASN1_GENERIC_ERROR; } + /* assuming non-negative n, we have enough space in buffer */ + temp[0] = '?'; + _asn1_ltostr (n + 1, temp + 1); + if (strlen(temp) >= LTOSTR_MAX_SIZE) + return ASN1_GENERIC_ERROR; _asn1_set_name (p2, temp); /* p2->type |= CONST_OPTION; */ + result = _asn1_node_array_set (&node->numbered_children, n, p2); + if (result != ASN1_SUCCESS) + return result; + p2->parent = node; return ASN1_SUCCESS; } diff --git a/lib/element.h b/lib/element.h index 732054e..b84e3a2 100644 --- a/lib/element.h +++ b/lib/element.h @@ -37,4 +37,14 @@ int _asn1_convert_integer (const unsigned char *value, void _asn1_hierarchical_name (asn1_node_const node, char *name, int name_size); +static inline asn1_node_const +_asn1_node_array_get (const struct asn1_node_array_st *array, size_t position) +{ + return position < array->size ? array->nodes[position] : NULL; +} + +int +_asn1_node_array_set (struct asn1_node_array_st *array, size_t position, + asn1_node node); + #endif diff --git a/lib/int.h b/lib/int.h index 4f2d98d..41b12b0 100644 --- a/lib/int.h +++ b/lib/int.h @@ -31,6 +31,12 @@ #define ASN1_SMALL_VALUE_SIZE 16 +struct asn1_node_array_st +{ + asn1_node *nodes; + size_t size; +}; + /* This structure is also in libtasn1.h, but then contains less fields. You cannot make any modifications to these first fields without breaking ABI. */ @@ -47,6 +53,8 @@ struct asn1_node_st asn1_node left; /* Pointer to the next list element */ /* private fields: */ unsigned char small_value[ASN1_SMALL_VALUE_SIZE]; /* For small values */ + asn1_node parent; /* Pointer to the parent node */ + struct asn1_node_array_st numbered_children; /* Array of unnamed child nodes for caching */ /* values used during decoding/coding */ int tmp_ival; diff --git a/lib/parser_aux.c b/lib/parser_aux.c index 415905a..4281cc9 100644 --- a/lib/parser_aux.c +++ b/lib/parser_aux.c @@ -126,6 +126,7 @@ asn1_find_node (asn1_node_const pointer, const char *name) const char *n_start; unsigned int nsize; unsigned int nhash; + const struct asn1_node_array_st *numbered_children; if (pointer == NULL) return NULL; @@ -209,6 +210,7 @@ asn1_find_node (asn1_node_const pointer, const char *name) if (p->down == NULL) return NULL; + numbered_children = &p->numbered_children; p = p->down; if (p == NULL) return NULL; @@ -222,6 +224,12 @@ asn1_find_node (asn1_node_const pointer, const char *name) } else { /* no "?LAST" */ + if (n[0] == '?' && c_isdigit (n[1])) + { + long position = strtol (n + 1, NULL, 10); + if (position > 0 && position < LONG_MAX) + p = _asn1_node_array_get (numbered_children, position - 1); + } while (p) { if (p->name_hash == nhash && !strcmp (p->name, n)) @@ -509,6 +517,8 @@ _asn1_remove_node (asn1_node node, unsigned int flags) if (node->value != node->small_value) free (node->value); } + + free (node->numbered_children.nodes); free (node); } diff --git a/lib/structure.c b/lib/structure.c index 9c95b9e..32692ad 100644 --- a/lib/structure.c +++ b/lib/structure.c @@ -31,6 +31,9 @@ #include #include "parser_aux.h" #include +#include "c-ctype.h" +#include "element.h" +#include extern char _asn1_identifierMissing[]; @@ -391,6 +394,16 @@ asn1_delete_element (asn1_node structure, const char *element_name) if (source_node == NULL) return ASN1_ELEMENT_NOT_FOUND; + if (source_node->parent + && source_node->name[0] == '?' + && c_isdigit (source_node->name[1])) + { + long position = strtol (source_node->name + 1, NULL, 10); + if (position > 0 && position < LONG_MAX) + _asn1_node_array_set (&source_node->parent->numbered_children, + position - 1, NULL); + } + p2 = source_node->right; p3 = _asn1_find_left (source_node); if (!p3) -- 2.47.1