build-ng.sh failed on mksquashfs: segmentation fault

[GoogleCodeExporter]

What steps will reproduce the problem?
1. extract rfw.bin use extract-ng.sh, (unsquashfs_all.sh modified according to 
issue 56 already)
2. detected filesystem is squashfs-3.2-r2-lzma
3. build new firmware without modify any file, using ./build-ng.sh

What is the expected output? What do you see instead?
 Expect build-ng.sh success. The actual output is:
-------------------------------------------------
Building new squashfs file system...
Parallel mksquashfs: Using 4 processors
Creating little endian 3.0 filesystem on fmk/new-filesystem.squashfs, block 
size 65536.
[                                                                ]   1/569   
0%./build-ng.sh: line 53: 12789 Segmentation fault      $SUDO $MKFS "$ROOTFS" 
"$FSOUT" $ENDIANESS -all-root
Remaining free bytes in firmware image: 2891776
-------------------------------------------------

What version of the product are you using? On what operating system?

 firmware-mod-kit r301
 Ubuntu 11.10 x86_64

Please provide any additional information below.
 squashfs-3.2-r2-lzma 

Original issue reported on code.google.com by din...@gmail.com on 15 Mar 2012 at 6:00

Attachments:

• rfw.bin

[GoogleCodeExporter]

I checked the source code again. It seems the problem is:
In trunk/src/others/squashfs-3.2-r2-lzma/CPP/7zip/Compress/LZMA_Alone/comp.cc : 
217, function sqlzma_cm has this prototype: 
    sqlzma_cm(struct sqlzma_opts *opts, z_stream *stream, Bytef *next_in, uInt avail_in, Bytef *next_out, uInt avail_out);

In 
trunk/src/others/squashfs-3.2-r2-lzma/squashfs3.2-r2/squashfs-tools/mksquashfs.c
 : 601, caller use this:
     res = sqlzma_cm(un.un_lzma, stream, s, size, d, block_size);
     un_lzma is of int type.

It seems the first parameter is in wrong type.

Original comment by din...@gmail.com on 20 Mar 2012 at 1:09

[GoogleCodeExporter]

Please hold.

Original comment by jeremy.collake@gmail.com on 20 Mar 2012 at 1:23

[GoogleCodeExporter]

First, this is some messy ass code ;p. This was plucked from some firmware, so 
don't blame us, lol. Firmware authors often just hack onto the squashfs base 
code in haste.

The compiler should throw an error if it was wholly incompatible, not being 
able to convert it to the appropriate type without typecasting, assuming normal 
errors weren't suppressed or a less strict mode invoked. However, it appears 
that it may have thought the structure compatible since both start with two 
INTs (one containing only two INTs).

It is actually being passed an auto-typecast'd pointer to the first variable of 
this structure:

struct sqlzma_un {
    int         un_lzma;
    struct sized_buf    un_a[SQUN_LAST];
    unsigned char           un_prob[31960]; /* unlzma 64KB - 1MB */
    z_stream        un_stream;
#define un_cmbuf    un_stream.next_in
#define un_cmlen    un_stream.avail_in
#define un_resbuf   un_stream.next_out
#define un_resroom  un_stream.avail_out
#define un_reslen   un_stream.total_out
};

*Expecting:*

struct sqlzma_opts {
    unsigned int    try_lzma;
    unsigned int    dicsize;
};

Therefore, it may be referencing the second field of the first structure as the 
dictionary size. So, let's look at IT:

struct sized_buf {
    unsigned int    sz;
    unsigned char   *buf;
};

NOTICE, it IS an integer too - the size of the buffer the next pointer is 
pointing to. That may be why the compiler is allowing it if the compiler 
settings are not strict, since it sees a pointer to two INTs regardless.

So, you may be right. If this is a valid value, it may work fine - but it may 
crash if it is invalid. For instance, 1 is not a good dictionary size, lol ;).

For ME, on Ubuntu 11.10 x86, I was able to extract and build this firmware 
without a crash. So, that does suggest the possibility of a crash affected by 
potentially uninitialized memory that could vary from system to system, x32 - 
x64, or run to run.

However, this code needs further examination before I can say that for sure. I 
could 'just fix' it, but it isn't 'broken' in my test bed, so I suggest you 
experiment with it and verify that is the crash location. Like I said, I'm 
surprised it builds with such an error, so I suspect something is being 
overlooked...

Try inserting *before* line 601 something like this:
un.un_a[0].sz=64435; /* a reasonable dictionary size */

... and see what happens, just out of curiosity.

Once you verify that's the crash location, I'll be happy to patch it up. Though 
I need more time to look at it to ensure all is right, and gotta get back to 
paying work again for the time being.

Original comment by jeremy.collake@gmail.com on 20 Mar 2012 at 2:05

[GoogleCodeExporter]

Or, rather, do a proper fix (the last suggestion being a stupid hack just to 
try to make sure this is the crash cause):

sqlzma_opts sqopts;
sqopts.try_lzma=un.un_lzma;
sqopts.dicsize=64435;
res = sqlzma_cm(&sqopts, stream, s, size, d, block_size);

Original comment by jeremy.collake@gmail.com on 20 Mar 2012 at 2:11

[GoogleCodeExporter]

The author acknowledges on line 50 of sqlzma.h that this is 'very dirty code'. 
As such, I am not going to commit this change until there is verification it 
fixes the fault you see. While it should, I don't want to get into something 
else unless necessary. Let me know.

Original comment by jeremy.collake@gmail.com on 20 Mar 2012 at 1:15

[GoogleCodeExporter]

Hi, I can't make it work... Where do I have to add the lines of code? If I add 
them befor line 601 of mksquashfs.c the MAKE says me that sqlzma_opts is not 
defined anywhere...

Can you commit the bug-fixing or help me making changes?

Thank you, Jacopo.

Original comment by jacoporu...@hotmail.it on 30 May 2012 at 7:08

[GoogleCodeExporter]

Jacopo, are you using the same version of squashfs-lzma?
For other version, this fix maybe not working as well.
Or, just search for sqlzma_cm function call inside mksquashfs.c to determine 
the line number.

Original comment by din...@gmail.com on 2 Jun 2012 at 12:01

[GoogleCodeExporter]

I also faced the segfault problem but I can confirm that by changing 

res = sqlzma_cm(un.un_lzma, stream, s, size, d, block_size);

in /src/others/squashfs-3.2-r2-lzma/squashfs3.2-r2/squashfs-tools/mksquashfs.c 
: line 601

to

struct sqlzma_opts sqopts;
sqopts.try_lzma=un.un_lzma;
sqopts.dicsize=64435;
res = sqlzma_cm(&sqopts, stream, s, size, d, block_size);

and then rebuilding fixes the problem beautifully!

Original comment by Q1e...@gmail.com on 12 Dec 2012 at 2:10

[GoogleCodeExporter]

I will go ahead and commit this change. Should have long ago.

Original comment by jeremy.collake@gmail.com on 12 Dec 2012 at 2:31

[GoogleCodeExporter]

Original comment by jeremy.collake@gmail.com on 12 Dec 2012 at 2:35

• Changed state: Fixed