From: Vegard Nossum <vegard.nossum@oracle.com>
To: ltp@lists.linux.it
Subject: [LTP] [lkp] [ext4] 5405511e1a: ltp.acl_test01.fail]
Date: Mon, 11 Jul 2016 13:26:47 +0200 [thread overview]
Message-ID: <57838277.3050700@oracle.com> (raw)
In-Reply-To: <20160711031553.GP26097@thunk.org>
On 07/11/2016 05:15 AM, Theodore Ts'o wrote:
> On Mon, Jul 11, 2016 at 09:59:54AM +0800, kernel test robot wrote:
>>
>> FYI, we noticed the following commit:
>>
>> https://github.com/0day-ci/linux Vegard-Nossum/ext4-validate-number-of-clusters-in-group/20160708-041426
>> commit 5405511e1a984ab644fa9e29a0d3d958b835ab75 ("ext4: validate number of meta clusters in group")
[...]
>
> Vegard, I'm guessing you didn't have a chance to test your patch
> before you sent it to the list?
I test all my patches against the failing test-case and a few other images.
This patch specifically I think was sent with an [RFC] tag which I
intended to signal that I'm *not* sure of the fix.
That said, I could do a better job of running more conventional fs tests
on my patches, so I'll incorporate xfstests into my workflow.
> bit_max = ext4_num_clusters_in_group(sb, i);
> if ((bit_max >> 3) >= sb->s_blocksize) {
> ext4_msg(sb, KERN_WARNING, "clusters in "
> "group %u exceeds block size", i);
> goto failed_mount;
> }
>
>
> This is the test which is failing, but it will fail by default on
> pretty much all ext4 file systems, since by default there will be
> 32768 blocks (clusters) per group, with a 4k block size (and 32768 >>
> 3 == 4096). And in the test that failed, this was a 1k block size
> with 8192 blocks per blocks (and 8192 >> 3 == 1024).
Ugh, brain-o on my part. It should say > rather than >=, agreed?
> Anyway, as I mentioned before, I'd much rather do very specific sanity
> checking on superblock fields, instead of sanity checking calculated
> values such as ext4_num_clusters_in_group().
>
> Perhaps the easist thing to do is to run e2fsck -n on those file
> systems that are causing problems?
The function (ext4_init_block_bitmap()) has even more problems than the
ones I reported to the list so far; ext4_block_bitmap(),
ext4_inode_bitmap(), and ext4_inode_table() may _also_ point outside the
buffer and cause random corruptions.
I'll try to come up with a new (and better tested) patch.
Thanks,
Vegard
WARNING: multiple messages have this Message-ID (diff)
From: Vegard Nossum <vegard.nossum@oracle.com>
To: lkp@lists.01.org
Subject: Re: [ext4] 5405511e1a: ltp.acl_test01.fail]
Date: Mon, 11 Jul 2016 13:26:47 +0200 [thread overview]
Message-ID: <57838277.3050700@oracle.com> (raw)
In-Reply-To: <20160711031553.GP26097@thunk.org>
[-- Attachment #1: Type: text/plain, Size: 2101 bytes --]
On 07/11/2016 05:15 AM, Theodore Ts'o wrote:
> On Mon, Jul 11, 2016 at 09:59:54AM +0800, kernel test robot wrote:
>>
>> FYI, we noticed the following commit:
>>
>> https://github.com/0day-ci/linux Vegard-Nossum/ext4-validate-number-of-clusters-in-group/20160708-041426
>> commit 5405511e1a984ab644fa9e29a0d3d958b835ab75 ("ext4: validate number of meta clusters in group")
[...]
>
> Vegard, I'm guessing you didn't have a chance to test your patch
> before you sent it to the list?
I test all my patches against the failing test-case and a few other images.
This patch specifically I think was sent with an [RFC] tag which I
intended to signal that I'm *not* sure of the fix.
That said, I could do a better job of running more conventional fs tests
on my patches, so I'll incorporate xfstests into my workflow.
> bit_max = ext4_num_clusters_in_group(sb, i);
> if ((bit_max >> 3) >= sb->s_blocksize) {
> ext4_msg(sb, KERN_WARNING, "clusters in "
> "group %u exceeds block size", i);
> goto failed_mount;
> }
>
>
> This is the test which is failing, but it will fail by default on
> pretty much all ext4 file systems, since by default there will be
> 32768 blocks (clusters) per group, with a 4k block size (and 32768 >>
> 3 == 4096). And in the test that failed, this was a 1k block size
> with 8192 blocks per blocks (and 8192 >> 3 == 1024).
Ugh, brain-o on my part. It should say > rather than >=, agreed?
> Anyway, as I mentioned before, I'd much rather do very specific sanity
> checking on superblock fields, instead of sanity checking calculated
> values such as ext4_num_clusters_in_group().
>
> Perhaps the easist thing to do is to run e2fsck -n on those file
> systems that are causing problems?
The function (ext4_init_block_bitmap()) has even more problems than the
ones I reported to the list so far; ext4_block_bitmap(),
ext4_inode_bitmap(), and ext4_inode_table() may _also_ point outside the
buffer and cause random corruptions.
I'll try to come up with a new (and better tested) patch.
Thanks,
Vegard
WARNING: multiple messages have this Message-ID (diff)
From: Vegard Nossum <vegard.nossum@oracle.com>
To: "Theodore Ts'o" <tytso@mit.edu>,
kernel test robot <xiaolong.ye@intel.com>,
0day robot <fengguang.wu@intel.com>,
"Darrick J. Wong" <darrick.wong@oracle.com>,
LKML <linux-kernel@vger.kernel.org>,
lkp@01.org, ltp@lists.linux.it
Subject: Re: [lkp] [ext4] 5405511e1a: ltp.acl_test01.fail]
Date: Mon, 11 Jul 2016 13:26:47 +0200 [thread overview]
Message-ID: <57838277.3050700@oracle.com> (raw)
In-Reply-To: <20160711031553.GP26097@thunk.org>
On 07/11/2016 05:15 AM, Theodore Ts'o wrote:
> On Mon, Jul 11, 2016 at 09:59:54AM +0800, kernel test robot wrote:
>>
>> FYI, we noticed the following commit:
>>
>> https://github.com/0day-ci/linux Vegard-Nossum/ext4-validate-number-of-clusters-in-group/20160708-041426
>> commit 5405511e1a984ab644fa9e29a0d3d958b835ab75 ("ext4: validate number of meta clusters in group")
[...]
>
> Vegard, I'm guessing you didn't have a chance to test your patch
> before you sent it to the list?
I test all my patches against the failing test-case and a few other images.
This patch specifically I think was sent with an [RFC] tag which I
intended to signal that I'm *not* sure of the fix.
That said, I could do a better job of running more conventional fs tests
on my patches, so I'll incorporate xfstests into my workflow.
> bit_max = ext4_num_clusters_in_group(sb, i);
> if ((bit_max >> 3) >= sb->s_blocksize) {
> ext4_msg(sb, KERN_WARNING, "clusters in "
> "group %u exceeds block size", i);
> goto failed_mount;
> }
>
>
> This is the test which is failing, but it will fail by default on
> pretty much all ext4 file systems, since by default there will be
> 32768 blocks (clusters) per group, with a 4k block size (and 32768 >>
> 3 == 4096). And in the test that failed, this was a 1k block size
> with 8192 blocks per blocks (and 8192 >> 3 == 1024).
Ugh, brain-o on my part. It should say > rather than >=, agreed?
> Anyway, as I mentioned before, I'd much rather do very specific sanity
> checking on superblock fields, instead of sanity checking calculated
> values such as ext4_num_clusters_in_group().
>
> Perhaps the easist thing to do is to run e2fsck -n on those file
> systems that are causing problems?
The function (ext4_init_block_bitmap()) has even more problems than the
ones I reported to the list so far; ext4_block_bitmap(),
ext4_inode_bitmap(), and ext4_inode_table() may _also_ point outside the
buffer and cause random corruptions.
I'll try to come up with a new (and better tested) patch.
Thanks,
Vegard
next prev parent reply other threads:[~2016-07-11 11:26 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-11 1:59 [LTP] [lkp] [ext4] 5405511e1a: ltp.acl_test01.fail] kernel test robot
2016-07-11 1:59 ` kernel test robot
2016-07-11 1:59 ` kernel test robot
2016-07-11 3:15 ` [LTP] [lkp] " Theodore Ts'o
2016-07-11 3:15 ` Theodore Ts'o
2016-07-11 3:15 ` Theodore Ts'o
2016-07-11 5:08 ` [LTP] [lkp] " Ye Xiaolong
2016-07-11 5:08 ` Ye Xiaolong
2016-07-11 5:08 ` Ye Xiaolong
2016-07-11 11:26 ` Vegard Nossum [this message]
2016-07-11 11:26 ` [lkp] " Vegard Nossum
2016-07-11 11:26 ` Vegard Nossum
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57838277.3050700@oracle.com \
--to=vegard.nossum@oracle.com \
--cc=ltp@lists.linux.it \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.