mouse07410 / asn1c

The ASN.1 Compiler
http://lionet.info/asn1c/
BSD 2-Clause "Simplified" License
94 stars 71 forks source link

codes generated by asn1c crashed whiling decoding aper #49

Closed jeick2008 closed 5 years ago

jeick2008 commented 5 years ago

@velichkov Hello ,velichkov I got a strange problem, Could you help me ? Now the codes generated by asn1c can be compiled successfully, but it works wrong. the stack likes bellow: `0x0000000000c30cd4 in asn_get_few_bits (pd=0x7fffffffdd30, nbits=1) at ../src/asn_bit_data.c:108

1 0x0000000000c3fc87 in SEQUENCE_decode_aper (opt_codec_ctx=0x7fffffffdd70, td=0x12cd980 , constraints=0x0, sptr=0x7fffffffddc8,

pd=0x7fffffffdd30) at ../src/constr_SEQUENCE.c:1515

2 0x0000000000bffeb2 in aper_decode (opt_codec_ctx=0x7fffffffdd70, td=0x12cd980 , sptr=0x7fffffffddc8, buffer=0xa, size=165352176,

skip_bits=0, unused_bits=0) at ../src/per_decoder.c:171

3 0x0000000000c0194b in ANY_to_type_aper (st=0x9db27f0, td=0x12cd980 , struct_ptr=0x7fffffffde08) at ../src/ANY.c:237

4 0x0000000000500a59 in procInitReset (pc=14, value=0x9db27f0) at /mnt/hgfs/DPI/BC-DPI/src/http-parser/src/s1_mme_parser.c:1663

5 0x000000000050300a in parseInitialMessage (in_msg=0x9db0a08) at /mnt/hgfs/DPI/BC-DPI/src/http-parser/src/s1_mme_parser.c:2359

6 0x0000000000503416 in sctp_parse_s1ap (data=0x7fffe14001fe, len=28) at /mnt/hgfs/DPI/BC-DPI/src/http-parser/src`

the pcap is partOfS1_reset.zip It's same with sample-S1AP-PDU-8.aper (reset), but convert-example -iaper sample-S1AP-PDU-8.aper ,everything goes well. But, I make the codes to be static lib ,makefile is Makefile.zip I splite src & hdr file into src and include.

In my code (64bit platform), decoding : `" S1AP_PDU_t *pdu_t = NULL; asn_dec_rval_t rval = {0};

/* parse S1AP header */
rval = aper_decode(NULL, &asn_DEF_S1AP_PDU, (void **)&pdu_t,
                   data, len, 0, 0);"`

In GDB mode ,I found the pointer malloc in static lib within 32bit not 64bit.... the coredump runtime stack like above. Does anyting go wrong when generate static lib or invoking the interfaces of asn1c codes ? thank you so much.

velichkov commented 5 years ago

Please provide minimal reproducible test that is ready to be compiled and executed - including all asn.1, c files and makefiles. And also try to use markdown in you messages as for example the backtrace you provided is mostly unreadable.

jeick2008 commented 5 years ago

@velichkov @mouse07410 @brchiu s1ap_decoder_demo.zip this is my demo project. The asn1 encode data has been formated to C array in main.c and Output into the binary file. reset_asn_data_jeick2008.zip

Within ./convert-sample -iaper reset_asn_datajecik2008.asn everything is ok, but in my demo, it goes wrong. -_

velichkov commented 5 years ago

Within ./convert-sample -iaper reset_asn_data_jecik2008.asn everything is ok,

This is a pretty strong indication that the problem is not within the asn1c project but somewhere in your code

but in my demo, it goes wrong. -

First try fixing all warnings

/tmp/s1ap_decoder_demo/s1ap_parser.c: In function ‘procInitReset’:
/tmp/s1ap_decoder_demo/s1ap_parser.c:80:64: warning: passing argument 4 of ‘parseField’ from incompatible pointer type [-Wincompatible-pointer-types]
             parseField(i, field[i]->id, field[i]->criticality, &(field[i]->value)); \
                                                                ^~~~~~~~~~~~~~~~~~
/tmp/s1ap_decoder_demo/s1ap_parser.c:93:5: note: in expansion of macro ‘PARSE_CMD_’
     PARSE_CMD_(reset, asn_DEF_Reset,
     ^~~~~~~~~~
/tmp/s1ap_decoder_demo/s1ap_parser.c:66:70: note: expected ‘ANY_t *’ {aka ‘struct ANY *’} but argument is of type ‘struct ResetIEs__value *’
 static void parseField(int index, long id, long criticality, ANY_t * value)
                                                              ~~~~~~~~^~~~~
/tmp/s1ap_decoder_demo/s1ap_parser.c: At top level:
/tmp/s1ap_decoder_demo/s1ap_parser.c:133:41: warning: initialization of ‘void (*)(void **)’ from incompatible pointer type ‘void (*)(UnsuccessfulOutcome_t **)’ {aka ‘void (*)(struct UnsuccessfulOutcome **)’} [-Wincompatible-pointer-types]
     [S1AP_PDU_PR_unsuccessfulOutcome] = parseUnsuccessfulOutcome,
                                         ^~~~~~~~~~~~~~~~~~~~~~~~
/tmp/s1ap_decoder_demo/s1ap_parser.c:133:41: note: (near initialization for ‘msgclass_parser_array[3]’)

after commenting out these two lines it stops crashing so the problem is not in the aper_decode but somewhere in your code, probably in the parseInitialMessage function.

diff --git a/s1ap_parser.c b/s1ap_parser.c
index 26a1e3a..8391c69 100644
--- a/s1ap_parser.c
+++ b/s1ap_parser.c
@@ -144,8 +144,8 @@ void s1ap_parser(const void* data, size_t len)
     rval = aper_decode(NULL, &asn_DEF_S1AP_PDU, (void **)&pdu_t,
                        data, len, 0, 0);

-    if ((RC_OK == rval.code) && (NULL != msgclass_parser_array[pdu_t->present]))
-        msgclass_parser_array[pdu_t->present](&pdu_t->choice);
+    //if ((RC_OK == rval.code) && (NULL != msgclass_parser_array[pdu_t->present]))^M
+    //    msgclass_parser_array[pdu_t->present](&pdu_t->choice);^M

     ASN_STRUCT_FREE(asn_DEF_S1AP_PDU, pdu_t);
 }
