Double free corruption with alternative input layout

[tmarcu]

This patch (https://github.com/clearlinux/swupd-server/pull/45) causes a double free on two tests. Travis-ci is now enabled with the latest commits, so all the tests will be automatically run for following PRs to catch such regressions.

[pohly]

Do you have additional details about the double free? Was it detected by glibc or valgrind?

I've not seen that in my own testing. If it's in the code paths that only happen when building without the alternative input layout, then I some help tracking down the problem would be welcome.

[pohly]

Actually, trying the modified swupd-server with traditional meta-swupd (and thus the original input layout) won't be that hard to test for me - I'll try tomorrow and report back.

[phmccarty]

@pohly: @tmarcu and I were seeing the same two test failures, but they appeared to manifest for different reasons. My failures are below.

The "file-name-blacklisted" test fails with swupd_create_update: src/manifest.c:120: manifest_from_file: Assertion `0' failed..

The "state-file" test fails with a segfault. Backtrace:

#0  0x00007f842b0c626b in malloc_consolidate () from /usr/lib/libc.so.6
#1  0x00007f842b0c7d2a in _int_malloc () from /usr/lib/libc.so.6
#2  0x00007f842b0c9d44 in malloc () from /usr/lib/libc.so.6
#3  0x00007f842b0b5ab2 in __GI__IO_file_doallocate () from /usr/lib/libc.so.6
#4  0x00007f842b0c3ad6 in __GI__IO_doallocbuf () from /usr/lib/libc.so.6
#5  0x00007f842b0c2e38 in __GI__IO_file_overflow () from /usr/lib/libc.so.6
#6  0x00007f842b0c1f06 in __GI__IO_file_xsputn () from /usr/lib/libc.so.6
#7  0x00007f842b0b795d in __GI__IO_padn () from /usr/lib/libc.so.6
#8  0x00007f842b097e5e in vfprintf () from /usr/lib/libc.so.6
#9  0x00007f842b145ae9 in __fprintf_chk () from /usr/lib/libc.so.6
#10 0x0000000000407ba3 in fprintf (__fmt=0x40f5f0 "%3i.%03i %5s %s:%03i\t| %s\t| %s\t| %s\n", __stream=<optimized out>) at /usr/include/bits/stdio2.h:97
#11 __log_message (file=file@entry=0x0, msg=msg@entry=0x40f890 "Reading manifest", filename=filename@entry=0x40f6c8 "src/manifest.c", linenr=linenr@entry=126, fmt=fmt@entry=0x40d7f1 "%s") at src/log.c:125
#12 0x000000000040988a in manifest_from_file (version=0, component=component@entry=0x40f917 "full") at src/manifest.c:126
#13 0x0000000000402df4 in main (argc=<optimized out>, argv=<optimized out>) at src/create_update.c:316

[pohly]

After setting up compilation without meta-swupd and running "make check", I was able to reproduce the failure. As usual with C, the root cause was a missing pointer initialization in that particular code path. Here's the fix:

diff --git a/src/create_update.c b/src/create_update.c
index cb7bf66..93d58d0 100644
--- a/src/create_update.c
+++ b/src/create_update.c
@@ -140,7 +140,7 @@ static bool parse_options(int argc, char **argv)
 static void populate_dirs(int version)
 {
        char *newversiondir;
-       char *newversiondircontent;
+       char *newversiondircontent = NULL;

        string_or_die(&newversiondir, "%s/%d", image_dir, version);

Please test and merge PR #55.

[matthewrsj]

looks like this was resolved by #55 ?

[phmccarty]

@matthewrsj Yes, I think so. We did not follow up on that PR yet.