linux-kernel - Re: Kernel 3.1.0-rc4 oops when connecting iPod

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAG1a4rs+-K8GukJE-C8a7JCbw6usdY1FLxjXM7AWYEUpADXusQ@mail.gmail.com>
Date:	Sat, 3 Sep 2011 16:52:40 -0400
From:	Pavel Ivanov <paivanof@...il.com>
To:	Hin-Tak Leung <hintak_leung@...oo.co.uk>
Cc:	linux-fsdevel@...r.kernel.org,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: Kernel 3.1.0-rc4 oops when connecting iPod

Hin-Tak,

"if(sbi->s_backup_vhdr)" as you suggested didn't help. But I made
another change. I made fs/hfsplus/super.c to look like this near the
line 529:

out_free_vhdr:
	printk(KERN_ERR "hfs: sbi->s_vhdr = %p, sbi->s_backup_vhdr = %p\n",
sbi->s_vhdr, sbi->s_backup_vhdr);
	kfree(sbi->s_vhdr);
	kfree(sbi->s_backup_vhdr);

And here's what I see after connecting an iPod:

[   92.549197] hfs: filesystem size too large blksz_shift=14,
total_blocks=486494
[   92.635714] hfs: sbi->s_vhdr = ffff88013703a690, sbi->s_backup_vhdr
= ffff88013703e268
[   92.730543] =============================================================================
[   92.828425] BUG kmalloc-4096: Invalid object pointer 0xffff88013703a690
...
[   93.213890] =============================================================================
[   93.311834] BUG kmalloc-4096: Invalid object pointer 0xffff88013703e268
...
[   93.868618] hfs: filesystem size too large blksz_shift=14,
total_blocks=486494
[   93.955327] hfs: sbi->s_vhdr = ffff8801343c37d8, sbi->s_backup_vhdr
= ffff8801343c5120
[   94.050133] =============================================================================
[   94.148026] BUG kmalloc-4096: Invalid object pointer 0xffff8801343c37d8
...
[   94.533895] =============================================================================
[   94.631746] BUG kmalloc-4096: Invalid object pointer 0xffff8801343c5120


So these 2 pointers are exactly what causing the trouble.


Pavel


On Sat, Sep 3, 2011 at 2:57 AM, Hin-Tak Leung <hintak_leung@...oo.co.uk> wrote:
> --- On Sat, 3/9/11, Pavel Ivanov <paivanof@...il.com> wrote:
>
>> On Fri, Sep 2, 2011 at 11:59 PM,
>> Hin-Tak Leung <hintak_leung@...oo.co.uk>
>> wrote:
>> >> With kernel 3.1.0-rc4 any attempt to connect iPod
>> to USB
>> >> leads to
>> >> kernel oops. I'd say that stacktrace of the oops
>> is pretty
>> >> much random
>> >> and not related to HFS. But I was able to get
>> useful info
>> >> from it when
>> >> I recompiled with CONFIG_SLUB_DEBUG_ON=y. In this
>> case I
>> >> don't get
>> >> oops but the following instead:
>> >
>> > There are a few hfsplus related changes to do
>> protection against invalid data like this, but may be there
>> are more. It would be useful to have the output from your
>> > objdump -l -d hfsplus.ko | grep  -A 1000
>> '<hfsplus_fill_super>'
>> > (the -l gives line numbers against the kernel tree, so
>> would be useful if you run this against the ko there...)
>>
>> Output of this command is in attachment.
>
> That's interesting. You said "hfs: filesystem size too large." always appears twice (with kernel 3.1-rc4) before it oops. And in your 2.6.38.11 kernel, you had "hfs: unable to find HFS+ superblock" twice.
>
> The oops place is the "kfree(sbi->s_backup_vhdr)" in line 529 in fs/hfsplus/super.c:
>
> 527: out_free_vhdr:
> 528:        kfree(sbi->s_vhdr);
> 529:        kfree(sbi->s_backup_vhdr);
>
> It would appear the s_backup_vhdr is somehow garbage but that was not caught in the 3.1-rc4 version of hfsplus_read_wrapper() ; it was caught by the 2.6.38.11 version of hfsplus_read_wrapper(). hfsplus_read_wrapper() was changed in the 2.6.39/3.0 time frame by this:
>
>    commit 52399b171dfaea02b6944cd6feba49b624147126
>    Author: Christoph Hellwig <hch@...era.com>
>    Date:   Tue Nov 23 14:37:47 2010 +0100
>
>        hfsplus: use raw bio access for the volume headers
>
> That's code I don't quite understand (I worked on the hfsplus journal code recently, supposedly mentoring for that GSoC project).
>
> If you are happy enough to do a bit of experimenting, can you try putting a
> "if(sbi->s_backup_vhdr)" before line 529?
>
> Also it is curious why it wasn't caught in wrapper.c arond 229 to 236 ending with:
> "if (sbi->s_backup_vhdr->signature != sbi->s_vhdr->signature)"
>
>
> The file system too large comes from line 402 in super.c:
> -----------------------
>        err = generic_check_addressable(sbi->alloc_blksz_shift,
>                                        sbi->total_blocks);
>        if (err) {
>                printk(KERN_ERR "hfs: filesystem size too large.\n");
>                goto out_free_vhdr;
>
> -----------------------
> So it might be interesting to see what is too large... try changing that to:
>
> printk(KERN_ERR "hfs: filesystem size too large blksz_shift=%d, total_blocks=%d\n", sbi->alloc_blksz_shift, sbi->total_blocks);
>
> ?
>
> It is a 42GB image - if it were smaller I would suggest dd'ing that and upload it somewhere to check...
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/