Re: Kernel 3.1.0-rc4 oops when connecting iPod

From: Pavel Ivanov
Date: Sat Sep 03 2011 - 17:05:16 EST


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@xxxxxxxxxxx> wrote:
> --- On Sat, 3/9/11, Pavel Ivanov <paivanof@xxxxxxxxx> wrote:
>
>> On Fri, Sep 2, 2011 at 11:59 PM,
>> Hin-Tak Leung <hintak_leung@xxxxxxxxxxx>
>> 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@xxxxxxxxxx>
>    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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/