Bug with corrupted squashfs image

Bug with corrupted squashfs image

[Eric Sesterhenn]

hi,

mounting the squashfs image at http://www.cccmz.de/~snakebyte/squashfs.9.img.bz2
results in the following oops:

[  113.485219] BUG: unable to handle kernel NULL pointer dereference at (null)
[  113.485615] IP: [<c032b94a>] zlib_inflate+0x85a/0x18d0
[  113.485883] Oops: 0002 [#1] DEBUG_PAGEALLOC
[  113.486123] last sysfs file: /sys/block/sda/size
[  113.486257] Modules linked in: nfsd exportfs nfs lockd nfs_acl auth_rpcgss sunrpc ipv6 fuse unix
[  113.487294] 
[  113.487464] Pid: 4461, comm: mount Not tainted (2.6.29-rc1 #90) 
[  113.487609] EIP: 0060:[<c032b94a>] EFLAGS: 00010246 CPU: 0
[  113.487752] EIP is at zlib_inflate+0x85a/0x18d0
[  113.487889] EAX: 00000000 EBX: 00000003 ECX: 00000001 EDX: 00000000
[  113.488049] ESI: 00000000 EDI: c7df452b EBP: c7d59c78 ESP: c7d59b6c
[  113.488049]  DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
[  113.488049] Process mount (pid: 4461, ti=c7d59000 task=c7dd3710 task.ti=c7d59000)
[  113.488049] Stack:
[  113.488049]  c7df0068 c7df0054 c7df02ec 00000036 00000002 00000000 c7d59c08 c014ee4c
[  113.488049]  00000083 00000031 00000003 00000138 0000011e 00000000 c7990f98 c7df0000
[  113.488049]  c7df052c c7df02ec c7df0050 c7df0068 c7df006c c7df052c 0000003f c7df0054
[  113.488049] Call Trace:
[  113.488049]  [<c014ee4c>] ? __lock_acquire+0x26c/0x1110
[  113.488049]  [<c0140001>] ? posix_cpu_clock_get+0x1/0x160
[  113.488049]  [<c062a421>] ? mutex_lock_nested+0x1f1/0x2c0
[  113.488049]  [<c062a42b>] ? mutex_lock_nested+0x1fb/0x2c0
[  113.488049]  [<c022277d>] ? squashfs_read_data+0x3fd/0x830
[  113.488049]  [<c02228b9>] ? squashfs_read_data+0x539/0x830
[  113.488049]  [<c022303c>] ? squashfs_cache_get+0x25c/0x330
[  113.488049]  [<c014e941>] ? trace_hardirqs_on_caller+0x151/0x1c0
[  113.488049]  [<c02231ff>] ? squashfs_read_metadata+0x6f/0x140
[  113.488049]  [<c0224f32>] ? squashfs_read_inode+0x82/0x7e0
[  113.488049]  [<c01ba039>] ? new_inode+0x79/0x80
[  113.488049]  [<c02262e2>] ? squashfs_fill_super+0x4c2/0x9a0
[  113.488049]  [<c01a9633>] ? get_sb_bdev+0x123/0x150
[  113.488049]  [<c01b0030>] ? do_lookup+0x130/0x1c0
[  113.488049]  [<c018ba01>] ? kstrdup+0x31/0x60
[  113.488049]  [<c0225c61>] ? squashfs_get_sb+0x21/0x30
[  113.488049]  [<c0225e20>] ? squashfs_fill_super+0x0/0x9a0
[  113.488049]  [<c01a8e69>] ? vfs_kern_mount+0x59/0x130
[  113.488049]  [<c01a8f99>] ? do_kern_mount+0x39/0xe0
[  113.488049]  [<c01be4b4>] ? do_mount+0x434/0x7b0
[  113.488049]  [<c01bc9ac>] ? copy_mount_options+0x3c/0x130
[  113.488049]  [<c01be8b4>] ? sys_mount+0x84/0xb0
[  113.488049]  [<c0103551>] ? sysenter_do_call+0x12/0x31
[  113.488049] Code: 70 ff ff ff 3b 85 70 ff ff ff 8b b5 30 ff ff ff 0f 46 c8 29 ca 29 8d 70 ff ff ff 89 56 3c 31 d2 90 0f b6 04 17 8b b5 6c ff ff ff <88> 04 16 83 c2 01 39 ca 75 ec 8b 85 30 ff ff ff 01 d6 89 b5 6c 
[  113.488049] EIP: [<c032b94a>] zlib_inflate+0x85a/0x18d0 SS:ESP 0068:c7d59b6c
[  113.502261] ---[ end trace 42a589fe0cbc2ff1 ]---


(gdb) l *(zlib_inflate+0x85a)
0xc032b94a is in zlib_inflate (lib/zlib_inflate/inflate.c:683).
678	            }
679	            if (copy > left) copy = left;
680	            left -= copy;
681	            state->length -= copy;
682	            do {
683	                *put++ = *from++;
684	            } while (--copy);
685	            if (state->length == 0) state->mode = LEN;
686	            break;
687	        case LIT:



I already reported a similar issue to Phillip but got no reply (lost in spam?),
so I duplicate it here:

[ 6053.337097] BUG: unable to handle kernel NULL pointer dereference at
(null)
[ 6053.337320] IP: [<c055118f>] zlib_inflate+0xfcc/0x15a9
[ 6053.337488] *pde = 00000000
[ 6053.337619] Oops: 0002 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[ 6053.337892] last sysfs file: /sys/block/ram9/range
[ 6053.337994] Modules linked in:
[ 6053.338020]
[ 6053.338020] Pid: 28143, comm: cat Tainted: G        W
(2.6.28-09185-g71dd273 #182) System Name
[ 6053.338020] EIP: 0060:[<c055118f>] EFLAGS: 00010206 CPU: 0
[ 6053.338020] EIP is at zlib_inflate+0xfcc/0x15a9
[ 6053.338020] EAX: 00000075 EBX: c1820000 ECX: 00001000 EDX: 00000000
[ 6053.338020] ESI: c182052c EDI: c18202ec EBP: cc37cc28 ESP: cc37cb10
[ 6053.338020]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[ 6053.338020] Process cat (pid: 28143, ti=cc37c000 task=cc328000
task.ti=cc37c000)
[ 6053.338020] Stack:
[ 6053.338020]  c07f0c15 c10ea164 c07f0c5e 00000001 c18202ec c182052c
000007ff 000001ff
[ 6053.338020]  c17477c0 cc37cbb0 00000000 cba5dd38 c182052c 66fee13b
00000581 c1820d74
[ 6053.338020]  cd050768 00000000 00000006 00001000 00000001 00000001
00000368 00000000
[ 6053.338020] Call Trace:
[ 6053.338020]  [<c07f0c15>] ? schedule+0x91d/0x943
[ 6053.338020]  [<c07f0c5e>] ? io_schedule+0x23/0x2d
[ 6053.338020]  [<c07f322c>] ? _spin_unlock_irqrestore+0x47/0x5d
[ 6053.338020]  [<c07f1021>] ? out_of_line_wait_on_bit+0x5d/0x65
[ 6053.338020]  [<c01ba5b8>] ? sync_buffer+0x0/0x3f
[ 6053.338020]  [<c013f715>] ? wake_bit_function+0x0/0x48
[ 6053.338020]  [<c02632e0>] ? squashfs_read_data+0x56c/0x770
[ 6053.338020]  [<c07f314e>] ? _spin_unlock+0x2c/0x41
[ 6053.338020]  [<c026383f>] ? squashfs_cache_get+0x155/0x29f
[ 6053.338020]  [<c02636e2>] ? squashfs_cache_put+0x53/0x5b
[ 6053.338020]  [<c02639a7>] ? squashfs_get_datablock+0x1e/0x23
[ 6053.338020]  [<c0264e5c>] ? squashfs_readpage+0x90d/0xb45
[ 6053.338020]  [<c017a6e7>] ? add_to_page_cache_locked+0x5e/0xbc
[ 6053.338020]  [<c014dd5d>] ? trace_hardirqs_on+0xb/0xd
[ 6053.338020]  [<c01814a0>] ? __do_page_cache_readahead+0x13a/0x16a
[ 6053.338020]  [<c01816c3>] ? ondemand_readahead+0x108/0x116
[ 6053.338020]  [<c0181752>] ? page_cache_sync_readahead+0x1b/0x20
[ 6053.338020]  [<c017ba12>] ? generic_file_aio_read+0x227/0x539
[ 6053.338020]  [<c019f702>] ? do_sync_read+0xc0/0xfe
[ 6053.338020]  [<c01a2541>] ? cp_new_stat64+0xed/0xff
[ 6053.338020]  [<c013f6e0>] ? autoremove_wake_function+0x0/0x35
[ 6053.338020]  [<c01a29aa>] ? sys_fstat64+0x27/0x2d
[ 6053.338020]  [<c019f642>] ? do_sync_read+0x0/0xfe
[ 6053.338020]  [<c019fe25>] ? vfs_read+0x8f/0x10b
[ 6053.338020]  [<c01a013d>] ? sys_read+0x40/0x65
[ 6053.338020]  [<c0102f21>] ? sysenter_do_call+0x12/0x31
[ 6053.338020] Code: ff 01 c7 89 bd 2c ff ff ff 83 7b 3c 00 0f 85 0c f1
ff ff eb 25 83 bd 34 ff ff ff 00 0f 84 b7 02 00 00 8b 43 3c 8b 95 2c ff
ff ff <88> 02 42 ff 8d 34 ff ff ff 89 95 2c ff ff ff c7 03 12 00 00 00
[ 6053.338020] EIP: [<c055118f>] zlib_inflate+0xfcc/0x15a9 SS:ESP
0068:cc37cb10
[ 6053.452067] ---[ end trace 4eaa2a86a8e2da24 ]---

(gdb) l *(zlib_inflate+0xfcc)
0xc055118f is in zlib_inflate (lib/zlib_inflate/inflate.c:689).
684                 } while (--copy);
685                 if (state->length == 0) state->mode = LEN;
686                 break;
687             case LIT:
688                 if (left == 0) goto inf_leave;
689                 *put++ = (unsigned char)(state->length);
690                 left--;
691                 state->mode = LEN;
692                 break;
693             case CHECK:


This image can be found at http://www.cccmz.de/~snakebyte/squashfs.4.img


Greetings, Eric

[Patch] NULL pointer deref with corrupted squashfs image

[Eric Sesterhenn]

* Eric Sesterhenn (snakebyte@gmx.de) wrote:
> hi,
> 
> mounting the squashfs image at http://www.cccmz.de/~snakebyte/squashfs.9.img.bz2
> results in the following oops:
> 
> [  113.485219] BUG: unable to handle kernel NULL pointer dereference at (null)
> [  113.485615] IP: [<c032b94a>] zlib_inflate+0x85a/0x18d0
> [  113.485883] Oops: 0002 [#1] DEBUG_PAGEALLOC
> [  113.486123] last sysfs file: /sys/block/sda/size
> [  113.486257] Modules linked in: nfsd exportfs nfs lockd nfs_acl auth_rpcgss sunrpc ipv6 fuse unix
> [  113.487294] 
> [  113.487464] Pid: 4461, comm: mount Not tainted (2.6.29-rc1 #90) 
> [  113.487609] EIP: 0060:[<c032b94a>] EFLAGS: 00010246 CPU: 0
> [  113.487752] EIP is at zlib_inflate+0x85a/0x18d0
> [  113.487889] EAX: 00000000 EBX: 00000003 ECX: 00000001 EDX: 00000000
> [  113.488049] ESI: 00000000 EDI: c7df452b EBP: c7d59c78 ESP: c7d59b6c
> [  113.488049]  DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
> [  113.488049] Process mount (pid: 4461, ti=c7d59000 task=c7dd3710 task.ti=c7d59000)
> [  113.488049] Stack:
> [  113.488049]  c7df0068 c7df0054 c7df02ec 00000036 00000002 00000000 c7d59c08 c014ee4c
> [  113.488049]  00000083 00000031 00000003 00000138 0000011e 00000000 c7990f98 c7df0000
> [  113.488049]  c7df052c c7df02ec c7df0050 c7df0068 c7df006c c7df052c 0000003f c7df0054
> [  113.488049] Call Trace:
> [  113.488049]  [<c014ee4c>] ? __lock_acquire+0x26c/0x1110
> [  113.488049]  [<c0140001>] ? posix_cpu_clock_get+0x1/0x160
> [  113.488049]  [<c062a421>] ? mutex_lock_nested+0x1f1/0x2c0
> [  113.488049]  [<c062a42b>] ? mutex_lock_nested+0x1fb/0x2c0
> [  113.488049]  [<c022277d>] ? squashfs_read_data+0x3fd/0x830
> [  113.488049]  [<c02228b9>] ? squashfs_read_data+0x539/0x830
> [  113.488049]  [<c022303c>] ? squashfs_cache_get+0x25c/0x330
> [  113.488049]  [<c014e941>] ? trace_hardirqs_on_caller+0x151/0x1c0
> [  113.488049]  [<c02231ff>] ? squashfs_read_metadata+0x6f/0x140
> [  113.488049]  [<c0224f32>] ? squashfs_read_inode+0x82/0x7e0
> [  113.488049]  [<c01ba039>] ? new_inode+0x79/0x80
> [  113.488049]  [<c02262e2>] ? squashfs_fill_super+0x4c2/0x9a0
> [  113.488049]  [<c01a9633>] ? get_sb_bdev+0x123/0x150
> [  113.488049]  [<c01b0030>] ? do_lookup+0x130/0x1c0
> [  113.488049]  [<c018ba01>] ? kstrdup+0x31/0x60
> [  113.488049]  [<c0225c61>] ? squashfs_get_sb+0x21/0x30
> [  113.488049]  [<c0225e20>] ? squashfs_fill_super+0x0/0x9a0
> [  113.488049]  [<c01a8e69>] ? vfs_kern_mount+0x59/0x130
> [  113.488049]  [<c01a8f99>] ? do_kern_mount+0x39/0xe0
> [  113.488049]  [<c01be4b4>] ? do_mount+0x434/0x7b0
> [  113.488049]  [<c01bc9ac>] ? copy_mount_options+0x3c/0x130
> [  113.488049]  [<c01be8b4>] ? sys_mount+0x84/0xb0
> [  113.488049]  [<c0103551>] ? sysenter_do_call+0x12/0x31
> [  113.488049] Code: 70 ff ff ff 3b 85 70 ff ff ff 8b b5 30 ff ff ff 0f 46 c8 29 ca 29 8d 70 ff ff ff 89 56 3c 31 d2 90 0f b6 04 17 8b b5 6c ff ff ff <88> 04 16 83 c2 01 39 ca 75 ec 8b 85 30 ff ff ff 01 d6 89 b5 6c 
> [  113.488049] EIP: [<c032b94a>] zlib_inflate+0x85a/0x18d0 SS:ESP 0068:c7d59b6c
> [  113.502261] ---[ end trace 42a589fe0cbc2ff1 ]---
> 
> 
> (gdb) l *(zlib_inflate+0x85a)
> 0xc032b94a is in zlib_inflate (lib/zlib_inflate/inflate.c:683).
> 678	            }
> 679	            if (copy > left) copy = left;
> 680	            left -= copy;
> 681	            state->length -= copy;
> 682	            do {
> 683	                *put++ = *from++;
> 684	            } while (--copy);
> 685	            if (state->length == 0) state->mode = LEN;
> 686	            break;
> 687	        case LIT:
> 
> 
> 
> I already reported a similar issue to Phillip but got no reply (lost in spam?),
> so I duplicate it here:
> 
> [ 6053.337097] BUG: unable to handle kernel NULL pointer dereference at
> (null)
> [ 6053.337320] IP: [<c055118f>] zlib_inflate+0xfcc/0x15a9
> [ 6053.337488] *pde = 00000000
> [ 6053.337619] Oops: 0002 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> [ 6053.337892] last sysfs file: /sys/block/ram9/range
> [ 6053.337994] Modules linked in:
> [ 6053.338020]
> [ 6053.338020] Pid: 28143, comm: cat Tainted: G        W
> (2.6.28-09185-g71dd273 #182) System Name
> [ 6053.338020] EIP: 0060:[<c055118f>] EFLAGS: 00010206 CPU: 0
> [ 6053.338020] EIP is at zlib_inflate+0xfcc/0x15a9
> [ 6053.338020] EAX: 00000075 EBX: c1820000 ECX: 00001000 EDX: 00000000
> [ 6053.338020] ESI: c182052c EDI: c18202ec EBP: cc37cc28 ESP: cc37cb10
> [ 6053.338020]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> [ 6053.338020] Process cat (pid: 28143, ti=cc37c000 task=cc328000
> task.ti=cc37c000)
> [ 6053.338020] Stack:
> [ 6053.338020]  c07f0c15 c10ea164 c07f0c5e 00000001 c18202ec c182052c
> 000007ff 000001ff
> [ 6053.338020]  c17477c0 cc37cbb0 00000000 cba5dd38 c182052c 66fee13b
> 00000581 c1820d74
> [ 6053.338020]  cd050768 00000000 00000006 00001000 00000001 00000001
> 00000368 00000000
> [ 6053.338020] Call Trace:
> [ 6053.338020]  [<c07f0c15>] ? schedule+0x91d/0x943
> [ 6053.338020]  [<c07f0c5e>] ? io_schedule+0x23/0x2d
> [ 6053.338020]  [<c07f322c>] ? _spin_unlock_irqrestore+0x47/0x5d
> [ 6053.338020]  [<c07f1021>] ? out_of_line_wait_on_bit+0x5d/0x65
> [ 6053.338020]  [<c01ba5b8>] ? sync_buffer+0x0/0x3f
> [ 6053.338020]  [<c013f715>] ? wake_bit_function+0x0/0x48
> [ 6053.338020]  [<c02632e0>] ? squashfs_read_data+0x56c/0x770
> [ 6053.338020]  [<c07f314e>] ? _spin_unlock+0x2c/0x41
> [ 6053.338020]  [<c026383f>] ? squashfs_cache_get+0x155/0x29f
> [ 6053.338020]  [<c02636e2>] ? squashfs_cache_put+0x53/0x5b
> [ 6053.338020]  [<c02639a7>] ? squashfs_get_datablock+0x1e/0x23
> [ 6053.338020]  [<c0264e5c>] ? squashfs_readpage+0x90d/0xb45
> [ 6053.338020]  [<c017a6e7>] ? add_to_page_cache_locked+0x5e/0xbc
> [ 6053.338020]  [<c014dd5d>] ? trace_hardirqs_on+0xb/0xd
> [ 6053.338020]  [<c01814a0>] ? __do_page_cache_readahead+0x13a/0x16a
> [ 6053.338020]  [<c01816c3>] ? ondemand_readahead+0x108/0x116
> [ 6053.338020]  [<c0181752>] ? page_cache_sync_readahead+0x1b/0x20
> [ 6053.338020]  [<c017ba12>] ? generic_file_aio_read+0x227/0x539
> [ 6053.338020]  [<c019f702>] ? do_sync_read+0xc0/0xfe
> [ 6053.338020]  [<c01a2541>] ? cp_new_stat64+0xed/0xff
> [ 6053.338020]  [<c013f6e0>] ? autoremove_wake_function+0x0/0x35
> [ 6053.338020]  [<c01a29aa>] ? sys_fstat64+0x27/0x2d
> [ 6053.338020]  [<c019f642>] ? do_sync_read+0x0/0xfe
> [ 6053.338020]  [<c019fe25>] ? vfs_read+0x8f/0x10b
> [ 6053.338020]  [<c01a013d>] ? sys_read+0x40/0x65
> [ 6053.338020]  [<c0102f21>] ? sysenter_do_call+0x12/0x31
> [ 6053.338020] Code: ff 01 c7 89 bd 2c ff ff ff 83 7b 3c 00 0f 85 0c f1
> ff ff eb 25 83 bd 34 ff ff ff 00 0f 84 b7 02 00 00 8b 43 3c 8b 95 2c ff
> ff ff <88> 02 42 ff 8d 34 ff ff ff 89 95 2c ff ff ff c7 03 12 00 00 00
> [ 6053.338020] EIP: [<c055118f>] zlib_inflate+0xfcc/0x15a9 SS:ESP
> 0068:cc37cb10
> [ 6053.452067] ---[ end trace 4eaa2a86a8e2da24 ]---
> 
> (gdb) l *(zlib_inflate+0xfcc)
> 0xc055118f is in zlib_inflate (lib/zlib_inflate/inflate.c:689).
> 684                 } while (--copy);
> 685                 if (state->length == 0) state->mode = LEN;
> 686                 break;
> 687             case LIT:
> 688                 if (left == 0) goto inf_leave;
> 689                 *put++ = (unsigned char)(state->length);
> 690                 left--;
> 691                 state->mode = LEN;
> 692                 break;
> 693             case CHECK:
> 
> 
> This image can be found at http://www.cccmz.de/~snakebyte/squashfs.4.img


here is a patch to fix both issues. I tested it with ~8000
corrupted images and the only issue i saw was an overwritten
redzone which I can also reproduce without the patch:

[  282.001181]
=============================================================================
[  282.001416] BUG kmalloc-32: Redzone overwritten
[  282.001545]
-----------------------------------------------------------------------------
[  282.001550] 
[  282.001840] INFO: 0xcb928d90-0xcb928d93. First byte 0x28 instead of
0xcc
[  282.002051] INFO: Allocated in squashfs_read_data+0x46/0x768 age=1
cpu=0 pid=5003
[  282.002051] INFO: Freed in squashfs_read_data+0x702/0x768 age=1 cpu=0
pid=5003
[  282.002051] INFO: Slab 0xc1317500 objects=51 used=50 fp=0xcb928dc0
flags=0x400000c3
[  282.002051] INFO: Object 0xcb928d70 @offset=3440 fp=0xcb928dc0
[  282.002051] 
[  282.002051] Bytes b4 0xcb928d60:  00 00 00 00 00 00 00 00 5a 5a 5a 5a
5a 5a 5a 5a ........ZZZZZZZZ
[  282.002051]   Object 0xcb928d70:  38 8e 00 c9 d0 8d 00 c9 68 8d 00 c9
08 8f 7e cc 8..��..�h..�..~
[  282.002051]   Object 0xcb928d80:  a0 8e 7e cc 38 8e 7e cc d0 8d 7e cc
90 ca 00 c9 ..~�8.~��.~�.�.
[  282.002051]  Redzone 0xcb928d90:  28 ca 00 c9
(�.�            
[  282.002051]  Padding 0xcb928db8:  5a 5a 5a 5a 5a 5a 5a 5a
ZZZZZZZZ        
[  282.002051] Pid: 5003, comm: mount Not tainted
2.6.29-rc1-00541-g5a6c0f1 #209
[  282.002051] Call Trace:
[  282.002051]  [<c018a769>] print_trailer+0xcd/0xd5
[  282.002051]  [<c018a7e9>] check_bytes_and_report+0x78/0x94
[  282.002051]  [<c018aa08>] check_object+0x49/0x191
[  282.002051]  [<c018b9ef>] __slab_free+0x198/0x287
[  282.002051]  [<c018bc8a>] kfree+0xc2/0xe9
[  282.002051]  [<c0252a82>] ? squashfs_read_data+0x702/0x768
[  282.002051]  [<c0252a82>] ? squashfs_read_data+0x702/0x768
[  282.002051]  [<c0252a82>] squashfs_read_data+0x702/0x768
[  282.002051]  [<c0107084>] ? native_sched_clock+0x41/0x68
[  282.002051]  [<c0252e42>] squashfs_cache_get+0x154/0x29d
[  282.002051]  [<c0253065>] squashfs_read_metadata+0x94/0x106
[  282.002051]  [<c02549ea>] squashfs_read_inode+0xbe/0x743
[  282.002051]  [<c019f71f>] ? new_inode+0x7b/0x81
[  282.002051]  [<c0256036>] squashfs_fill_super+0x8a7/0x9e1
[  282.002051]  [<c01cc844>] ? disk_name+0x2a/0x6c
[  282.002051]  [<c01919c5>] get_sb_bdev+0xf1/0x13f
[  282.002051]  [<c0178896>] ? kstrdup+0x2f/0x51
[  282.002051]  [<c02555d3>] squashfs_get_sb+0x18/0x1a
[  282.002051]  [<c025578f>] ? squashfs_fill_super+0x0/0x9e1
[  282.002051]  [<c019159c>] vfs_kern_mount+0x40/0x7b
[  282.002051]  [<c0191625>] do_kern_mount+0x37/0xbf
[  282.002051]  [<c01a2cb0>] do_mount+0x5cc/0x609
[  282.002051]  [<c07c6ecb>] ? lock_kernel+0x19/0x8c
[  282.002051]  [<c01a2d43>] ? sys_mount+0x56/0xa0
[  282.002051]  [<c01a2d56>] sys_mount+0x69/0xa0
[  282.002051]  [<c0102ea1>] sysenter_do_call+0x12/0x31
[  282.002051] FIX kmalloc-32: Restoring 0xcb928d90-0xcb928d93=0xcc
[  282.002051] 
[  282.011166] SQUASHFS error: sb_bread failed reading block
0x3fffee6bc00004
[  282.011319] SQUASHFS error: Unable to read metadata cache entry
[ffffb9af0000138b]
[  282.011527] SQUASHFS error: Unable to read inode 0x1f93

The image for the issue above can be found at
http://www.cccmz.de/~snakebyte/squashfs.7668.img


Non-PPC targets shouldnt inflate images to memory address 0.
check strm->next_out for NULL in case on non PPC architecture
to prevent a NULL-pointer dereference while inflating corrupted images.

Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>

--- linux/lib/zlib_inflate/inflate.c.orig	2009-01-16 15:40:04.000000000 +0100
+++ linux/lib/zlib_inflate/inflate.c	2009-01-16 15:41:42.000000000 +0100
@@ -347,8 +347,12 @@ int zlib_inflate(z_streamp strm, int flu
     static const unsigned short order[19] = /* permutation of code lengths */
         {16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15};
 
-    /* Do not check for strm->next_out == NULL here as ppc zImage
-       inflates to strm->next_out = 0 */
+    /* Since ppc zImage inflates to 0 we only check
+       strm->next_out for non-ppc targets0 */
+#ifndef CONFIG_PPC
+    if (!strm->next_out)
+        return Z_STREAM_ERROR;
+#endif
 
     if (strm == NULL || strm->state == NULL ||
         (strm->next_in == NULL && strm->avail_in != 0))
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Jörn Engel]

On Fri, 16 January 2009 18:45:25 +0100, Eric Sesterhenn wrote:
> 
> Non-PPC targets shouldnt inflate images to memory address 0.
> check strm->next_out for NULL in case on non PPC architecture
> to prevent a NULL-pointer dereference while inflating corrupted images.
> 
> Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>
> 
> --- linux/lib/zlib_inflate/inflate.c.orig	2009-01-16 15:40:04.000000000 +0100
> +++ linux/lib/zlib_inflate/inflate.c	2009-01-16 15:41:42.000000000 +0100
> @@ -347,8 +347,12 @@ int zlib_inflate(z_streamp strm, int flu
>      static const unsigned short order[19] = /* permutation of code lengths */
>          {16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15};
>  
> -    /* Do not check for strm->next_out == NULL here as ppc zImage
> -       inflates to strm->next_out = 0 */
> +    /* Since ppc zImage inflates to 0 we only check
> +       strm->next_out for non-ppc targets0 */
> +#ifndef CONFIG_PPC
> +    if (!strm->next_out)
> +        return Z_STREAM_ERROR;
> +#endif
>  
>      if (strm == NULL || strm->state == NULL ||
>          (strm->next_in == NULL && strm->avail_in != 0))

Unzipping to NULL is not an attribute of PPC, but rather of being called
from a bootloader that wants to unpack a kernel to NULL.  Which makes
this patch wrong on two accounts.  It leaves the bug for CONFIG_PPC and
it may break bootloaders on other architectures.  A quick grep shows
xtensa - no clue whether it loads the kernel to NULL or elsewhere.

I'd prefer zlib_inflate to take a flag parameter to disable the check.
Then we can have two wrappers roughly like this:

int zlib_inflate(z_streamp strm, int flush)
{
	return __zlib_inflate(strm, flush, 1);
}

int zlib_inflate_null_ok_for_bootloaders_only(z_streamp strm, int flush)
{
	return __zlib_inflate(strm, flush, 0);
}

Or we could even make the two wrappers inline functions and move them to
zlib.h.

Jörn

-- 
He that composes himself is wiser than he that composes a book.
-- B. Franklin
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Tom Rini]

On Fri, Jan 16, 2009 at 08:07:32PM +0100, Jörn Engel wrote:
> On Fri, 16 January 2009 18:45:25 +0100, Eric Sesterhenn wrote:
> > 
> > Non-PPC targets shouldnt inflate images to memory address 0.
> > check strm->next_out for NULL in case on non PPC architecture
> > to prevent a NULL-pointer dereference while inflating corrupted images.
> > 
> > Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>
> > 
> > --- linux/lib/zlib_inflate/inflate.c.orig	2009-01-16 15:40:04.000000000 +0100
> > +++ linux/lib/zlib_inflate/inflate.c	2009-01-16 15:41:42.000000000 +0100
> > @@ -347,8 +347,12 @@ int zlib_inflate(z_streamp strm, int flu
> >      static const unsigned short order[19] = /* permutation of code lengths */
> >          {16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15};
> >  
> > -    /* Do not check for strm->next_out == NULL here as ppc zImage
> > -       inflates to strm->next_out = 0 */
> > +    /* Since ppc zImage inflates to 0 we only check
> > +       strm->next_out for non-ppc targets0 */
> > +#ifndef CONFIG_PPC
> > +    if (!strm->next_out)
> > +        return Z_STREAM_ERROR;
> > +#endif
> >  
> >      if (strm == NULL || strm->state == NULL ||
> >          (strm->next_in == NULL && strm->avail_in != 0))
> 
> Unzipping to NULL is not an attribute of PPC, but rather of being called
> from a bootloader that wants to unpack a kernel to NULL.  Which makes
> this patch wrong on two accounts.  It leaves the bug for CONFIG_PPC and
> it may break bootloaders on other architectures.  A quick grep shows
> xtensa - no clue whether it loads the kernel to NULL or elsewhere.

I was about to say... so thanks! :)

> I'd prefer zlib_inflate to take a flag parameter to disable the check.
> Then we can have two wrappers roughly like this:
> 
> int zlib_inflate(z_streamp strm, int flush)
> {
> 	return __zlib_inflate(strm, flush, 1);
> }
> 
> int zlib_inflate_null_ok_for_bootloaders_only(z_streamp strm, int flush)
> {
> 	return __zlib_inflate(strm, flush, 0);
> }
> 
> Or we could even make the two wrappers inline functions and move them to
> zlib.h.

Sounds like a plan to me, except maybe zlib_inflate_unsafe() and a
comment above the wrapper saying what/why is going on?

-- 
Tom Rini
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Jörn Engel]

On Fri, 16 January 2009 16:07:00 -0700, Tom Rini wrote:
> 
> Sounds like a plan to me, except maybe zlib_inflate_unsafe() and a
> comment above the wrapper saying what/why is going on?

Eric, will you do the honors?  Since you did all the hard work before,
you derserve the fame as well. :)

Jörn

-- 
Joern's library part 11:
http://www.unicom.com/pw/reply-to-harmful.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Eric Sesterhenn]

* Jörn Engel (joern@logfs.org) wrote:
> On Fri, 16 January 2009 16:07:00 -0700, Tom Rini wrote:
> > 
> > Sounds like a plan to me, except maybe zlib_inflate_unsafe() and a
> > comment above the wrapper saying what/why is going on?
> 
> Eric, will you do the honors?  Since you did all the hard work before,
> you derserve the fame as well. :)

