Memory issue with json-c

[Rakesh K R]

Hi,

I have following code.

#define JSON_FREE(X) do {       \

    if ((X) != NULL) {            \

        json_decref((X));               \

        (X) = NULL;             \

    }                           \

} while(0)

char *

create_data(some_struct *mci, char *cust_id, int device){

    if (!mci || !cust_id) {

        return NULL;

    }

    

    char *result = NULL;

    json_t *srv_array = json_array();

    json_t *root = NULL;

    json_t *srv_obj = NULL;

    json_t *proto_type_obj = NULL;

    json_t *connected_to_obj = NULL;

    json_t *result_obj = NULL;

    json_error_t err = {0};

    if (!srv_array) {

        printf("OOM: Failed to create an array");

        return NULL;

    }

    get_value_from_map(cache_ctx, hash_server_name, server_key, TRUE, &count);

    if (count == 0)

        goto client_part;

    while ((mci_server = get_value_from_map(cust_ctx.cache_ctx, hash_server_name, server_key, FALSE, NULL)) != NULL) {

        if ((root = json_loads(mci_server, 0, &err)) == NULL) {

            goto cleanup;

        }

        if ((srv_obj = json_object_get(root, SERVICE)) == NULL) {

            JSON_FREE(root);

            goto cleanup;

        }

        if ((proto_type_obj = json_object_get(root, DEVICE_TYPE)) == NULL) {

            JSON_FREE(root);

            goto cleanup;

        }

        proto_type |= json_integer_value(proto_type_obj);

        if ((connected_to_obj = json_object_get(root, "DEVICE")) == NULL) {

            printf("Failed to get DEVICE from mci_string");

        } else {

            connected_to = json_integer_value(connected_to_obj);

        }

        idx  = json_integer_value(srv_obj);

        encountered_services[0] = 1;                                          

        if ((idx <= SVC_MAX) && (idx > 0)) {                                 

            if(encountered_services[idx] == 0){

                if(json_array_append_new(srv_array, srv_obj) == 0){

                    encountered_services[idx] = 1; 

                } else {

                    printf("Append unsuccessfull for service %d\n", idx);

                }

            }

        }

        free_str_arr(mci_server);

        JSON_FREE(root);

}

client_part:

    if (count > 0){

        dev_type = AG_SERVER;

        if (connected_to <= 0) {

            JSON_FREE(root);

            goto cleanup;

        }

    }

    if ((mci_client = get_map_values(cust_ctx.cache_ctx, hash_client_name, client_key)) != NULL) {

        dev_type |= AG_CLIENT;

        if ((root = json_loads(mci_client, 0, &err)) == NULL) {

            goto cleanup;

        }

        if ((proto_type_obj = json_object_get(root, DEVICE_TYPE)) == NULL) {

            JSON_FREE(root);

            goto cleanup;

        }

     

        if (connected_to <= 0) {

            if ((connected_to_obj = json_object_get(root, "DEVICE")) == NULL) {

                MDNS_INTERNAL_ERROR_FMT("Failed to get DEVICE from client mci_string");

                JSON_FREE(root);

                goto cleanup;

            }

            connected_to = json_integer_value(connected_to_obj);

        }

proto_type |= json_integer_value(proto_type_obj);

    } else {

        printf("No clients present with key %s", client_key);

    }

if ((result_obj = json_object()) == NULL) {

        goto cleanup;

    }

    if ((ret = json_object_set_new_nocheck(result_obj, "cust_id", json_string(cust_id))) < 0) {

        goto cleanup;

    }

    if ((ret = json_object_set_new_nocheck(result_obj, "mac", json_string(mac_addr))) < 0) {

        goto cleanup;

    }

    if ((ret = json_object_set_new_nocheck(result_obj, "protocol_type", json_integer(proto_type))) < 0) {

        goto cleanup;

    }

    if ((ret = json_object_set_new_nocheck(result_obj, "device_type", json_integer(dev_type))) < 0) {

        goto cleanup;

    }

    if ((ret = json_object_set_new_nocheck(result_obj, "services", srv_array)) < 0) {

        goto cleanup;

    }

    if ((ret = json_object_set_new_nocheck(result_obj, "connected_to", json_integer(connected_to))) < 0) {

        goto cleanup;

    }

    if ((result = json_dumps(result_obj, 0)) == NULL) {

        goto cleanup;

    }

cleanup:

    if(server_key){

        free(server_key);

        server_key = NULL;

    }

    if(client_key){

        free(client_key);

        client_key = NULL;

    }

    JSON_FREE(result_obj);

    JSON_FREE(root);

    JSON_FREE(srv_array);

    return result;

}

