| 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
| ||
|
Date: Mon, 03 Aug 2009 17:30:59 +0900
From: Akira Fujita <a-fujita@...jp.nec.com>
To: Peng Tao <bergwolf@...il.com>
CC: linux-ext4@...r.kernel.org, "Theodore Ts'o" <tytso@....edu>
Subject: Re: [PATCH 2/2] ext4: fix null pointer bug in move_extent.c
Peng Tao wrote:
> In mext_replace_branches(), the oext and dext virable might be NULL if the
> orig extent and donor extent are empty. Later calling *oext and *dext will
> trigger a kernel null pointer bug like this:
>
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<ffffffffa0636563>] mext_replace_branches+0x10c/0x2f3 [ext4]
> PGD 37dba067 PUD cd8ac067 PMD 0
> Oops: 0000 [#1] SMP
>
> The patch checks the two virables and returns EOPNOTSUPP if either of them
> is NULL.
>
> Signed-off-by: Peng Tao <bergwolf@...il.com>
> ---
> fs/ext4/move_extent.c | 12 ++++++++++++
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 5821e0b..4923f70 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -641,10 +641,22 @@ mext_replace_branches(handle_t *handle, struct inode *orig_inode,
> goto out;
> depth = ext_depth(orig_inode);
> oext = orig_path[depth].p_ext;
> + if (oext == NULL) {
> + ext4_debug("ext4 move extent: "
> + "orig extents should not be empty\n");
> + err = -EOPNOTSUPP;
> + goto out;
> + }
> tmp_oext = *oext;
>
> depth = ext_depth(donor_inode);
> dext = donor_path[depth].p_ext;
> + if (dext == NULL) {
> + ext4_debug("ext4 move extent: "
> + "donor extents should not be empty\n");
> + err = -EOPNOTSUPP;
> + goto out;
> + }
> tmp_dext = *dext;
>
> mext_calc_swap_extents(&tmp_dext, &tmp_oext, orig_off,
Yes, this problem occurs if extent which ext4_ext_path indicates is NULL,
but this check should be done in ext4_move_extents()
which is called before mext_replace_branches().
And in this case, ENOENT is better for error value, I think.
How about this patch?
Signed-off-by: Akira Fujita <a-fujita@...jp.nec.com>
---
move_extent.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
--- ../../LATEST/fs/ext4/move_extent.c 2009-08-03 14:53:43.000000000 +0900
+++ fs/ext4/move_extent.c 2009-08-03 15:03:33.000000000 +0900
@@ -1145,19 +1145,23 @@ ext4_move_extents(struct file *o_filp, s
if (file_end < block_end)
len -= block_end - file_end;
+ depth = ext_depth(orig_inode);
get_ext_path(orig_path, orig_inode, block_start, ret);
if (orig_path == NULL)
goto out2;
+ else if (orig_path[depth].p_ext == NULL) {
+ ret = -ENOENT;
+ goto out;
+ }
/* Get path structure to check the hole */
get_ext_path(holecheck_path, orig_inode, block_start, ret);
if (holecheck_path == NULL)
goto out;
- depth = ext_depth(orig_inode);
ext_cur = holecheck_path[depth].p_ext;
if (ext_cur == NULL) {
- ret = -EINVAL;
+ ret = -ENOENT;
goto out;
}
@@ -1280,11 +1284,14 @@ ext4_move_extents(struct file *o_filp, s
/* Decrease buffer counter */
if (holecheck_path)
ext4_ext_drop_refs(holecheck_path);
- get_ext_path(holecheck_path, orig_inode,
- seq_start, ret);
+ get_ext_path(holecheck_path, orig_inode, seq_start, ret);
if (holecheck_path == NULL)
break;
depth = holecheck_path->p_depth;
+ if (holecheck_path[depth].p_ext == NULL) {
+ ret = -ENOENT;
+ break;
+ }
/* Decrease buffer counter */
if (orig_path)
@@ -1292,6 +1299,10 @@ ext4_move_extents(struct file *o_filp, s
get_ext_path(orig_path, orig_inode, seq_start, ret);
if (orig_path == NULL)
break;
+ else if (orig_path[depth].p_ext == NULL) {
+ ret = -ENOENT;
+ break;
+ }
ext_cur = holecheck_path[depth].p_ext;
add_blocks = ext4_ext_get_actual_len(ext_cur);
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists