2011-09-30 19:48:53

by djwong

[permalink] [raw]
Subject: [PATCH] ext4: Fix comparison endianness problem in MMP initialization

As part of startup, the MMP initialization code does this:

mmp->mmp_seq = seq = cpu_to_le32(mmp_new_seq());

Next, mmp->mmp_seq is written out to disk, a delay happens, and then the MMP
block is read back in and the sequence value is tested:

if (seq != le32_to_cpu(mmp->mmp_seq)) {
/* fail the mount */

On a LE system such as x86, the *le32* functions do nothing and this works.
Unfortunately, on a BE system such as ppc64, this comparison becomes:

if (cpu_to_le32(new_seq) != le32_to_cpu(cpu_to_le32(new_seq)) {
/* fail the mount */

Except for a few palindromic sequence numbers, this test always causes the
mount to fail, which makes MMP filesystems generally unmountable on ppc64. The
attached patch fixes this situation.

(This fix came up while testing the metadata checksumming patchset)

Signed-off-by: Darrick J. Wong <[email protected]>
---

fs/ext4/mmp.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
index 9bdef3f..a7a4986 100644
--- a/fs/ext4/mmp.c
+++ b/fs/ext4/mmp.c
@@ -295,7 +295,8 @@ skip:
/*
* write a new random sequence number.
*/
- mmp->mmp_seq = seq = cpu_to_le32(mmp_new_seq());
+ seq = mmp_new_seq();
+ mmp->mmp_seq = cpu_to_le32(seq);

retval = write_mmp_block(bh);
if (retval)


2011-09-30 21:12:04

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH] ext4: Fix comparison endianness problem in MMP initialization

On 2011-09-30, at 1:49 PM, Darrick J. Wong wrote:
> As part of startup, the MMP initialization code does this:
>
> mmp->mmp_seq = seq = cpu_to_le32(mmp_new_seq());
>
> Next, mmp->mmp_seq is written out to disk, a delay happens, and then
> the MMP block is read back in and the sequence value is tested:
>
> if (seq != le32_to_cpu(mmp->mmp_seq)) {
> /* fail the mount */
>
> On a LE system such as x86, the *le32* functions do nothing and this
> works.
> Unfortunately, on a BE system such as ppc64, this comparison becomes:
>
> if (cpu_to_le32(new_seq) != le32_to_cpu(cpu_to_le32(new_seq)) {
> /* fail the mount */
>
> Except for a few palindromic sequence numbers, this test always causes
> the mount to fail, which makes MMP filesystems generally unmountable
> on ppc64. The attached patch fixes this situation.
>
> (This fix came up while testing the metadata checksumming patchset)

Thanks also for the catch, you are completely right. We've never run
this code on a big-endian system in all the years we had it.

> Signed-off-by: Darrick J. Wong <[email protected]>
> ---
>
> fs/ext4/mmp.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index 9bdef3f..a7a4986 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -295,7 +295,8 @@ skip:
> /*
> * write a new random sequence number.
> */
> - mmp->mmp_seq = seq = cpu_to_le32(mmp_new_seq());
> + seq = mmp_new_seq();
> + mmp->mmp_seq = cpu_to_le32(seq);
>
> retval = write_mmp_block(bh);
> if (retval)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Cheers, Andreas






2011-10-08 21:27:45

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: Fix comparison endianness problem in MMP initialization

On Fri, Sep 30, 2011 at 12:49:00PM -0700, Darrick J. Wong wrote:
> As part of startup, the MMP initialization code does this:
>
> mmp->mmp_seq = seq = cpu_to_le32(mmp_new_seq());
>
> Next, mmp->mmp_seq is written out to disk, a delay happens, and then the MMP
> block is read back in and the sequence value is tested:
>
> if (seq != le32_to_cpu(mmp->mmp_seq)) {
> /* fail the mount */
>
> On a LE system such as x86, the *le32* functions do nothing and this works.
> Unfortunately, on a BE system such as ppc64, this comparison becomes:
>
> if (cpu_to_le32(new_seq) != le32_to_cpu(cpu_to_le32(new_seq)) {
> /* fail the mount */
>
> Except for a few palindromic sequence numbers, this test always causes the
> mount to fail, which makes MMP filesystems generally unmountable on ppc64. The
> attached patch fixes this situation.
>
> (This fix came up while testing the metadata checksumming patchset)
>
> Signed-off-by: Darrick J. Wong <[email protected]>

Applied, thanks.

- Ted