[vz-dev] vzlogger segfault

[Jürgen Herrmann]

ich werde mal eine neues System installieren...
auf dem nur vzlogger dgb sind
vzlogger von stv0g ... und dann noch mal  laufen lassen



Am 14.10.2012 18:46, schrieb Jürgen Herrmann:
> noch ein Info :
> das System ist ein debian 6.05 in einer Virtualbox mit 256 MB 
> Speicher, das Hostystem hat 4GB....
>
>
> Am 14.10.2012 17:15, schrieb Thorben Thuermer:
>> (mit CC an steffen (vzlogger) und juri (libsml)...)
>>
>> On Sun, 14 Oct 2012 13:36:25 +0200
>> Jürgen Herrmann <juergen.herrmann at plugid.de> wrote:
>>> Kann es, das eine Variable nicht beim Start initaliert wird...
>> ganz so einfach ist das leider nicht, aber... (s.u.)
>>
>>> auch schreibt es keine Daten in die Datenbank .....
>> das ist ein anderes problem und liegt eher an einem fehler in deiner
>> konfiguration...
>>
>> nochmal ganz deutlich:
>> das gesamte verhalten:
>> * keine daten werden geloggt
>> * der absturz unten
>> deutet daruf hin, das die daten die von deinem zaehler kommen
>> (fuer den sml-parser) voelliger muell sind,
>> deswegen koennen keine zu loggenden werte ermittelt werden,
>> und irgendwann kommt dann eine sequenz, die der parser
>> nicht verarbeiten kann und es fuehrt (aufgrund von bugs!) zum crash.
>> (es ist die frage, wieviel zeit man da reinstecken will,
>>   ungueltige daten "korrekt" zu verarbeiten, aber auf jeden
>>   fall sollte man unkontrollierte abstuerze vermeiden.)
>>
>>> hier mal eine Terinal ausgabe
>> der backtrace ist SUPER, wir kommen dem problem naeher:
>>
>>> gdb --args vzlogger -f -v20 --config /etc/vzlogger.conf
>>> (gdb) bt full
>>> #0 0xb7e5a367 in memset () from /lib/i686/cmov/libc.so.6
>>> No symbol table info available.
>>> #1 0x08051237 in sml_buffer_init (length=4294967280) at 
>>> src/sml_shared.c:118
>>> buf = 0x8066af0
>> sml_buffer_init alloziert einen pufferm mit der uebergebenen groesse.
>> es wird eine VIEL zu grosse groesse uebergeben (mehr unten),
>> 4294967280 = 4 gigabyte,
>> malloc schlaegt fehl und liefert 0,
>> aber der rueckgabewert von malloc wird nicht geprueft,
>> sondern es wird versucht ueber den nullpointer zu schreiben
>> => segfault.
>>
>> ein erster fix an der stelle:
>> sml_buffer *sml_buffer_init(size_t length) {
>>          sml_buffer *buf = (sml_buffer *) malloc(sizeof(sml_buffer));
>> +       if (!buf) return null;
>>
>>> #2 0x080506cb in sml_file_parse (buffer=0xb791a1b8 "",
>>> buffer_len=4294967280)
>>> at src/sml_file.c:35
>> diese funktion ist recht unschuldig, sollte dann aber auch die
>> rueckgabe von sml_buffer_init pruefen und bei fehlern abbrechen.
>>
>> sml_file *sml_file_parse(unsigned char *buffer, size_t buffer_len) {
>>          sml_file *file = (sml_file*) malloc(sizeof(sml_file));
>>          memset(file, 0, sizeof(sml_file));
>>
>>          sml_buffer *buf = sml_buffer_init(buffer_len);
>> +       if (!buf){ free file; return null;}
>>
>>> #3 0x0804fd26 in meter_read_sml (meter=0x8066ca0, rds=0x8067510, n=32)
>>> at protocols/sml.c:163
>>> buffer = '\000' <repeats 7660 times>,
>>> "h\272\336\267\020ii\rܿ\221\267f\274\376\267\217\210߷Y\231\004\b\000\000\000\000ە\004\b\030\365\222\267\002\000\221\267\340\030\377\267ە\004\bh\272\371\267\364\357\377\267\230\200\336\267\a\000\000\000\\\300\221\267\001\303\376\267", 
>>>
>> [...]
>>> bytes = 0
>>> m = <value optimized out>
>>> file = 0x0
>> und hier ist wohl der uebeltaeter:
>> 159         /* wait until a we receive a new datagram from the meter
>> 160         bytes = sml_transport_read(handle->fd, buffer,
>> SML_BUFFER_LEN);
>> 161
>> 162         /* parse SML file & stripping escape sequences */
>> 163         file = sml_file_parse(buffer + 8, bytes - 16);
>>
>> es wird als puffergroesse bytes - 16 an sml_file_parse uebergeben,
>> aber bytes ist wohl kleiner als 16, damit gibt es einen unterlauf/
>> wraparound bei der subtraktion, und wir bekommen die viel zu
>> grosse puffergroesse!
>>
>> sinnvoll waehre da ein:
>> 160         bytes = sml_transport_read(handle->fd, buffer,
>> SML_BUFFER_LEN);
>> +           if (bytes <= 16) {
>> +                // warnung ausgeben und abbrechen
>> und:
>> 163         file = sml_file_parse(buffer + 8, bytes - 16);
>> +           if (!file) { // ditto
>>
>> kenne mich fuer die details leider nicht gut genug im code aus,
>> aber der STEFFEN nimmt sich dessen sicher gerne an!
>>
>> und vielleicht sollte auch in sml_transport_read() noch eine
>> pruefung gemacht werden, ob die gelesenen daten lang genug fuer eine
>> gueltige sml-nachricht sind?
>>
>> - T.
>
>
>