NULL pointer when make_indexed_dir returns -ENOSPC

PATCH Applied Allison Henderson 3 participants 5 messages May 12, 2011 – May 18, 2011 ext4 file system

lore

Share & tools

lore

Allison Henderson

author

2011-05-12 17:59 UTC #0 lore raw

PATCH v2 1/1 NULL pointer when make_indexed_dir returns -ENOSPC

Hi all,

I wasnt sure if people were expecting a v2 for this one, but I noticed the modified version of v1 was removed from the tree, so I am assuming we needed v2? This one handles marking the buffer dirty before calling do split to avoid the null pointer. It introduces a little overhead, but the only other option would be to introduce a new flag set to do_split. But if there's any one that would prefer the flags to this solution, please let me know. Thx! :)

Allison Henderson

Signed-off-by Allison Henderson <[email protected]>

---
:100644 100644 3c7a06e... b754b77... M fs/ext4/namei.c

 fs/ext4/namei.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

fs/ext4/namei.c +4 -2

index 3c7a06e..b754b77 100644
@@ -1413,6 +1413,10 @@ static int make_indexed_dir(handle_t *handle, struct dentry *dentry,
 	frame->at = entries;
 	frame->bh = bh;
 	bh = bh2;
+
+	ext4_handle_dirty_metadata(handle, dir, frame->bh);
+	ext4_handle_dirty_metadata(handle, dir, bh);
+
 	de = do_split(handle,dir, &bh, frame, &hinfo, &retval);
 	if (!de) {
 		/*

@@ -1421,8 +1425,6 @@ static int make_indexed_dir(handle_t *handle, struct dentry *dentry,
 		 * with corrupted filesystem.
 		 */
 		ext4_mark_inode_dirty(handle, dir);
-		ext4_handle_dirty_metadata(handle, dir, frame->bh);
-		ext4_handle_dirty_metadata(handle, dir, bh);
 		dx_release(frames);
 		return retval;
 	}

Signature · 1 line

1.7.1

Ted Ts'o

maintainer Applied

2011-05-15 04:21 UTC #1 lore raw

Quoted 10 lines · Allison Henderson

Hi all,
I wasnt sure if people were expecting a v2 for this one, but I noticed the modified version of v1 was removed from the tree, so I am assuming we needed v2? This one handles marking the buffer dirty before calling do split to avoid the null pointer. It introduces a little overhead, but the only other option would be to introduce a new flag set to do_split. But if there's any one that would prefer the flags to this solution, please let me know. Thx! :)

Quoted 3 lines

Allison Henderson
Signed-off-by Allison Henderson <[email protected]>

Applied, thanks. In the future, I'd appreciate it if you resent the patch with a formal commit description. I recreated the commit description from the older patch in this case.

- Ted

Allison Henderson

author

2011-05-15 15:05 UTC #2 lore raw

Quoted 13 lines · Ted Ts'o

Quoted 12 lines · Allison Henderson
Hi all,
I wasnt sure if people were expecting a v2 for this one, but I noticed the modified version of v1 was removed from the tree, so I am assuming we needed v2? This one handles marking the buffer dirty before calling do split to avoid the null pointer. It introduces a little overhead, but the only other option would be to introduce a new flag set to do_split. But if there's any one that would prefer the flags to this solution, please let me know. Thx! :)
Allison Henderson

Quoted 7 lines

Quoted 1 line
Signed-off-by Allison Henderson<[email protected]>
Applied, thanks. In the future, I'd appreciate it if you resent the patch with a formal commit description. I recreated the commit description from the older patch in this case.
- Ted

Thanks Ted. Sorry about the description, I think I have another small patch out there like that. I will send out an update. Thx!

Allison Henderson

Quoted 4 lines

--
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

Jan Kara

reviewer

2011-05-17 17:45 UTC #3 lore raw

In reply to Allison Henderson

Hello,

Quoted 7 lines · Allison Henderson

I wasnt sure if people were expecting a v2 for this one, but I noticed the modified version of v1 was removed from the tree, so I am assuming we needed v2? This one handles marking the buffer dirty before calling do split to avoid the null pointer. It introduces a little overhead, but the only other option would be to introduce a new flag set to do_split. But if there's any one that would prefer the flags to this solution, please let me know. Thx! :)

The patch looks OK. Just please CC me next time - a) because ext3 needs a
similar fix (added now) and b) because I was involved with the previous patch version. Thanks!

