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: <CAG1a4rt5wLihR-ZkUD78WRbHk39xWP0ypJzMe4K5G1O-k12-MQ@mail.gmail.com>
Date:	Sat, 10 Sep 2011 23:52:55 -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>,
	Christoph Hellwig <hch@...era.com>
Subject: Re: Kernel 3.1.0-rc4 oops when connecting iPod

On Tue, Sep 6, 2011 at 11:09 AM, Pavel Ivanov <paivanof@...il.com> wrote:
>>> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
>>> index c106ca2..5458be4 100644
>>> --- a/fs/hfsplus/super.c
>>> +++ b/fs/hfsplus/super.c
>>> @@ -345,6 +345,8 @@ static int hfsplus_fill_super(struct
>>> super_block
>>> *sb, void *data, int silent)
>>>      struct qstr str;
>>>      struct nls_table *nls = NULL;
>>>      int err;
>>> +    unsigned check_blksz_bits;
>>> +    u64 check_num_blocks;
>>>
>>>      err = -EINVAL;
>>>      sbi = kzalloc(sizeof(*sbi),
>>> GFP_KERNEL);
>>> @@ -399,10 +401,15 @@ static int hfsplus_fill_super(struct
>>> super_block
>>> *sb, void *data, int silent)
>>>      if (!sbi->rsrc_clump_blocks)
>>>
>>> sbi->rsrc_clump_blocks = 1;
>>>
>>> -    err =
>>> generic_check_addressable(sbi->alloc_blksz_shift,
>>> -
>>>
>>> sbi->total_blocks);
>>> +    check_blksz_bits =
>>> sbi->alloc_blksz_shift;
>>> +    check_num_blocks =
>>> sbi->total_blocks;
>>> +    if (sbi->fs_shift != 0) {
>>> +        check_num_blocks
>>> <<= sbi->fs_shift;
>>> +        check_blksz_bits -=
>>> sbi->fs_shift;
>>> +    }
>>> +    err =
>>> generic_check_addressable(check_blksz_bits,
>>> check_num_blocks);
>>>      if (err) {
>>>         printk(KERN_ERR "hfs:
>>> filesystem size too large.\n");
>>>          goto out_free_vhdr;
>>
>> This is a bit ugly - what it does is massage the two values taking into account of sbi->fs_shift to pass check_addressable() . I think either check_addressable should be extended, or this be abstracted out say, to check_hfs_addressable() (with __inline__ if desired) to be cleaner.
>
> Agreed. It was more of a quick hack to prove the correctness of such
> change direction. I'll think how to do it better.

Hin-Tak,

How about the following patch?


Subject: [PATCH] hfsplus: Allow to mount filesystem with block size
greater than PAGE_SIZE

Commit c6d5f5fa (hfsplus: lift the 2TB size limit) added call to
generic_check_addressable() but used the system's internal block size
which can be larger than PAGE_SIZE and will cause
generic_check_addressable() to return -EINVAL in this case. This
results in impossibility to mount file systems with such block sizes.
To fix it we have to use block size understandable by
generic_check_addressable() and adjust number of blocks accordingly.

Signed-off-by: Pavel Ivanov <paivanof@...il.com>
Cc: <stable@...nel.org>
---

diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index c106ca2..26ac5a5 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -399,8 +399,17 @@ static int hfsplus_fill_super(struct super_block
*sb, void *data, int silent)
 	if (!sbi->rsrc_clump_blocks)
 		sbi->rsrc_clump_blocks = 1;

-	err = generic_check_addressable(sbi->alloc_blksz_shift,
-					sbi->total_blocks);
+	/*
+	 * Check that filesystem data can be addressed by sector_t. Obvious way
+	 * to do that is to call generic_check_addressable with alloc_blksz_shift
+	 * and total_blocks as parameters. But alloc_blksz_shift can be greater
+	 * than PAGE_SIZE_SHIFT and in this case it won't be accepted by
+	 * generic_check_addressable. s_blocksize_bits will always be accepted
+	 * and difference between it and alloc_blksz_shift will be always in
+	 * fs_shift.
+	 */
+	err = generic_check_addressable(sb->s_blocksize_bits,
+			(u64)sbi->total_blocks << sbi->fs_shift);
 	if (err) {
 		printk(KERN_ERR "hfs: filesystem size too large.\n");
 		goto out_free_vhdr;
--
--
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/