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) {
next prev parent reply other threads:[~2007-08-13 18:11 UTC|newest]
Thread overview: 6+ 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 21:47 ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.