[vz-dev] vzlogger segfault

[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.