linux-ext4 - Re: fsstress-induced corruption reproduced

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4B424BE4.3030605@redhat.com>
Date:	Mon, 04 Jan 2010 14:13:24 -0600
From:	Eric Sandeen <sandeen@...hat.com>
To:	"Theodore Ts'o" <tytso@....edu>
CC:	linux-ext4@...r.kernel.org
Subject: Re: fsstress-induced corruption reproduced

Theodore Ts'o wrote:
> One of the things which has been annoying me for a while now is a
> hard-to-reproduce xfsqa failure in test #13 (fsstress), which causes the
> a test failure because the file system found to be inconsistent:
> 
> Inode NNN, i_blocks is X, should be Y.

Interesting, this apparently has gotten much worse since 2.6.32.

I wrote an xfstests reproducer, and couldn't hit it on .32; hit it right
off on 2.6.33-rc2.

Probably should find out why ;) I'll go take a look.

-Eric

> I finally reproduced it; the problem happens when we fallocate() a
> region of the file which we had recently written, and which is still in
> the page cache marked as delayed allocation blocks.  When we finally
> write those blocks out, since they are marked BH_Delay,
> ext4_get_blocks() calls ext4_da_update_reserve_space(), which ends up
> bumping i_blocks a second time and charging the blocks against the
> user's quota a second time.  Oops.
> 
> Fortunately the fsck problem is one that will be fixed with a preen (and
> if quota is enabled, a quotacheck), so it's not super serious, but we
> should fix it when we have a chance.  If anyone has time to look at it,
> please let me know.  Otherwise, I'll put it on my todo list.  I don't
> consider seriously urgent since the case is highly unlikely to occur in
> real life, and it doesn't have any security implications; the worst an
> attacker could do is end up charging excesss quota to herself.
> 
> I've included a simple reproduction case below; if you run this program,
> it will create a file "test-file" in the current working directory which
> will appear to be 32k, even though it is really only 16k long, and if
> you then unmount the test file system and run e2fsck -p on it, you will get
> the error message:
> 
> Inode XXX, i_blocks is 64, should be 32.  FIXED.
> 
> 	       	     	    	     	     - Ted
> 
> #define _GNU_SOURCE
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <string.h>
> #include <sys/types.h>
> #include <fcntl.h>
> #include <fcntl.h>
> 
> #define BUFSIZE 1024
> 
> int main(int argc, char **argv)
> {
> 	int	i, fd, ret;
> 	char	buf[BUFSIZE];
> 
> 	fd = open("test-file", O_RDWR|O_CREAT|O_TRUNC, 0644);
> 	if (fd < 0) {
> 		perror("open");
> 		exit(1);
> 	}
> 	memset(&buf, 0, BUFSIZE);
> 	for (i=0; i < 16; i++) {
> 		ret = write(fd, &buf, BUFSIZE);
> 		if (ret < 0) {
> 			perror("write");
> 			exit(1);
> 		}
> 		if (ret != BUFSIZE) {
> 			fprintf(stderr, "Write return expected %d, got %d\n",
> 				BUFSIZE, ret);
> 			exit(1);
> 		}
> 	}
> 	ret = fallocate(fd, 0, 0, 16384);
> 	if (ret < 0) {
> 		perror("fallocate");
> 		exit(1);
> 	}
> 	ret = fsync(fd);
> 	if (ret < 0) {
> 		perror("fsync");
> 		exit(1);
> 	}
> 	ret = close(fd);
> 	if (ret < 0) {
> 		perror("close");
> 		exit(1);
> 	}
> 	exit(0);
> }
> --
> 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

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