and I am seeing lot of errors from valgrind. Can someone help in identify the issue?

Valgrind logs:

==1846== Invalid read of size 4

==1846==    at 0x917683: do_dump (dump.c:184)

==1846==    by 0x91794D: do_dump (dump.c:249)

==1846==    by 0x917816: do_dump (dump.c:384)

==1846==    by 0x917D4E: json_dumps (dump.c:426)

==1846==    by 0x79DCE4: create_data (mdns_json.c:2189)

==1846==    by 0x74DEC4: main (mdns_main.c:278)

==1846==  Address 0x10def160 is 0 bytes inside a block of size 24 free'd

==1846==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==1846==    by 0x9182A7: json_decref (jansson.h:112)

==1846==    by 0x9182A7: hashtable_do_clear (hashtable.c:142)

==1846==    by 0x918338: hashtable_close (hashtable.c:205)

==1846==    by 0x91B620: json_delete_object (value.c:78)

==1846==    by 0x91B620: json_delete (value.c:935)

==1846==    by 0x78E115: json_decref (jansson.h:112)

==1846==    by 0x79D096: create_data (mdns_json.c:2102)

==1846==    by 0x74DEC4: main (mdns_main.c:278)

==1846== 

==1846== Invalid read of size 4

==1846==    at 0x91768C: do_dump (dump.c:184)

==1846==    by 0x91794D: do_dump (dump.c:249)

==1846==    by 0x917816: do_dump (dump.c:384)

==1846==    by 0x917D4E: json_dumps (dump.c:426)

==1846==    by 0x79DCE4: create_data (mdns_json.c:2189)

==1846==    by 0x74DEC4: main (mdns_main.c:278)

==1846==  Address 0x10def160 is 0 bytes inside a block of size 24 free'd

==1846==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==1846==    by 0x9182A7: json_decref (jansson.h:112)

==1846==    by 0x9182A7: hashtable_do_clear (hashtable.c:142)

==1846==    by 0x918338: hashtable_close (hashtable.c:205)

==1846==    by 0x91B620: json_delete_object (value.c:78)

==1846==    by 0x91B620: json_delete (value.c:935)

==1846==    by 0x78E115: json_decref (jansson.h:112)

==1846==    by 0x79D096: create_data (mdns_json.c:2102)

==1846==    by 0x74DEC4: main (mdns_main.c:278)

==1846== 

==1846== Invalid read of size 4

==1846==    at 0x91B437: json_integer_value (value.c:810)

==1846==    by 0x91799C: do_dump (dump.c:199)

==1846==    by 0x91794D: do_dump (dump.c:249)

==1846==    by 0x917816: do_dump (dump.c:384)

==1846==    by 0x917D4E: json_dumps (dump.c:426)

==1846==    by 0x79DCE4: create_data (mdns_json.c:2189)

==1846==    by 0x74DEC4: main (mdns_main.c:278)

==1846==  Address 0x10def160 is 0 bytes inside a block of size 24 free'd

==1846==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==1846==    by 0x9182A7: json_decref (jansson.h:112)

==1846==    by 0x9182A7: hashtable_do_clear (hashtable.c:142)

==1846==    by 0x918338: hashtable_close (hashtable.c:205)

==1846==    by 0x91B620: json_delete_object (value.c:78)

==1846==    by 0x91B620: json_delete (value.c:935)

==1846==    by 0x78E115: json_decref (jansson.h:112)

==1846==    by 0x79D096: create_data (mdns_json.c:2102)

==1846==    by 0x74DEC4: main (mdns_main.c:278)

==1846== 

==1846== Invalid read of size 8

==1846==    at 0x91B440: json_integer_value (value.c:813)

==1846==    by 0x91799C: do_dump (dump.c:199)

==1846==    by 0x91794D: do_dump (dump.c:249)

==1846==    by 0x917816: do_dump (dump.c:384)

==1846==    by 0x917D4E: json_dumps (dump.c:426)

==1846==    by 0x79DCE4: create_data (mdns_json.c:2189)

==1846==    by 0x74DEC4: main (mdns_main.c:278)