Sure, will send a patch during next week

Greetings, Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Eric Sesterhenn]

* Jörn Engel (joern@logfs.org) wrote:
> On Fri, 16 January 2009 16:07:00 -0700, Tom Rini wrote:
> > 
> > Sounds like a plan to me, except maybe zlib_inflate_unsafe() and a
> > comment above the wrapper saying what/why is going on?
> 
> Eric, will you do the honors?  Since you did all the hard work before,
> you derserve the fame as well. :)

Since I am not sure either about xtensa I added chris to the cc list.


Some callees of zlib_inflate() might accidentally pass a NULL
pointer in strm->next_out which zlib_inflate() should catch.
Others like the powerpc gunzip_partial expect to be able
to extract a zImage to memory location 0. This introduces
zlib_inflate_usafe() for those and adds a check for the rest

Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>

--- linux/arch/powerpc/boot/gunzip_util.c.orig	2009-01-20 10:23:11.000000000 +0100
+++ linux/arch/powerpc/boot/gunzip_util.c	2009-01-20 10:23:31.000000000 +0100
@@ -109,7 +109,7 @@ int gunzip_partial(struct gunzip_state *
 
 		state->s.next_out = dst;
 		state->s.avail_out = dstlen;
-		r = zlib_inflate(&state->s, Z_FULL_FLUSH);
+		r = zlib_inflate_unsafe(&state->s, Z_FULL_FLUSH);
 		if (r != Z_OK && r != Z_STREAM_END)
 			fatal("inflate returned %d msg: %s\n\r", r, state->s.msg);
 		len = state->s.next_out - (unsigned char *)dst;
--- linux/include/linux/zlib.h.orig	2009-01-20 10:11:33.000000000 +0100
+++ linux/include/linux/zlib.h	2009-01-20 10:19:56.000000000 +0100
@@ -329,7 +329,23 @@ extern int zlib_inflateInit (z_streamp s
 */
 
 
-extern int zlib_inflate (z_streamp strm, int flush);
+extern int __zlib_inflate (z_streamp strm, int flush, int check_out);
+/*
+    These two wrappers decide wheter strm->next_out gets checked for NULL.
+    The zlib_inflate_unsafe() version got added because the PPC zImage
+    gets extracted to memory address 0 and therefore
+    we avoid this check for zlib_inflate_unsafe()
+*/
+static inline int zlib_inflate(z_streamp strm, int flush)
+{
+	return __zlib_inflate(strm, flush, 1);
+}
+
+static inline int zlib_inflate_unsafe(z_streamp strm, int flush)
+{
+	return __zlib_inflate(strm, flush, 0);
+}
+
 /*
     inflate decompresses as much data as possible, and stops when the input
   buffer becomes empty or the output buffer becomes full. It may introduce
--- linux/lib/zlib_inflate/inflate_syms.c.orig	2009-01-20 10:22:21.000000000 +0100
+++ linux/lib/zlib_inflate/inflate_syms.c	2009-01-20 10:22:32.000000000 +0100
@@ -11,7 +11,7 @@
 #include <linux/zlib.h>
 
 EXPORT_SYMBOL(zlib_inflate_workspacesize);
-EXPORT_SYMBOL(zlib_inflate);
+EXPORT_SYMBOL(__zlib_inflate);
 EXPORT_SYMBOL(zlib_inflateInit2);
 EXPORT_SYMBOL(zlib_inflateEnd);
 EXPORT_SYMBOL(zlib_inflateReset);
--- linux/lib/zlib_inflate/inflate.c.orig	2009-01-20 10:20:14.000000000 +0100
+++ linux/lib/zlib_inflate/inflate.c	2009-01-20 10:22:03.000000000 +0100
@@ -329,7 +329,7 @@ static int zlib_inflateSyncPacket(z_stre
    will return Z_BUF_ERROR if it has not reached the end of the stream.
  */
 
-int zlib_inflate(z_streamp strm, int flush)
+int __zlib_inflate(z_streamp strm, int flush, int check_out)
 {
     struct inflate_state *state;
     const unsigned char *next;  /* next input */
@@ -347,8 +347,10 @@ int zlib_inflate(z_streamp strm, int flu
     static const unsigned short order[19] = /* permutation of code lengths */
         {16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15};
 
-    /* Do not check for strm->next_out == NULL here as ppc zImage
-       inflates to strm->next_out = 0 */
+    /* We only check strm->next_out if the callee requests it,
+       since ppc extracts the ppc zImage to 0 */
+    if (check_out && !strm->next_out)
+        return Z_STREAM_ERROR;
 
     if (strm == NULL || strm->state == NULL ||
         (strm->next_in == NULL && strm->avail_in != 0))
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Eric Sesterhenn]

* J=C3=B6rn Engel (joern@logfs.org) wrote:
> On Fri, 16 January 2009 16:07:00 -0700, Tom Rini wrote:
> >=20
> > Sounds like a plan to me, except maybe zlib_inflate_unsafe() and a
> > comment above the wrapper saying what/why is going on?
>=20
> Eric, will you do the honors?  Since you did all the hard work before,
> you derserve the fame as well. :)

Since I am not sure either about xtensa I added chris to the cc list.


Some callees of zlib_inflate() might accidentally pass a NULL
pointer in strm->next_out which zlib_inflate() should catch.
Others like the powerpc gunzip_partial expect to be able
to extract a zImage to memory location 0. This introduces
zlib_inflate_usafe() for those and adds a check for the rest

Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>

--- linux/arch/powerpc/boot/gunzip_util.c.orig	2009-01-20 10:23:11.00000000=
0 +0100
+++ linux/arch/powerpc/boot/gunzip_util.c	2009-01-20 10:23:31.000000000 +01=
00
@@ -109,7 +109,7 @@ int gunzip_partial(struct gunzip_state *
=20
 		state->s.next_out =3D dst;
 		state->s.avail_out =3D dstlen;
-		r =3D zlib_inflate(&state->s, Z_FULL_FLUSH);
+		r =3D zlib_inflate_unsafe(&state->s, Z_FULL_FLUSH);
 		if (r !=3D Z_OK && r !=3D Z_STREAM_END)
 			fatal("inflate returned %d msg: %s\n\r", r, state->s.msg);
 		len =3D state->s.next_out - (unsigned char *)dst;
--- linux/include/linux/zlib.h.orig	2009-01-20 10:11:33.000000000 +0100
+++ linux/include/linux/zlib.h	2009-01-20 10:19:56.000000000 +0100
@@ -329,7 +329,23 @@ extern int zlib_inflateInit (z_streamp s
 */
=20
=20
-extern int zlib_inflate (z_streamp strm, int flush);
+extern int __zlib_inflate (z_streamp strm, int flush, int check_out);
+/*
+    These two wrappers decide wheter strm->next_out gets checked for NULL.
+    The zlib_inflate_unsafe() version got added because the PPC zImage
+    gets extracted to memory address 0 and therefore
+    we avoid this check for zlib_inflate_unsafe()
+*/
+static inline int zlib_inflate(z_streamp strm, int flush)
+{
+	return __zlib_inflate(strm, flush, 1);
+}
+
+static inline int zlib_inflate_unsafe(z_streamp strm, int flush)
+{
+	return __zlib_inflate(strm, flush, 0);
+}
+
 /*
     inflate decompresses as much data as possible, and stops when the input
   buffer becomes empty or the output buffer becomes full. It may introduce
--- linux/lib/zlib_inflate/inflate_syms.c.orig	2009-01-20 10:22:21.00000000=
0 +0100
+++ linux/lib/zlib_inflate/inflate_syms.c	2009-01-20 10:22:32.000000000 +01=
00
@@ -11,7 +11,7 @@
 #include <linux/zlib.h>
=20
 EXPORT_SYMBOL(zlib_inflate_workspacesize);
-EXPORT_SYMBOL(zlib_inflate);
+EXPORT_SYMBOL(__zlib_inflate);
 EXPORT_SYMBOL(zlib_inflateInit2);
 EXPORT_SYMBOL(zlib_inflateEnd);
 EXPORT_SYMBOL(zlib_inflateReset);
--- linux/lib/zlib_inflate/inflate.c.orig	2009-01-20 10:20:14.000000000 +01=
00
+++ linux/lib/zlib_inflate/inflate.c	2009-01-20 10:22:03.000000000 +0100
@@ -329,7 +329,7 @@ static int zlib_inflateSyncPacket(z_stre
    will return Z_BUF_ERROR if it has not reached the end of the stream.
  */
=20
-int zlib_inflate(z_streamp strm, int flush)
+int __zlib_inflate(z_streamp strm, int flush, int check_out)
 {
     struct inflate_state *state;
     const unsigned char *next;  /* next input */
@@ -347,8 +347,10 @@ int zlib_inflate(z_streamp strm, int flu
     static const unsigned short order[19] =3D /* permutation of code lengt=
hs */
         {16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15};
=20
-    /* Do not check for strm->next_out =3D=3D NULL here as ppc zImage
-       inflates to strm->next_out =3D 0 */
+    /* We only check strm->next_out if the callee requests it,
+       since ppc extracts the ppc zImage to 0 */
+    if (check_out && !strm->next_out)
+        return Z_STREAM_ERROR;
=20
     if (strm =3D=3D NULL || strm->state =3D=3D NULL ||
         (strm->next_in =3D=3D NULL && strm->avail_in !=3D 0))

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Jörn Engel]

On Tue, 20 January 2009 17:47:14 +0100, Eric Sesterhenn wrote:
> 
> Some callees of zlib_inflate() might accidentally pass a NULL

s/callee/caller/ ?

Apart from this, looks fine to me - modulo xtensa of course.

Jörn

-- 
Sometimes, asking the right question is already the answer.
-- Unknown
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Jörn Engel]

On Tue, 20 January 2009 17:47:14 +0100, Eric Sesterhenn wrote:
> 
> Some callees of zlib_inflate() might accidentally pass a NULL

s/callee/caller/ ?

Apart from this, looks fine to me - modulo xtensa of course.

Jörn

-- 
Sometimes, asking the right question is already the answer.
-- Unknown

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Tom Rini]

On Tue, Jan 20, 2009 at 05:47:14PM +0100, Eric Sesterhenn wrote:
> * Jörn Engel (joern@logfs.org) wrote:
> > On Fri, 16 January 2009 16:07:00 -0700, Tom Rini wrote:
> > > 
> > > Sounds like a plan to me, except maybe zlib_inflate_unsafe() and a
> > > comment above the wrapper saying what/why is going on?
> > 
> > Eric, will you do the honors?  Since you did all the hard work before,
> > you derserve the fame as well. :)
> 
> Since I am not sure either about xtensa I added chris to the cc list.

How about we just change all callers from arch/*/boot to use the _unsafe
version?  Then..

> +/*
> +    These two wrappers decide wheter strm->next_out gets checked for NULL.
> +    The zlib_inflate_unsafe() version got added because the PPC zImage
> +    gets extracted to memory address 0 and therefore
> +    we avoid this check for zlib_inflate_unsafe()

These two wrappers decide wheter strm->next_out gets checked for NULL.
The zlib_inflate_unsafe() version is primarily used in the pre-Linux
'boot' directory code to allow for extraction to memory address 0 and
therefore we avoid this check.

-- 
Tom Rini
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Tom Rini]

On Tue, Jan 20, 2009 at 05:47:14PM +0100, Eric Sesterhenn wrote:
> * Jörn Engel (joern@logfs.org) wrote:
> > On Fri, 16 January 2009 16:07:00 -0700, Tom Rini wrote:
> > > 
> > > Sounds like a plan to me, except maybe zlib_inflate_unsafe() and a
> > > comment above the wrapper saying what/why is going on?
> > 
> > Eric, will you do the honors?  Since you did all the hard work before,
> > you derserve the fame as well. :)
> 
> Since I am not sure either about xtensa I added chris to the cc list.

How about we just change all callers from arch/*/boot to use the _unsafe
version?  Then..

> +/*
> +    These two wrappers decide wheter strm->next_out gets checked for NULL.
> +    The zlib_inflate_unsafe() version got added because the PPC zImage
> +    gets extracted to memory address 0 and therefore
> +    we avoid this check for zlib_inflate_unsafe()

These two wrappers decide wheter strm->next_out gets checked for NULL.
The zlib_inflate_unsafe() version is primarily used in the pre-Linux
'boot' directory code to allow for extraction to memory address 0 and
therefore we avoid this check.

-- 
Tom Rini

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Eric Sesterhenn]

* Tom Rini (trini@kernel.crashing.org) wrote:
> On Tue, Jan 20, 2009 at 05:47:14PM +0100, Eric Sesterhenn wrote:
> > * Jörn Engel (joern@logfs.org) wrote:
> > > On Fri, 16 January 2009 16:07:00 -0700, Tom Rini wrote:
> > > > 
> > > > Sounds like a plan to me, except maybe zlib_inflate_unsafe() and a
> > > > comment above the wrapper saying what/why is going on?
> > > 
> > > Eric, will you do the honors?  Since you did all the hard work before,
> > > you derserve the fame as well. :)
> > 
> > Since I am not sure either about xtensa I added chris to the cc list.
> 
> How about we just change all callers from arch/*/boot to use the _unsafe
> version?  Then..
> 
> > +/*
> > +    These two wrappers decide wheter strm->next_out gets checked for NULL.
> > +    The zlib_inflate_unsafe() version got added because the PPC zImage
> > +    gets extracted to memory address 0 and therefore
> > +    we avoid this check for zlib_inflate_unsafe()
> 
> These two wrappers decide wheter strm->next_out gets checked for NULL.
> The zlib_inflate_unsafe() version is primarily used in the pre-Linux
> 'boot' directory code to allow for extraction to memory address 0 and
> therefore we avoid this check.

This integrates Feedback from Tom to change xtensa and a comment, and
the callee/caller replace noticed by joern.




Some callers of zlib_inflate() might accidentally pass a NULL
pointer in strm->next_out which zlib_inflate() should catch.
Others like the powerpc gunzip_partial expect to be able
to extract a zImage to memory location 0. This introduces
zlib_inflate_usafe() for those and adds a check for the rest


Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>

--- linux/arch/powerpc/boot/gunzip_util.c.orig	2009-01-21 09:27:39.000000000 +0100
+++ linux/arch/powerpc/boot/gunzip_util.c	2009-01-21 09:27:51.000000000 +0100
@@ -109,7 +109,7 @@ int gunzip_partial(struct gunzip_state *
 
 		state->s.next_out = dst;
 		state->s.avail_out = dstlen;
-		r = zlib_inflate(&state->s, Z_FULL_FLUSH);
+		r = zlib_inflate_unsafe(&state->s, Z_FULL_FLUSH);
 		if (r != Z_OK && r != Z_STREAM_END)
 			fatal("inflate returned %d msg: %s\n\r", r, state->s.msg);
 		len = state->s.next_out - (unsigned char *)dst;
--- linux/arch/xtensa/boot/lib/zmem.c.orig	2009-01-21 09:22:44.000000000 +0100
+++ linux/arch/xtensa/boot/lib/zmem.c	2009-01-21 09:22:26.000000000 +0100
@@ -68,7 +68,7 @@ void gunzip (void *dst, int dstlen, unsi
         s.avail_in = *lenp - i;
         s.next_out = dst;
         s.avail_out = dstlen;
-        r = zlib_inflate(&s, Z_FINISH);
+        r = zlib_inflate_unsafe(&s, Z_FINISH);
         if (r != Z_OK && r != Z_STREAM_END) {
                 //puts("inflate returned "); puthex(r); puts("\n");
                 exit();
--- linux/include/linux/zlib.h.orig	2009-01-21 09:27:28.000000000 +0100
+++ linux/include/linux/zlib.h	2009-01-21 09:28:55.000000000 +0100
@@ -329,7 +329,23 @@ extern int zlib_inflateInit (z_streamp s
 */
 
 
-extern int zlib_inflate (z_streamp strm, int flush);
+extern int __zlib_inflate (z_streamp strm, int flush, int check_out);
+/*
+    These two wrappers decide wheter strm->next_out gets checked for NULL.
+    The zlib_inflate_unsafe() version is primarily used in the pre-Linux
+    'boot' directory code to allow for extraction to memory address 0 and
+    therefore we avoid this check.
+*/
+static inline int zlib_inflate(z_streamp strm, int flush)
+{
+	return __zlib_inflate(strm, flush, 1);
+}
+
+static inline int zlib_inflate_unsafe(z_streamp strm, int flush)
+{
+	return __zlib_inflate(strm, flush, 0);
+}
+
 /*
     inflate decompresses as much data as possible, and stops when the input
   buffer becomes empty or the output buffer becomes full. It may introduce
--- linux/lib/zlib_inflate/inflate.c.orig	2009-01-21 09:27:11.000000000 +0100
+++ linux/lib/zlib_inflate/inflate.c	2009-01-21 09:29:10.000000000 +0100
@@ -329,7 +329,7 @@ static int zlib_inflateSyncPacket(z_stre
    will return Z_BUF_ERROR if it has not reached the end of the stream.
  */
 
-int zlib_inflate(z_streamp strm, int flush)
+int __zlib_inflate(z_streamp strm, int flush, int check_out)
 {
     struct inflate_state *state;
     const unsigned char *next;  /* next input */
@@ -347,8 +347,10 @@ int zlib_inflate(z_streamp strm, int flu
     static const unsigned short order[19] = /* permutation of code lengths */
         {16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15};
 
-    /* Do not check for strm->next_out == NULL here as ppc zImage
-       inflates to strm->next_out = 0 */
+    /* We only check strm->next_out if the caller requests it,
+       since ppc extracts the ppc zImage to 0 */
+    if (check_out && !strm->next_out)
+        return Z_STREAM_ERROR;
 
     if (strm == NULL || strm->state == NULL ||
         (strm->next_in == NULL && strm->avail_in != 0))
--- linux/lib/zlib_inflate/inflate_syms.c.orig	2009-01-21 09:27:16.000000000 +0100
+++ linux/lib/zlib_inflate/inflate_syms.c	2009-01-21 09:27:51.000000000 +0100
@@ -11,7 +11,7 @@
 #include <linux/zlib.h>
 
 EXPORT_SYMBOL(zlib_inflate_workspacesize);
-EXPORT_SYMBOL(zlib_inflate);
+EXPORT_SYMBOL(__zlib_inflate);
 EXPORT_SYMBOL(zlib_inflateInit2);
 EXPORT_SYMBOL(zlib_inflateEnd);
 EXPORT_SYMBOL(zlib_inflateReset);
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Eric Sesterhenn]

* Tom Rini (trini@kernel.crashing.org) wrote:
> On Tue, Jan 20, 2009 at 05:47:14PM +0100, Eric Sesterhenn wrote:
> > * J=C3=B6rn Engel (joern@logfs.org) wrote:
> > > On Fri, 16 January 2009 16:07:00 -0700, Tom Rini wrote:
> > > >=20
> > > > Sounds like a plan to me, except maybe zlib_inflate_unsafe() and a
> > > > comment above the wrapper saying what/why is going on?
> > >=20
> > > Eric, will you do the honors?  Since you did all the hard work before,
> > > you derserve the fame as well. :)
> >=20
> > Since I am not sure either about xtensa I added chris to the cc list.
>=20
> How about we just change all callers from arch/*/boot to use the _unsafe
> version?  Then..
>=20
> > +/*
> > +    These two wrappers decide wheter strm->next_out gets checked for N=
ULL.
> > +    The zlib_inflate_unsafe() version got added because the PPC zImage
> > +    gets extracted to memory address 0 and therefore
> > +    we avoid this check for zlib_inflate_unsafe()
>=20
> These two wrappers decide wheter strm->next_out gets checked for NULL.
> The zlib_inflate_unsafe() version is primarily used in the pre-Linux
> 'boot' directory code to allow for extraction to memory address 0 and
> therefore we avoid this check.

This integrates Feedback from Tom to change xtensa and a comment, and
the callee/caller replace noticed by joern.




Some callers of zlib_inflate() might accidentally pass a NULL
pointer in strm->next_out which zlib_inflate() should catch.
Others like the powerpc gunzip_partial expect to be able
to extract a zImage to memory location 0. This introduces
zlib_inflate_usafe() for those and adds a check for the rest


Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>

--- linux/arch/powerpc/boot/gunzip_util.c.orig	2009-01-21 09:27:39.00000000=
0 +0100
+++ linux/arch/powerpc/boot/gunzip_util.c	2009-01-21 09:27:51.000000000 +01=
00
@@ -109,7 +109,7 @@ int gunzip_partial(struct gunzip_state *
=20
 		state->s.next_out =3D dst;
 		state->s.avail_out =3D dstlen;
-		r =3D zlib_inflate(&state->s, Z_FULL_FLUSH);
+		r =3D zlib_inflate_unsafe(&state->s, Z_FULL_FLUSH);
 		if (r !=3D Z_OK && r !=3D Z_STREAM_END)
 			fatal("inflate returned %d msg: %s\n\r", r, state->s.msg);
 		len =3D state->s.next_out - (unsigned char *)dst;
--- linux/arch/xtensa/boot/lib/zmem.c.orig	2009-01-21 09:22:44.000000000 +0=
100
+++ linux/arch/xtensa/boot/lib/zmem.c	2009-01-21 09:22:26.000000000 +0100
@@ -68,7 +68,7 @@ void gunzip (void *dst, int dstlen, unsi
         s.avail_in =3D *lenp - i;
         s.next_out =3D dst;
         s.avail_out =3D dstlen;
-        r =3D zlib_inflate(&s, Z_FINISH);
+        r =3D zlib_inflate_unsafe(&s, Z_FINISH);
         if (r !=3D Z_OK && r !=3D Z_STREAM_END) {
                 //puts("inflate returned "); puthex(r); puts("\n");
                 exit();
--- linux/include/linux/zlib.h.orig	2009-01-21 09:27:28.000000000 +0100
+++ linux/include/linux/zlib.h	2009-01-21 09:28:55.000000000 +0100
@@ -329,7 +329,23 @@ extern int zlib_inflateInit (z_streamp s
 */
=20
=20
-extern int zlib_inflate (z_streamp strm, int flush);
+extern int __zlib_inflate (z_streamp strm, int flush, int check_out);
+/*
+    These two wrappers decide wheter strm->next_out gets checked for NULL.
+    The zlib_inflate_unsafe() version is primarily used in the pre-Linux
+    'boot' directory code to allow for extraction to memory address 0 and
+    therefore we avoid this check.
+*/
+static inline int zlib_inflate(z_streamp strm, int flush)
+{
+	return __zlib_inflate(strm, flush, 1);
+}
+
+static inline int zlib_inflate_unsafe(z_streamp strm, int flush)
+{
+	return __zlib_inflate(strm, flush, 0);
+}
+
 /*
     inflate decompresses as much data as possible, and stops when the input
   buffer becomes empty or the output buffer becomes full. It may introduce
--- linux/lib/zlib_inflate/inflate.c.orig	2009-01-21 09:27:11.000000000 +01=
00
+++ linux/lib/zlib_inflate/inflate.c	2009-01-21 09:29:10.000000000 +0100
@@ -329,7 +329,7 @@ static int zlib_inflateSyncPacket(z_stre
    will return Z_BUF_ERROR if it has not reached the end of the stream.
  */
=20
-int zlib_inflate(z_streamp strm, int flush)
+int __zlib_inflate(z_streamp strm, int flush, int check_out)
 {
     struct inflate_state *state;
     const unsigned char *next;  /* next input */
@@ -347,8 +347,10 @@ int zlib_inflate(z_streamp strm, int flu
     static const unsigned short order[19] =3D /* permutation of code lengt=
hs */
         {16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15};
=20
-    /* Do not check for strm->next_out =3D=3D NULL here as ppc zImage
-       inflates to strm->next_out =3D 0 */
+    /* We only check strm->next_out if the caller requests it,
+       since ppc extracts the ppc zImage to 0 */
+    if (check_out && !strm->next_out)
+        return Z_STREAM_ERROR;
=20
     if (strm =3D=3D NULL || strm->state =3D=3D NULL ||
         (strm->next_in =3D=3D NULL && strm->avail_in !=3D 0))
--- linux/lib/zlib_inflate/inflate_syms.c.orig	2009-01-21 09:27:16.00000000=
0 +0100
+++ linux/lib/zlib_inflate/inflate_syms.c	2009-01-21 09:27:51.000000000 +01=
00
@@ -11,7 +11,7 @@
 #include <linux/zlib.h>
=20
 EXPORT_SYMBOL(zlib_inflate_workspacesize);
-EXPORT_SYMBOL(zlib_inflate);
+EXPORT_SYMBOL(__zlib_inflate);
 EXPORT_SYMBOL(zlib_inflateInit2);
 EXPORT_SYMBOL(zlib_inflateEnd);
 EXPORT_SYMBOL(zlib_inflateReset);

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Phillip Lougher]

Eric Sesterhenn wrote:

> Some callers of zlib_inflate() might accidentally pass a NULL
> pointer in strm->next_out which zlib_inflate() should catch.
> Others like the powerpc gunzip_partial expect to be able
> to extract a zImage to memory location 0. This introduces
> zlib_inflate_usafe() for those and adds a check for the rest
> 
> 
> +    These two wrappers decide wheter strm->next_out gets checked for NULL.

"wheter" -> "whether"

Apart from that looks OK to me.

Phillip

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Phillip Lougher]

Eric Sesterhenn wrote:

> Some callers of zlib_inflate() might accidentally pass a NULL
> pointer in strm->next_out which zlib_inflate() should catch.
> Others like the powerpc gunzip_partial expect to be able
> to extract a zImage to memory location 0. This introduces
> zlib_inflate_usafe() for those and adds a check for the rest
> 
> 
> +    These two wrappers decide wheter strm->next_out gets checked for NULL.

"wheter" -> "whether"

Apart from that looks OK to me.

Phillip

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Phillip Lougher]

Eric Sesterhenn wrote:

>> I already reported a similar issue to Phillip but got no reply (lost in spam?),
>> so I duplicate it here:

Thanks for testing Squashfs.  I've not ignored your emails, but I've
been busy job hunting, and so have not had time to look into this
until now.

I've replied to the btrfs corrupted filesystem thread, but for people not following that I've copied it here.

I hardened Squashfs against fsfuzzer back in November 2006 (remember
the month of kernel bugs, or MOKB, which highlighted a number of
issues with Squashfs).  Your testing has thrown up a regression that I
inadvertently  put in last month!

As part of the mainlining effort I changed Squashfs to allocate
buffers in 4K page sizes rather than use vmalloced large buffers.   As
far as zlib goes, it means zlib_inflate now decompresses into a
sequence of 4K buffers rather than one large buffer.   What this means
is zlib_inflate is called repeatedly moving to the next 4K page
whenever zlib_inflate asks for another buffer (stream.avail_out == 0).

Your testing have thrown up the case where zlib_inflate is asking for
too many output buffers, i.e. it has returned with Z_OK,
stream.avail_in !=0 (more input data to be processed), and
stream.avail_out == 0 (I'd like another output buffer).  but it has
consumed all the output buffers.  This isn't checked (the code assumes
zlib will do the right thing on corrupt data and bomb out).  My guess
is either zlib_inflate is getting confused with corrupt data, or
fsfuzzer gets lucky sometimes and corrupts the filesystem to point to
another valid but larger compressed block (i.e. in your test
filesystems the 4K datablock is being corrupted to point to an 8K
metadata block).

This ultimately leads to an oops in zlib_inflate where it has been
passed a bogus or NULL steam.next_out pointer.

I'll create a patch and send it to you if you're happy to test it.

Phillip

Re: [Patch] NULL pointer deref with corrupted squashfs image

[Jörn Engel]

On Thu, 22 January 2009 02:48:43 +0000, Phillip Lougher wrote:
> 
> My guess
> is either zlib_inflate is getting confused with corrupt data

Which is easy enough.  As one would expect of a decent compressor, there
is little redundancy in the zlib stream that can be used for error
checking.  The 2-byte header has some, literal blocks have the length
field twice and compressed blocks contain a couple of illegal symbols.

The best way to protect oneself against accidental errors is checksums.
And the zlib decision to checksum the _un_compressed data clearly
doesn't help in this case, as the experienced problem occurs before the
check.  Also explains the "small .gz expands to gigabytes of data"
attack, btw.

Given a malicious attacker with enough time and resources, checksums
obviously don't help.  They will simply match the corrupt data.

Jörn

-- 
Joern's library part 3:
http://inst.eecs.berkeley.edu/~cs152/fa05/handouts/clark-test.pdf
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html