linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@redhat.com>
To: 7eggert@gmx.de
Cc: linux-kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	security@kernel.org
Subject: [PATCH V3] limit minixfs printks on corrupted dir i_size, CVE-2006-6058
Date: Mon, 13 Aug 2007 12:54:40 -0500	[thread overview]
Message-ID: <46C09AE0.5010902@redhat.com> (raw)
In-Reply-To: <E1IJFr4-0000is-Ox@be1.lrz>

Bodo Eggert wrote:

> Warning: I'm only looking at the patch.
>
> You are supposed to print an error message for a user, not to write in a
> chat window to a 1337 script kiddie. OK, you just matched the current style,
> and your patch is IMHO OK for a quick security fix, but:
>
> - Security fixes should be CCed to the security mailing list, shouldn't they?
>   (It might be security@ or stable@, I'll remember tomorrow, but then I'd
>    forget to comment)
> - Imagine you have three mounts containing a minix fs, how can you tell which
>   one is the the defective one?
> - The message says "minix_bmap", while the patch suggests it's in
>   block_to_path. Therefore I asume "minix_bmap" to have only random
>   informational value.
> - Does block < 0 or block > $size make a difference?
> - the printk lacks the loglevel.
> - Asuming minix supports error handling, shouldn't it do something?
>
> I'd suggest a message saying something like "minix: Bad block address on
> device 08:15, needs fsck".
>   
Ok, do you like this slightly better?  It states the subsystem, the 
function with the error, the block nr. in the case of a too-large block,
and the block device on which the error occurred.  Honestly minix.fsck
doesn't handle the situation well either, so at this point I hesitate
to recommend it in the print.  :)

--------------------------

This attempts to address CVE-2006-6058
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2006-6058
 
first reported at http://projects.info-pull.com/mokb/MOKB-17-11-2006.html

Essentially a corrupted minix dir inode reporting a very large
i_size will loop for a very long time in minix_readdir, minix_find_entry,
etc, because on EIO they just move on to try the next page.  This is
under the BKL, printk-storming as well.  This can lock up the machine
for a very long time.  Simply ratelimiting the printks gets things back
under control.  Make the message a bit more informative while we're here.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Index: linux-2.6.22-rc4/fs/minix/itree_v1.c
===================================================================
--- linux-2.6.22-rc4.orig/fs/minix/itree_v1.c
+++ linux-2.6.22-rc4/fs/minix/itree_v1.c
@@ -23,11 +23,16 @@ static inline block_t *i_data(struct ino
 static int block_to_path(struct inode * inode, long block, int offsets[DEPTH])
 {
 	int n = 0;
+	char b[BDEVNAME_SIZE];
 
 	if (block < 0) {
-		printk("minix_bmap: block<0\n");
+		printk("MINIX-fs: block_to_path: block %ld < 0 on dev %s\n",
+			block, bdevname(inode->i_sb->s_bdev, b));
 	} else if (block >= (minix_sb(inode->i_sb)->s_max_size/BLOCK_SIZE)) {
-		printk("minix_bmap: block>big\n");
+		if (printk_ratelimit())
+			printk("MINIX-fs: block_to_path: "
+			       "block %ld too big on dev %s\n",
+				block, bdevname(inode->i_sb->s_bdev, b));
 	} else if (block < 7) {
 		offsets[n++] = block;
 	} else if ((block -= 7) < 512) {
Index: linux-2.6.22-rc4/fs/minix/itree_v2.c
===================================================================
--- linux-2.6.22-rc4.orig/fs/minix/itree_v2.c
+++ linux-2.6.22-rc4/fs/minix/itree_v2.c
@@ -23,12 +23,17 @@ static inline block_t *i_data(struct ino
 static int block_to_path(struct inode * inode, long block, int offsets[DEPTH])
 {
 	int n = 0;
+	char b[BDEVNAME_SIZE];
 	struct super_block *sb = inode->i_sb;
 
 	if (block < 0) {
-		printk("minix_bmap: block<0\n");
+		printk("MINIX-fs: block_to_path: block %ld < 0 on dev %s\n",
+			block, bdevname(sb->s_bdev, b));
 	} else if (block >= (minix_sb(inode->i_sb)->s_max_size/sb->s_blocksize)) {
-		printk("minix_bmap: block>big\n");
+		if (printk_ratelimit())
+			printk("MINIX-fs: block_to_path: "
+			       "block %ld too big on dev %s\n",
+				block, bdevname(sb->s_bdev, b));
 	} else if (block < 7) {
 		offsets[n++] = block;
 	} else if ((block -= 7) < 256) {



  parent reply	other threads:[~2007-08-13 17:56 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <8Qlff-3jX-29@gated-at.bofh.it>
     [not found] ` <8Qn7m-6li-27@gated-at.bofh.it>
2007-08-09 21:47   ` [PATCH V2] limit minixfs printks on corrupted dir i_size, CVE-2006-6058 Bodo Eggert
2007-08-09 22:08     ` Eric Sandeen
2007-08-13 17:54     ` Eric Sandeen [this message]
2007-08-15 11:51       ` [PATCH V3] " Bodo Eggert
2007-08-15 14:59         ` Eric Sandeen

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=46C09AE0.5010902@redhat.com \
    --to=sandeen@redhat.com \
    --cc=7eggert@gmx.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=security@kernel.org \
    /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).