linux-kernel - Re: [PATCH] fat: fix corruption in fat_alloc_new

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: <87v9u3xf5q.fsf@mail.parknet.co.jp>
Date:   Sun, 08 Sep 2019 19:20:49 +0900
From:   OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
To:     Jan Stancek <jstancek@...hat.com>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fat: fix corruption in fat_alloc_new_dir()

Jan Stancek <jstancek@...hat.com> writes:

> sb_getblk does not guarantee that buffer_head is uptodate. If there is
> async read running in parallel for same buffer_head, it can overwrite
> just initialized msdos_dir_entry, leading to corruption:
>   FAT-fs (loop0): error, corrupted directory (invalid entries)
>   FAT-fs (loop0): Filesystem has been set read-only
>
> This can happen for example during LTP statx04, which creates loop
> device, formats it (mkfs.vfat), mounts it and immediately creates
> a new directory. In parallel, systemd-udevd is probing new block
> device, which leads to async read.
>
>   do_mkdirat                      ksys_read
>    vfs_mkdir                       vfs_read
>     vfat_mkdir                      __vfs_read
>      fat_alloc_new_dir               new_sync_read
>        /* init de[0], de[1] */        blkdev_read_iter
>                                        generic_file_read_iter
>                                         generic_file_buffered_read
>                                          blkdev_readpage
>                                           block_read_full_page
>
> Faster reproducer (based on LTP statx04):
>
> int main(void)
> {
> 	int i, j, ret, fd, loop_fd, ctrl_fd;
> 	int loop_num;
> 	char loopdev[256], tmp[256], testfile[256];
>
> 	mkdir("/tmp/mntpoint", 0777);
> 	for (i = 0; ; i++) {
> 		printf("Iteration: %d\n", i);
> 		sprintf(testfile, "/tmp/test.img.%d", getpid());
>
> 		ctrl_fd = open("/dev/loop-control", O_RDWR);
> 		loop_num = ioctl(ctrl_fd, LOOP_CTL_GET_FREE);
> 		close(ctrl_fd);
> 		sprintf(loopdev, "/dev/loop%d", loop_num);
>
> 		fd = open(testfile, O_WRONLY|O_CREAT|O_TRUNC, 0600);
> 		fallocate(fd, 0, 0, 256*1024*1024);
> 		close(fd);
>
> 		fd = open(testfile, O_RDWR);
> 		loop_fd = open(loopdev, O_RDWR);
> 		ioctl(loop_fd, LOOP_SET_FD, fd);
> 		close(loop_fd);
> 		close(fd);
>
> 		sprintf(tmp, "mkfs.vfat %s", loopdev);
> 		system(tmp);
> 		mount(loopdev, "/tmp/mntpoint", "vfat", 0, NULL);
>
> 		for (j = 0; j < 200; j++) {
> 			sprintf(tmp, "/tmp/mntpoint/testdir%d", j);
> 			ret = mkdir(tmp, 0777);
> 			if (ret) {
> 				perror("mkdir");
> 				break;
> 			}
> 		}
>
> 		umount("/tmp/mntpoint");
> 		loop_fd = open(loopdev, O_RDWR);
> 		ioctl(loop_fd, LOOP_CLR_FD, fd);
> 		close(loop_fd);
> 		unlink(testfile);
>
> 		if (ret)
> 			break;
> 	}
>
> 	return 0;
> }
>
> Issue triggers within minute on HPE Apollo 70 (arm64, 64GB RAM, 224 CPUs).

Using the device while mounting same device doesn't work reliably like
this race.  (getblk() is intentionally used to get the buffer to write
new data.)

mount(2) internally opens the device by EXCL mode, so I guess udev opens
without EXCL (I dont know if it is intent or not).

Thanks.
-- 
OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>