* [PATCH 2/5] ext4: ext4_rename should dirty dir_bh with the correct directory
2011-10-21 21:17 [PATCH 1/5] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode Darrick J. Wong
@ 2011-10-21 21:18 ` Darrick J. Wong
2011-10-25 13:08 ` Ted Ts'o
2011-10-21 21:18 ` [PATCH 3/5] ext4: ext4_mkdir should dirty dir_block with the parent inode Darrick J. Wong
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2011-10-21 21:18 UTC (permalink / raw)
To: Tao Ma, Theodore Tso; +Cc: linux-ext4, linux-kernel
When ext4_rename performs a directory rename (move), dir_bh is a buffer that is
modified to update the '..' link in the directory being moved (old_inode).
However, ext4_handle_dirty_metadata is called with the old parent directory
inode (old_dir) and dir_bh, which is incorrect because dir_bh does not belong
to the parent inode. Fix this error.
Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---
fs/ext4/namei.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 310b356..6d3fab4 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2530,7 +2530,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
PARENT_INO(dir_bh->b_data, new_dir->i_sb->s_blocksize) =
cpu_to_le32(new_dir->i_ino);
BUFFER_TRACE(dir_bh, "call ext4_handle_dirty_metadata");
- retval = ext4_handle_dirty_metadata(handle, old_dir, dir_bh);
+ retval = ext4_handle_dirty_metadata(handle, old_inode, dir_bh);
if (retval) {
ext4_std_error(old_dir->i_sb, retval);
goto end_rename;
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/5] ext4: ext4_mkdir should dirty dir_block with the parent inode
2011-10-21 21:17 [PATCH 1/5] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode Darrick J. Wong
2011-10-21 21:18 ` [PATCH 2/5] ext4: ext4_rename should dirty dir_bh with the correct directory Darrick J. Wong
@ 2011-10-21 21:18 ` Darrick J. Wong
2011-10-25 13:09 ` Ted Ts'o
2011-10-21 21:18 ` [PATCH 4/5] ext4: Prevent stack overrun in ext4_file_open when recording last known mountpoint Darrick J. Wong
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2011-10-21 21:18 UTC (permalink / raw)
To: Tao Ma, Theodore Tso; +Cc: linux-ext4, linux-kernel
ext4_mkdir calls ext4_handle_dirty_metadata with dir_block and the inode "dir".
Unfortunately, dir_block belongs to the newly created directory (which is
"inode"), not the parent directory (which is "dir"). Fix the incorrect
association.
Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---
fs/ext4/namei.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 6d3fab4..50c7294 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1863,7 +1863,7 @@ retry:
ext4_set_de_type(dir->i_sb, de, S_IFDIR);
inode->i_nlink = 2;
BUFFER_TRACE(dir_block, "call ext4_handle_dirty_metadata");
- err = ext4_handle_dirty_metadata(handle, dir, dir_block);
+ err = ext4_handle_dirty_metadata(handle, inode, dir_block);
if (err)
goto out_clear_inode;
err = ext4_mark_inode_dirty(handle, inode);
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4/5] ext4: Prevent stack overrun in ext4_file_open when recording last known mountpoint
2011-10-21 21:17 [PATCH 1/5] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode Darrick J. Wong
2011-10-21 21:18 ` [PATCH 2/5] ext4: ext4_rename should dirty dir_bh with the correct directory Darrick J. Wong
2011-10-21 21:18 ` [PATCH 3/5] ext4: ext4_mkdir should dirty dir_block with the parent inode Darrick J. Wong
@ 2011-10-21 21:18 ` Darrick J. Wong
2011-10-25 13:22 ` Ted Ts'o
2011-10-21 21:18 ` [PATCH 5/5] ext4: Fix endian problem in MMP initialization Darrick J. Wong
2011-10-25 13:13 ` [PATCH 1/5] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode Ted Ts'o
4 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2011-10-21 21:18 UTC (permalink / raw)
To: Tao Ma, Theodore Tso; +Cc: linux-ext4, linux-kernel
In ext4_file_open, the filesystem records the mountpoint of the first file that
is opened after mounting the filesystem. It does this by allocating a 64-byte
stack buffer, calling d_path() to grab the mount point through which this file
was accessed, and then memcpy()ing 64 bytes into the superblock's
s_last_mounted field, starting from the return value of d_path(), which is
stored as "cp". However, if cp > buf (which it frequently is since path
components are prepended starting at the end of buf) then we can end up copying
stack data into the superblock.
Writing stack variables into the superblock doesn't sound like a great idea, so
use strlcpy instead. Andi Kleen suggested using strlcpy instead of strncpy.
Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---
fs/ext4/file.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index e4095e9..9781099 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -181,8 +181,8 @@ static int ext4_file_open(struct inode * inode, struct file * filp)
path.dentry = mnt->mnt_root;
cp = d_path(&path, buf, sizeof(buf));
if (!IS_ERR(cp)) {
- memcpy(sbi->s_es->s_last_mounted, cp,
- sizeof(sbi->s_es->s_last_mounted));
+ strlcpy(sbi->s_es->s_last_mounted, cp,
+ sizeof(sbi->s_es->s_last_mounted));
ext4_mark_super_dirty(sb);
}
}
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4/5] ext4: Prevent stack overrun in ext4_file_open when recording last known mountpoint
2011-10-21 21:18 ` [PATCH 4/5] ext4: Prevent stack overrun in ext4_file_open when recording last known mountpoint Darrick J. Wong
@ 2011-10-25 13:22 ` Ted Ts'o
0 siblings, 0 replies; 11+ messages in thread
From: Ted Ts'o @ 2011-10-25 13:22 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Tao Ma, linux-ext4, linux-kernel
On Fri, Oct 21, 2011 at 02:18:18PM -0700, Darrick J. Wong wrote:
> In ext4_file_open, the filesystem records the mountpoint of the first file that
> is opened after mounting the filesystem. It does this by allocating a 64-byte
> stack buffer, calling d_path() to grab the mount point through which this file
> was accessed, and then memcpy()ing 64 bytes into the superblock's
> s_last_mounted field, starting from the return value of d_path(), which is
> stored as "cp". However, if cp > buf (which it frequently is since path
> components are prepended starting at the end of buf) then we can end up copying
> stack data into the superblock.
>
> Writing stack variables into the superblock doesn't sound like a great idea, so
> use strlcpy instead. Andi Kleen suggested using strlcpy instead of strncpy.
>
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
Thanks, applied.
- Ted
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/5] ext4: Fix endian problem in MMP initialization
2011-10-21 21:17 [PATCH 1/5] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode Darrick J. Wong
` (2 preceding siblings ...)
2011-10-21 21:18 ` [PATCH 4/5] ext4: Prevent stack overrun in ext4_file_open when recording last known mountpoint Darrick J. Wong
@ 2011-10-21 21:18 ` Darrick J. Wong
2011-10-22 17:25 ` Andreas Dilger
2011-10-25 13:35 ` Ted Ts'o
2011-10-25 13:13 ` [PATCH 1/5] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode Ted Ts'o
4 siblings, 2 replies; 11+ messages in thread
From: Darrick J. Wong @ 2011-10-21 21:18 UTC (permalink / raw)
To: Tao Ma, Theodore Tso; +Cc: linux-ext4, linux-kernel
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.
Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---
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)
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] ext4: Fix endian problem in MMP initialization
2011-10-21 21:18 ` [PATCH 5/5] ext4: Fix endian problem in MMP initialization Darrick J. Wong
@ 2011-10-22 17:25 ` Andreas Dilger
2011-10-25 13:35 ` Ted Ts'o
1 sibling, 0 replies; 11+ messages in thread
From: Andreas Dilger @ 2011-10-22 17:25 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Tao Ma, Theodore Tso, linux-ext4@vger.kernel.org, linux-kernel
You can add my
Acked-by: Andreas Dilger <adilger@dilger.ca>
to this.
Cheers, Andreas
On 2011-10-21, at 3:18 PM, "Darrick J. Wong" <djwong@us.ibm.com> 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.
>
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
> 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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] ext4: Fix endian problem in MMP initialization
2011-10-21 21:18 ` [PATCH 5/5] ext4: Fix endian problem in MMP initialization Darrick J. Wong
2011-10-22 17:25 ` Andreas Dilger
@ 2011-10-25 13:35 ` Ted Ts'o
1 sibling, 0 replies; 11+ messages in thread
From: Ted Ts'o @ 2011-10-25 13:35 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Tao Ma, linux-ext4, linux-kernel
On Fri, Oct 21, 2011 at 02:18:25PM -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.
>
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
Thanks, this is already in the ext4 tree, although not yet in the
master branch (just the dev branch).
- Ted
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode
2011-10-21 21:17 [PATCH 1/5] ext4: ext4_dx_add_entry should dirty directory metadata with the directory inode Darrick J. Wong
` (3 preceding siblings ...)
2011-10-21 21:18 ` [PATCH 5/5] ext4: Fix endian problem in MMP initialization Darrick J. Wong
@ 2011-10-25 13:13 ` Ted Ts'o
4 siblings, 0 replies; 11+ messages in thread
From: Ted Ts'o @ 2011-10-25 13:13 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Tao Ma, linux-ext4, linux-kernel
On Fri, Oct 21, 2011 at 02:17:59PM -0700, Darrick J. Wong wrote:
> ext4_dx_add_entry manipulates bh2 and frames[0].bh, which are two buffer_heads
> that point to directory blocks assigned to the directory inode. However, the
> function calls ext4_handle_dirty_metadata with the inode of the file that's
> being added to the directory, not the directory inode itself. Therefore,
> correct the code to dirty the directory buffers with the directory inode, not
> the file inode.
>
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
This has been accpeted into the ext4 tree already (git commit:
5930ea643 in the master branch)
- Ted
^ permalink raw reply [flat|nested] 11+ messages in thread