Hello, I'm using the Perl module SNMP.pm in asynchronous mode to query some values from some SNMP agents periodically in a daemon. This used to crash after some time, so I ran the whole thing (my daemon in Perl) under gdb to get backtraces and investigate the cause.
The SNMP.pm uses XS code (which is translated to C code during build) to
call snmplib. The XS code contains a callback function, __snmp_xs_cb,
which is supposed to be called back exactly once by snmplib: when the
SNMP reply was received or the request timed out. __snmp_xs_cb would
then call a callback implemented in Perl by the user of SNMP.pm.
Most of the time this works well.
But it turned out that sometimes __snmp_xs_cb would be called *twice*
(or maybe even more often) for the same SNMP request. The problem is,
__snmp_xs_cb uses some dynamically allocated data, cb_data, which is
freed once __snmp_xs_cb is called. So any call after the first
dereferenced a pointer to already freed memory. That use after free
resulted in a crash.
Using gdb and inspecting the local variables in __snmp_xs_cb after a
crash I could identify the conditions when this mysterious "superfluous
callback" was called:
int dont_free = (op==1 && reqid==0) ||
(pdu && pdu->command == SNMP_MSG_REPORT);
My problem is: I cannot interpret my findings!
op==1 means NETSNMP_CALLBACK_OP_RECEIVED_MESSAGE (I should have used the
symbolic value instead of the literal 1, but I got it from gdb), OK.
Every request is assigned a request id "reqid" by snmplib. A reqid of 0
would be a callback unrelated to any request? Of course not, because
cb_data belongs to a specific request! But what does this condition
indicate then?
SNMP_MSG_REPORT means some SNMPv3 problem. __snmp_xs_cb will also call
the Perl callback in this case, but to the Perl code it just looks like
a timeout. I think snmplib already handles this case, so it should not
reach the Perl code. Also compare
http://sourceforge.net/p/net-snmp/patches/1256/ . That patch even
prevents snmplib from calling its callback, fixing it one level earlier
in the call stack. But I cannot decide which fix is right.
Attached patch prevents the use-after-free condition and I would be
grateful if you could explain to me why it works and help me prepare it
for integration into net-snmp master.
Best regards,
Michael Bunk
From 98119418b4a020b9d08135d439b8a53ca7fd9a38 Mon Sep 17 00:00:00 2001
From: Michael Bunk <m...@computer-leipzig.com>
Date: Fri, 25 Apr 2014 15:09:19 +0200
Subject: [PATCH] Fix perl crash because of use after free
Under certain conditions (which I don't understand but which are represented by
the new variable dont_free), the callback function would be called twice. But
for the second callback, certain perl variables were already freed, leading to
use after free and perl crashing.
This patch fixes the situation by preventing the perl variables to be freed in
the first callback, so they are still available later, when the actual SNMP
reply arrives.
---
perl/SNMP/SNMP.xs | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/perl/SNMP/SNMP.xs b/perl/SNMP/SNMP.xs
index 2a734e8..a7ef97a 100644
--- a/perl/SNMP/SNMP.xs
+++ b/perl/SNMP/SNMP.xs
@@ -1198,6 +1198,7 @@ void *cb_data;
int old_numeric, old_printfull, old_format;
netsnmp_transport *transport = NULL;
+ int dont_free = (op==1 && reqid==0) || (pdu && pdu->command == SNMP_MSG_REPORT);
SV* cb = ((struct snmp_xs_cb_data*)cb_data)->perl_cb;
SV* sess_ref = ((struct snmp_xs_cb_data*)cb_data)->sess_ref;
SV **err_str_svp = hv_fetch((HV*)SvRV(sess_ref), "ErrorStr", 8, 1);
@@ -1207,8 +1208,13 @@ void *cb_data;
ENTER;
SAVETMPS;
- if (cb_data != ss->callback_magic)
+ if(dont_free)
+ {
+ }
+ else if (cb_data != ss->callback_magic)
+ {
free(cb_data);
+ }
sv_setpv(*err_str_svp, (char*)snmp_errstring(pdu->errstat));
sv_setiv(*err_num_svp, pdu->errstat);
@@ -1365,7 +1371,12 @@ void *cb_data;
default:;
} /* switch op */
if (cb_data != ss->callback_magic)
- sv_2mortal(cb);
+ {
+ if(!dont_free)
+ {
+ sv_2mortal(cb);
+ }
+ }
cb = __push_cb_args2(cb,
(SvTRUE(varlist_ref) ? sv_2mortal(varlist_ref):varlist_ref),
(SvTRUE(traplist_ref) ? sv_2mortal(traplist_ref):traplist_ref));
@@ -1374,7 +1385,12 @@ void *cb_data;
FREETMPS;
LEAVE;
if (cb_data != ss->callback_magic)
- sv_2mortal(sess_ref);
+ {
+ if(!dont_free)
+ {
+ sv_2mortal(sess_ref);
+ }
+ }
return 1;
}
--
1.7.10.4
smime.p7s
Description: S/MIME Cryptographic Signature
------------------------------------------------------------------------------ Start Your Social Network Today - Download eXo Platform Build your Enterprise Intranet with eXo Platform Software Java Based Open Source Intranet - Social, Extensible, Cloud Ready Get Started Now And Turn Your Intranet Into A Collaboration Platform http://p.sf.net/sfu/ExoPlatform
_______________________________________________ Net-snmp-coders mailing list Net-snmp-coders@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
