GNOME Bugzilla – Bug 721177
typelib compiler writes uninitialised memory to typelib file
Last modified: 2015-02-07 16:55:14 UTC
I noticed that the result of building a .typelib file is non-deterministic and started to investigate why that is. It didn't appear to be based on timestamp (like non-determinism in .pyc or .a files) so I started reading the code and got a suspicion.... Checking with valgrind confirms that we are passing uninitialised data to write(): ==26496== Memcheck, a memory error detector ==26496== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. ==26496== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info ==26496== Command: /home/desrt/.cache/jhbuild/checkout/gobject-introspection/.libs/lt-g-ir-compiler --includedir=. --includedir=./gir --includedir=. --includedir=. --includedir=. gir/cairo-1.0.gir -o gir/cairo-1.0.typelib ==26496== ==26496== Syscall param write(buf) points to uninitialised byte(s) ==26496== at 0x3A6DCE6890: __write_nocancel (in /usr/lib64/libc-2.18.so) ==26496== by 0x3A6DC76D22: _IO_file_write@@GLIBC_2.2.5 (in /usr/lib64/libc-2.18.so) ==26496== by 0x3A6DC7818B: _IO_do_write@@GLIBC_2.2.5 (in /usr/lib64/libc-2.18.so) ==26496== by 0x3A6DC77A7F: _IO_file_close_it@@GLIBC_2.2.5 (in /usr/lib64/libc-2.18.so) ==26496== by 0x3A6DC6BC27: fclose@@GLIBC_2.2.5 (in /usr/lib64/libc-2.18.so) ==26496== by 0x406E49: write_out_typelib (compiler.c:99) ==26496== by 0x40723D: main (compiler.c:215) ==26496== Address 0x4e51643 is not stack'd, malloc'd or (recently) free'd
Created attachment 264980 [details] [review] typelib compiler: properly initialise memory The typelib compiler was writing uninitialised memory to the output file. There were two sources of this uninitialised memory: the hash writer included some uninitialised memory in its output,a nd the bytes added after the hash output for padding were also not being initialised. Fix this by having the hash code report the required size including padding (and don't add padding at the call site). Then, modify the writer function in the hash code to initialise the memory to zero before writing.
Review of attachment 264980 [details] [review]: Thanks a lot for this patch! Just one comment, please commit after that. ::: girepository/gthash.c @@ +140,3 @@ g_return_val_if_fail (builder->buildable, 0 ); + return ALIGN_VALUE (builder->packed_size, 4); I'd prefer to keep value alignment close to writing out the typelib; most of the ALIGN_VALUE calls are in girmodule.c and girnode.c. So instead the invocation of _gi_typelib_hash_builder_pack() gains an ALIGN_VALUE too, or we just do required_size = ALIGN_VALUE (required_size) after the call to _get_buffer_size().
Created attachment 265060 [details] [review] typelib compiler: properly initialise memory The typelib compiler was writing uninitialised memory to the output file. There were two sources of this uninitialised memory: the hash writer included some uninitialised memory in its output, and the bytes added after the hash output for padding were also not being initialised. Fix this by passing the padded size to the hash code writer function and having that function initialise the entire memory region to zero before writing.
Attachment 265060 [details] pushed as fe7fd5d - typelib compiler: properly initialise memory
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]