trafodion-codereview mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From DaveBirdsall <...@git.apache.org>
Subject [GitHub] trafodion pull request #1748: [TRAFODION-3236] fix the issue of buffer overr...
Date Fri, 23 Nov 2018 22:57:58 GMT
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/trafodion/pull/1748#discussion_r236024891
  
    --- Diff: core/sqf/src/tm/tm.cpp ---
    @@ -2789,7 +2789,7 @@ void tm_process_msg(BMS_SRE *pp_sre)
     {
         short                  lv_ret;
         char                   la_send_buffer[4096];
    -    char                   la_recv_buffer[sizeof(Tm_Req_Msg_Type)];
    +    char                   la_recv_buffer[pp_sre->sre_reqDataSize];
    --- End diff --
    
    It is not obvious to me why this change solves the problem. For example, at line 2808
below, we check pp_src->src_reqDataSize to see if it is too big to fit in la_recv_buffer,
and if so, we dynamically allocate a buffer la_recv_buffer_ddl for it. It looks like the remaining
code in the method makes assumptions that certain message types are always shorter than sizeof(Tm_Req_Msg_Type)
though; but then the logic would read the message into the wrong buffer and it would fail
in some other way. Could you provide an explanation of why it solves the problem?
    
    Also, I'm wondering if we could get rid of dynamically allocating la_recv_buffer_ddl and
just use la_recv_buffer, by using dynamic array sizing as you have done here. That would simplify
the logic and reduce heap pressure.
    
    @zcorrea, what do you think?


---

Mime
View raw message