linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Ext4 Developers List <linux-ext4@vger.kernel.org>
Cc: nnk@google.com, keescook@google.com, Theodore Ts'o <tytso@mit.edu>
Subject: [PATCH] libext2fs: fix potential buffer overflow in closefs()
Date: Sat, 14 Feb 2015 19:51:37 -0500	[thread overview]
Message-ID: <1423961497-9715-1-git-send-email-tytso@mit.edu> (raw)

The bug fix in f66e6ce4446: "libext2fs: avoid buffer overflow if
s_first_meta_bg is too big" had a typo in the fix for
ext2fs_closefs().  In practice most of the security exposure was from
the openfs path, since this meant if there was a carefully crafted
file system, buffer overrun would be triggered when the file system was
opened.

However, if corrupted file system didn't trip over some corruption
check, and then the file system was modified via tune2fs or debugfs,
such that the superblock was marked dirty and then written out via the
closefs() path, it's possible that the buffer overrun could be
triggered when the file system is closed.

Also clear up a signed vs unsigned warning while we're at it.

Thanks to Nick Kralevich <nnk@google.com> for asking me to look at
compiler warning in the code in question, which led me to notice the
bug in f66e6ce4446.

Addresses: CVE-2015-1572

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 lib/ext2fs/closefs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c
index 1f99113..ab5b2fb 100644
--- a/lib/ext2fs/closefs.c
+++ b/lib/ext2fs/closefs.c
@@ -287,7 +287,7 @@ errcode_t ext2fs_flush2(ext2_filsys fs, int flags)
 	dgrp_t		j;
 #endif
 	char	*group_ptr;
-	int	old_desc_blocks;
+	blk64_t	old_desc_blocks;
 	struct ext2fs_numeric_progress_struct progress;
 
 	EXT2_CHECK_MAGIC(fs, EXT2_ET_MAGIC_EXT2FS_FILSYS);
@@ -346,7 +346,7 @@ errcode_t ext2fs_flush2(ext2_filsys fs, int flags)
 	group_ptr = (char *) group_shadow;
 	if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG) {
 		old_desc_blocks = fs->super->s_first_meta_bg;
-		if (old_desc_blocks > fs->super->s_first_meta_bg)
+		if (old_desc_blocks > fs->desc_blocks)
 			old_desc_blocks = fs->desc_blocks;
 	} else
 		old_desc_blocks = fs->desc_blocks;
-- 
2.1.0


                 reply	other threads:[~2015-02-15  0:52 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=1423961497-9715-1-git-send-email-tytso@mit.edu \
    --to=tytso@mit.edu \
    --cc=keescook@google.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=nnk@google.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).