lydict_val_eq using cb_data as strncmp's len, is it right in some conditions ?

[lumanyu180]

Hi,
I have a problem in libyang, it occurs in func lyht_resize, assert(!ret); I analysis and find error in func: lyht_insert_with_resize_cb, line: ht->val_equal(val_p, &rec->val, 1, ht->cb_data), it is func: lydict_val_eq, *(size_t *)cb_data is 5, str1 is "Service58844", str2 is "Service13604", so it return 1。
It occurs when dict_insert (ctx=0xb696c0, value=0x7fff7f0bff40 "96507", len=5, zerocopy=1), in fact , the data to be inserted is "96507", and its len is 5. but when it is inserted , hash need to be resize. And during resize , data whose value is "Service58844" and data whose value is "Service13604" conflict. But "Service58844", len is 11, not 5. So if it is not right when strncmp in func:lydict_val_eq using len 5。
The backtrace is ：
lyht_insert_with_resize_cb
lyht_insert
lyht_resize
lyht_insert_with_resize_cb
lyht_insert
dict_insert (ctx=0xb696c0, value=0x7fff7f0bff40 "96507", len=5, zerocopy=1)

Thanks very much !

[michalvasko]

Hi,
right, should be fixed now.

Regards,
Michal

[lumanyu180]

Hi,
Thanks for your reply very much. I see the commit. cb_data is not used. But in some condition, str1 may be longer than it should be. For example str1 is : "md:ietf-yang-metadata<....", and str2 is "md". And len is 2, In this condition, str1 is equal to str2. But ,if we use strcmp , the result is not equal. I try to modify it as this: add len in struct dict_rec.

• struct dict_rec {
  --
    | 66 | + char *value;
    | 67 | + uint32_t refcount;
    | 68 | ++ size_t len;
    | 69 | + }

+@@ -202,7 +212,8 @@ dict_insert(struct ly_ctx *ctx, char *value, size_t len, int zerocopy)

  | 49 | + /* create record for lyht_insert */
  | 50 | + rec.value = value;
  | 51 | + rec.refcount = 1;
  | 52 | +-
  | 53 | ++ rec.len = len;
  | 54 | ++
  | 55 | + LOGDBG(LY_LDGDICT, "inserting "%s"", rec.value);
  | 56 | + ret = lyht_insert(ctx->dict.hash_tab, (void *)&rec, hash, (void **)&match);