jeick2008 commented 5 years ago

@velichkov So thanks for your help, I use the libasncodec.a instead of compiled by myself. trying to fix the warnings. so much thanks.

jeick2008 commented 5 years ago

@velichkov You're right, errors are caused by my code , But another question is when to free the pointers in structure, such as :


    asn_dec_rval_t rval = {0};

    /* parse S1AP header */
    rval = aper_decode(NULL, &asn_DEF_S1AP_PDU, (void **)&pdu_t,
                       data, len, 0, 0);
    ********* all asn1(aper) endocder infos have been decoded   *********
    ASN_STRUCT_FREE(asn_DEF_S1AP_PDU, pdu_t);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
typedef struct S1AP_PDU {
    S1AP_PDU_PR present;
    union S1AP_PDU_u {
        struct InitiatingMessage    *initiatingMessage;
        struct SuccessfulOutcome    *successfulOutcome;
        struct UnsuccessfulOutcome  *unsuccessfulOutcome;
        /*
         * This type is extensible,
         * possible extensions are below.
         */
    } choice;

    /* Context for parsing across buffer boundaries */
    asn_struct_ctx_t _asn_ctx;
} S1AP_PDU_t;```
the memory of pdu_t are freed by ASN_STRUCT_FREE, but the pointers in struct S1AP_PDU_t ((recursive, many pointers,such as pdu_t.choice.successfulOutcom)) are not freed, 
these pointers' memory need to be released manually after the information is used up?
Thank you so much.
velichkov commented 5 years ago

You're right, errors are caused by my code

What was the problem and how did you solved it?

the memory of pdu_t are freed by ASN_STRUCT_FREE, but the pointers in struct S1AP_PDU_t ((recursive, many pointers,such as pdu_t.choice.successfulOutcom)) are not freed,

ASN_STRUCT_FREE calls the free_struct method and in case of CHOICE or SEQUENCE it calls ASN_STRUCT_FREE recursively so it frees everything. Here is the related code

constr_TYPE.h

 69 /*
 70  * Free the structure including freeing the memory pointed to by ptr itself.
 71  */
 72 #define ASN_STRUCT_FREE(asn_DEF, ptr) \
 73     (asn_DEF).op->free_struct(&(asn_DEF), (ptr), ASFM_FREE_EVERYTHING)

S1AP-PDU.c

 65 asn_TYPE_descriptor_t asn_DEF_S1AP_PDU = {
 66     "S1AP-PDU",
 67     "S1AP-PDU",
 68     &asn_OP_CHOICE,

constr_CHOICE.c

1481 asn_TYPE_operation_t asn_OP_CHOICE = {
1482     CHOICE_free,
1483     CHOICE_print,
1220 void
1221 CHOICE_free(const asn_TYPE_descriptor_t *td, void *ptr,
1222             enum asn_struct_free_method method) {
1223     const asn_CHOICE_specifics_t *specs =
1224         (const asn_CHOICE_specifics_t *)td->specifics;
1225     unsigned present;
1226 
1227     if(!td || !ptr)
1228         return;
1229 
1230     ASN_DEBUG("Freeing %s as CHOICE", td->name);
1231 
1232     /*
1233      * Figure out which CHOICE element is encoded.
1234      */
1235     present = _fetch_present_idx(ptr, specs->pres_offset, specs->pres_size);
1236 
1237     /*
1238      * Free that element.
1239      */
1240     if(present > 0 && present <= td->elements_count) {
1241         asn_TYPE_member_t *elm = &td->elements[present-1];
1242         void *memb_ptr;
1243 
1244         if(elm->flags & ATF_POINTER) {
1245             memb_ptr = *(void **)((char *)ptr + elm->memb_offset);
1246             if(memb_ptr)
1247                 ASN_STRUCT_FREE(*elm->type, memb_ptr);
velichkov commented 5 years ago

And here is a valgrind log that shows that there are not memory leaks.

$ valgrind --tool=memcheck --leak-check=full ./s1ap_decoder 
==19051== Memcheck, a memory error detector
==19051== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==19051== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==19051== Command: ./s1ap_decoder
==19051== 
==19051== 
==19051== HEAP SUMMARY:
==19051==     in use at exit: 0 bytes in 0 blocks
==19051==   total heap usage: 27 allocs, 27 frees, 775 bytes allocated
==19051== 
==19051== All heap blocks were freed -- no leaks are possible
==19051== 
==19051== For counts of detected and suppressed errors, rerun with: -v
==19051== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
jeick2008 commented 5 years ago

thank you so much.