==1846==  Address 0x10def170 is 16 bytes inside a block of size 24 free'd

==1846==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==1846==    by 0x9182A7: json_decref (jansson.h:112)

==1846==    by 0x9182A7: hashtable_do_clear (hashtable.c:142)

==1846==    by 0x918338: hashtable_close (hashtable.c:205)

==1846==    by 0x91B620: json_delete_object (value.c:78)

==1846==    by 0x91B620: json_delete (value.c:935)

==1846==    by 0x78E115: json_decref (jansson.h:112)

==1846==    by 0x79D096: create_data (mdns_json.c:2102)

==1846==    by 0x74DEC4: main (mdns_main.c:278)

==1846== 

==1846== Invalid read of size 8

==1846==    at 0x91B649: json_decref (jansson.h:111)

==1846==    by 0x91B649: json_delete_array (value.c:367)

==1846==    by 0x91B649: json_delete (value.c:938)

==1846==    by 0x9182A7: json_decref (jansson.h:112)

==1846==    by 0x9182A7: hashtable_do_clear (hashtable.c:142)

==1846==    by 0x918338: hashtable_close (hashtable.c:205)

==1846==    by 0x91B620: json_delete_object (value.c:78)

==1846==    by 0x91B620: json_delete (value.c:935)

==1846==    by 0x78E115: json_decref (jansson.h:112)

==1846==    by 0x79DE76: create_data (mdns_json.c:2205)

==1846==    by 0x74DEC4: main (mdns_main.c:278)

==1846==  Address 0x10def168 is 8 bytes inside a block of size 24 free'd

==1846==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)

==1846==    by 0x9182A7: json_decref (jansson.h:112)

==1846==    by 0x9182A7: hashtable_do_clear (hashtable.c:142)

==1846==    by 0x918338: hashtable_close (hashtable.c:205)

==1846==    by 0x91B620: json_delete_object (value.c:78)

==1846==    by 0x91B620: json_delete (value.c:935)

==1846==    by 0x78E115: json_decref (jansson.h:112)

==1846==    by 0x79D096: create_data (mdns_json.c:2102)

==1846==    by 0x74DEC4: main (mdns_main.c:278)

[Graeme Smecher]

Hi Rakesh,

Thanks for including code, and props for using Valgrind -- however, this
is too much code to reasonably ask a volunteer mailing list to debug for
you. Next time, please trim your example code to something manageable
that demonstrates the problem in isolation.

Specific comments below.

best,
Graeme

Here, you have retrieved the "root" object.

>         }
>         if ((srv_obj = json_object_get(root, SERVICE)) == NULL) {

Here, you have retrieved a borrowed reference to an object inside
"root". The reference counts in "root" are all still 1. It's not a
problem yet -- but read on.

>             JSON_FREE(root);
>             goto cleanup;
>         }
>         if ((proto_type_obj = json_object_get(root, DEVICE_TYPE)) == NULL) {
>             JSON_FREE(root);
>             goto cleanup;
>         }
>         proto_type |= json_integer_value(proto_type_obj);
>         if ((connected_to_obj = json_object_get(root, "DEVICE")) == NULL) {
>             printf("Failed to get DEVICE from mci_string");
>         } else {
>             connected_to = json_integer_value(connected_to_obj);
>         }
>         idx  = json_integer_value(srv_obj);
>         encountered_services[0] = 1;                                   
>       
>         if ((idx <= SVC_MAX) && (idx > 0)) {                           
>      
>             if(encountered_services[idx] == 0){
>                 if(json_array_append_new(srv_array, srv_obj) == 0){

Here, you have inserted the borrowed reference into "srv_array".
Although you now have 2 JSON objects referencing this object, its
reference count is still only 1. Again, you've set up a bug but haven't
triggered it yet: read on. (Consider "json_array_append" instead of
json_array_append_new.)

>                     encountered_services[idx] = 1; 
>                 } else {
>                     printf("Append unsuccessfull for service %d\n", idx);
>                 }
>             }
>         }
>         free_str_arr(mci_server);
>         JSON_FREE(root);

BUG! Here, you clean up "root" and all its children -- since you have
not increased the reference count to "srv_obj", then JSON objects shared
with "srv_array" are cleaned up too. Your "srv_array" object now holds
invalid references.

The above ~25 lines or so of code would be much simpler if you used
json_pack().