[Bro] rpc decoder possible double-free problem
rpang at cs.princeton.edu
Fri Aug 17 09:34:47 PDT 2007
Thanks a bunch for pointing out the problem.
One solution is to add &refcount to "RPC_Call" and make FindCall()
return a Ref'ed pointer. (~RPC_Reply() will not delete call_, but
But this solution is not perfect either, because it doesn't work for:
* * *
A potentially better solution is to add the "ref" at the let var definition:
call = ref(conn.FindCall(...))
But binpac currently does not recognize this syntax. It requires us to
add "ref" as a binpac keyword. I will work on it once I'm back next
* * *
Another possibility is to avoid cleaning up of &let variables. There
are a few caveats in this case:
1. All "withinput" variables should be released. This is easy to check.
2. Bytestring's should be released, too. This I plan to solve by
making every bytestring release itself when going out of scope.
3. Maybe others? I can't find any, but can't tell for sure.
I think I prefer the former solution. What do you think?
On 7/26/07, jmzhou.ml at gmail.com <jmzhou.ml at gmail.com> wrote:
> I notice that in ~RPC_Reply, there is a statement "delete call_;". However,
> this call_ is instantiated by calling t_context->connection()->FindCall(
> msg()->xid()). This means that it is not "new"ed. This call_ supposely is
> deleted in the ~RPC_Message. Thus, there is a potential double-free here.
> The root cause is that binpac generates the "delete call_" for &let field.
> This is incorrect behavior. However, I haven't figured out how to fix it
> in binpac.
> A quick fix to this problem is to modify the rpc-protocol.pac:RPC_Reply,
> replacing the RPC_AcceptedReply(call) and RPC_RejectedReply(call) with:
> The future is not set. There is no fate but what we make
> for ourselves. - Terminator II, Judgment Day
> Bro mailing list
> bro at bro-ids.org
More information about the Bro