Honza

Quoted 40 lines

Signed-off-by Allison Henderson <[email protected]>

---
:100644 100644 3c7a06e... b754b77... M fs/ext4/namei.c

 fs/ext4/namei.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

fs/ext4/namei.c +4 -0

index 3c7a06e..b754b77 100644
@@ -1413,6 +1413,10 @@ static int make_indexed_dir(handle_t *handle,
 struct dentry *dentry,
 	frame->at = entries;
 	frame->bh = bh;
 	bh = bh2;
+
+	ext4_handle_dirty_metadata(handle, dir, frame->bh);
+	ext4_handle_dirty_metadata(handle, dir, bh);
+
 	de = do_split(handle,dir, &bh, frame, &hinfo, &retval);
 	if (!de) {

fs/ext4/namei.c +0 -2

/*

@@ -1421,8 +1425,6 @@ static int make_indexed_dir(handle_t *handle,
 struct dentry *dentry,
 		 * with corrupted filesystem.
 		 */
 		ext4_mark_inode_dirty(handle, dir);
-		ext4_handle_dirty_metadata(handle, dir, frame->bh);
-		ext4_handle_dirty_metadata(handle, dir, bh);
 		dx_release(frames);
 		return retval;

}

Signature · 6 lines

1.7.1

--
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

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

Allison Henderson

author

2011-05-18 15:23 UTC #4 lore raw

Quoted 10 lines · Jan Kara

Hello,
Quoted 7 lines · Allison Henderson
I wasnt sure if people were expecting a v2 for this one, but I noticed the modified version of v1 was removed from the tree, so I am assuming we needed v2? This one handles marking the buffer dirty before calling do split to avoid the null pointer. It introduces a little overhead, but the only other option would be to introduce a new flag set to do_split. But if there's any one that would prefer the flags to this solution, please let me know. Thx! :)

Quoted 5 lines

The patch looks OK. Just please CC me next time - a) because ext3 needs a
similar fix (added now) and b) because I was involved with the previous patch version. Thanks!
Honza

Sure, I just thought I would save people from the extra spam since it was already seen, but I will keep you cc'd next time. Thx!

Allison Henderson

Quoted 40 lines

Signed-off-by Allison Henderson<[email protected]>

---
:100644 100644 3c7a06e... b754b77... M fs/ext4/namei.c

  fs/ext4/namei.c |    6 ++++--
  1 files changed, 4 insertions(+), 2 deletions(-)

fs/ext4/namei.c +4 -0

index 3c7a06e..b754b77 100644
@@ -1413,6 +1413,10 @@ static int make_indexed_dir(handle_t *handle,
 struct dentry *dentry,
  	frame->at = entries;
  	frame->bh = bh;
  	bh = bh2;
+
+	ext4_handle_dirty_metadata(handle, dir, frame->bh);
+	ext4_handle_dirty_metadata(handle, dir, bh);
+
  	de = do_split(handle,dir,&bh, frame,&hinfo,&retval);
  	if (!de) {

fs/ext4/namei.c +0 -2

@@ -1421,8 +1425,6 @@ static int make_indexed_dir(handle_t *handle,
 struct dentry *dentry,
  		 * with corrupted filesystem.
  		 */
  		ext4_mark_inode_dirty(handle, dir);
-		ext4_handle_dirty_metadata(handle, dir, frame->bh);
-		ext4_handle_dirty_metadata(handle, dir, bh);
  		dx_release(frames);
  		return retval;

}
--
1.7.1

--
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