+@@ -32,13 +32,22 @@ lydict_val_eq(void *val1_p, void *val2_p, int UNUSED(mod), void *cb_data)

  | 17 | +
  | 18 | + const char *str1 = ((struct dict_rec *)val1_p)->value;
  | 19 | + const char *str2 = ((struct dict_rec *)val2_p)->value;
  | 20 | ++ size_t str1len = ((struct dict_rec *)val1_p)->len;
  | 21 | ++ size_t str2len = ((struct dict_rec *)val2_p)->len;
  | 22 | +
  | 23 | + if (!str1 || !str2 || !cb_data) {
  | 24 | + LOGARG;
  | 25 | + return 0;
  | 26 | + }
  | 27 | +
  | 28 | +- if (strncmp(str1, str2, *(size_t )cb_data) == 0) {
  | 29 | ++ if (str1len != str2len) {
  | 30 | ++ return 0;
  | 31 | ++ }
  | 32 | ++
  | 33 | ++ / val1_p: get specified content in xml message by val1_p->value and val1_p->len
  | 34 | ++ * val2_p: string in hash table
  | 35 | ++ */
  | 36 | ++ if (strncmp(str1, str2, str1len) == 0) {
  | 37 | + return 1;
  | 38 | + }

+@@ -160,6 +169,7 @@ lydict_remove(struct ly_ctx *ctx, const char *value)

  | 41 | + /* create record for lyht_find call */
  | 42 | + rec.value = (char )value;
  | 43 | + rec.refcount = 0;
  | 44 | ++ rec.len = len;
  | 45 | +
  | 46 | + pthread_mutex_lock(&ctx->dict.lock);
  | 47 | + / set len as data for compare callback */
  | 48 | +@@ -202,7 +212,8 @@ dict_insert(struct ly_ctx *ctx, char value, size_t len, int zerocopy)
  | 49 | + / create record for lyht_insert */
  | 50 | + rec.value = value;
  | 51 | + rec.refcount = 1;
  | 52 | +-
  | 53 | ++ rec.len = len;
  | 54 | ++
  | 55 | + LOGDBG(LY_LDGDICT, "inserting "%s"", rec.value);

[michalvasko]

Hi,
we will not be changing the dict structure in the current libyang, so your change cannot be used. Also, this other function is used only for values already in the dictionary, which are always terminated by \0 so there is no problem.

Regards,
Michal

[lumanyu180]

Hi,
Thanks for your reply very much. I got your meaning, but it seems this other function is used
not only for values already in the dictionary. It may used for new value to be inserted. I test, it seems it is not right.

[michalvasko]

Hi,
okay, there was one more place where it needed to be fixed, Is it fine now?

Regards,
Michal

[foxge]

Hi

version: 1.0.130

The same prolbem happened,

    /* add all the old records into the new records array */
    for (i = 0; i < old_size; ++i) {
        rec = lyht_get_rec(old_recs, ht->rec_size, i);
        if (rec->hits > 0) {
            ret = lyht_insert(ht, rec->val, rec->hash, NULL);
            assert(!ret);  //Here triiger abort
            (void)ret;
        }
    }

#0  0x00007ffff2aa9417 in raise () from /lib64/libc.so.6
#1  0x00007ffff2aaab08 in abort () from /lib64/libc.so.6
#2  0x00007ffff2aa23d6 in __assert_fail_base () from /lib64/libc.so.6
#3  0x00007ffff2aa2482 in __assert_fail () from /lib64/libc.so.6
#4  0x00007ffff33175dc in lyht_resize (ht=0xae5de0, enlarge=1)
    at hash_table.c:390
#5  0x00007ffff33184ed in lyht_insert_with_resize_cb (ht=0xae5de0, val_p=0x7fffa51ee700, hash=3309099932, resize_val_equal=0x0, match_p=0x7fffa51ee6f8)
    at hash_table.c:794
#6  0x00007ffff33185af in lyht_insert (ht=0xae5de0, val_p=0x7fffa51ee700, hash=3309099932, match_p=0x7fffa51ee6f8)
    at hash_table.c:820
#7  0x00007ffff3316f3b in dict_insert (ctx=0xae5d50, value=0x7fff7f039db0 "96401", len=5, zerocopy=1)
    at hash_table.c:209
#8  0x00007ffff3317160 in lydict_insert_zc (ctx=0xae5d50, value=0x7fff7f039db0 "96401")
    at hash_table.c:269
#9  0x00007ffff33361b2 in lyxml_parse_elem (ctx=0xae5d50, 
    
    at xml.c:1105
#10 0x00007ffff33360c5 in lyxml_parse_elem (ctx=0xae5d50, 
    
    at xml.c:1085
#11 0x00007ffff33360c5 in lyxml_parse_elem (ctx=0xae5d50, 
    
    at xml.c:1085
#12 0x00007ffff33360c5 in lyxml_parse_elem (ctx=0xae5d50, 
    
    at xml.c:1085
#13 0x00007ffff33360c5 in lyxml_parse_elem (ctx=0xae5d50, 
    
    at xml.c:1085
#14 0x00007ffff33360c5 in lyxml_parse_elem (ctx=0xae5d50, 
    
    at xml.c:1085
#15 0x00007ffff33366a6 in lyxml_parse_mem (ctx=0xae5d50, 
    

Some personal informations are hide.

[michalvasko]

Hi,
the commits are referenced here you can easily check that they were done after 1.0.130 was released. If you reproduce it with the latest release 1.0.167, let us know. Otherwise, there is nothing for us to do.

Regards,
Michal