Hard lock when mounting loopback file

Hard lock when mounting loopback file

[Kyle]

I have a digital camera flash card that is locking up my machine (stock 
redhat 7.2 w/2.4.9-13 kernel).

I can mount the card, but as soon as I browse the filesystem, the 
machine locks hard.  I successfully copied the file system from the raw 
device to a file and tried mounting it as:

mount -o loop flash.img /mnt/flash

and it still locks up the machine just as before.  This makes me think 
it has nothing to do with the USB reader or the SCSI emulation, etc.

My guess is I have a corrupt filesystem on the flash that the filesystem 
handler (vfat) is intolerant of (all my other flash cards work fine).  

This seems like a possible kernel bug to me.  I'm not much of a kernel 
expert but I have a copy of the offending image if anyone wants to or 
can look at it.  (ftp://actarg.com/pub/misc/flash.img)  Is there someone 
that knows how to figure out if the driver can spit out a harmless 
message about filesystem corruption rather than taking the whole kernel 
down?

Kyle Bateman

Re: Hard lock when mounting loopback file

[Andrew Morton]

Kyle wrote:
> 
> I have a digital camera flash card that is locking up my machine (stock
> redhat 7.2 w/2.4.9-13 kernel).
> 
> I can mount the card, but as soon as I browse the filesystem, the
> machine locks hard.  I successfully copied the file system from the raw
> device to a file and tried mounting it as:
> 
> mount -o loop flash.img /mnt/flash
> 
> and it still locks up the machine just as before.  This makes me think
> it has nothing to do with the USB reader or the SCSI emulation, etc.
> 
> My guess is I have a corrupt filesystem on the flash that the filesystem
> handler (vfat) is intolerant of (all my other flash cards work fine).
> 
> This seems like a possible kernel bug to me.  I'm not much of a kernel
> expert but I have a copy of the offending image if anyone wants to or
> can look at it.  (ftp://actarg.com/pub/misc/flash.img)  Is there someone
> that knows how to figure out if the driver can spit out a harmless
> message about filesystem corruption rather than taking the whole kernel
> down?
> 

I don't know a thing about fat layout, but it appears that it uses a
linked list of blocks, and if that list ends up pointing back onto
itself, the kernel goes into an infinite loop in several places chasing
its way to the end of the list.

The below patch fixed it for me, and I was able to mount and read
your filesystem image.

Unless someone has a smarter fix, I'll send this to the kernel
maintainers in a week or two.



--- linux-2.4.18-pre3/fs/fat/misc.c	Fri Oct 12 13:48:42 2001
+++ linux-akpm/fs/fat/misc.c	Sat Jan 12 23:28:03 2002
@@ -478,6 +478,8 @@ static int raw_scan_nonroot(struct super
 	printk("raw_scan_nonroot: start=%d\n",start);
 #endif
 	do {
+		int old_start = start;
+
 		for (count = 0; count < MSDOS_SB(sb)->cluster_size; count++) {
 			if ((cluster = raw_scan_sector(sb,(start-2)*
 			    MSDOS_SB(sb)->cluster_size+MSDOS_SB(sb)->data_start+
@@ -486,6 +488,11 @@ static int raw_scan_nonroot(struct super
 		}
 		if (!(start = fat_access(sb,start,-1))) {
 			fat_fs_panic(sb,"FAT error");
+			break;
+		}
+		if (start == old_start) {
+			/* Prevent infinite loop on corrupt fs */
+			fat_fs_panic(sb, "FAT loop");
 			break;
 		}
 #ifdef DEBUG
--- linux-2.4.18-pre3/fs/fat/inode.c	Thu Nov 22 23:02:58 2001
+++ linux-akpm/fs/fat/inode.c	Sat Jan 12 23:37:44 2002
@@ -392,12 +392,18 @@ static void fat_read_root(struct inode *
 		MSDOS_I(inode)->i_start = sbi->root_cluster;
 		if ((nr = MSDOS_I(inode)->i_start) != 0) {
 			while (nr != -1) {
+				int old_nr = nr;
 				inode->i_size += 1 << sbi->cluster_bits;
 				if (!(nr = fat_access(sb, nr, -1))) {
 					printk("Directory %ld: bad FAT\n",
 					       inode->i_ino);
 					break;
 				}
+				if (nr == old_nr) {
+					printk("Directory %ld: FAT loop\n",
+					       inode->i_ino);
+					break;
+				}
 			}
 		}
 	} else {
@@ -918,9 +924,15 @@ static void fat_fill_inode(struct inode 
 #endif
 		if ((nr = MSDOS_I(inode)->i_start) != 0)
 			while (nr != -1) {
+				int old_nr = nr;
 				inode->i_size += 1 << sbi->cluster_bits;
 				if (!(nr = fat_access(sb, nr, -1))) {
 					printk("Directory %ld: bad FAT\n",
+					    inode->i_ino);
+					break;
+				}
+				if (nr == old_nr) {
+					printk("Directory %ld: FAT loop\n",
 					    inode->i_ino);
 					break;
 				}

Re: Hard lock when mounting loopback file

[Marius Gedminas]

On Sat, Jan 12, 2002 at 11:49:04PM -0800, Andrew Morton wrote:
> I don't know a thing about fat layout, but it appears that it uses a
> linked list of blocks, and if that list ends up pointing back onto
> itself, the kernel goes into an infinite loop in several places chasing
> its way to the end of the list.
> 
> The below patch fixed it for me, and I was able to mount and read
> your filesystem image.
> 
> Unless someone has a smarter fix, I'll send this to the kernel
> maintainers in a week or two.

It seems to me that this patch will find only those infinite loops where
the last link of the chain points to itself.  But there could be loops
where the last link points to the middle of the chain.

Additional check on the number of followed links could be useful there.
No chain should be longer than the number of clusters on the fs.
Although on large FAT32 filesystems the number of clusters can be high,
a very long loop is still better than an infinite one.  (In cases where
we know the file size, this limit can be reduced to
file_size/cluster_size + 1 links).

Marius Gedminas
-- 
This company has performed an illegal operation and will be shut down. If
the problem persists, contact your vendor or appeal to a higher court.

Re: Hard lock when mounting loopback file

[Andrew Morton]

Marius Gedminas wrote:
> 
> On Sat, Jan 12, 2002 at 11:49:04PM -0800, Andrew Morton wrote:
> > I don't know a thing about fat layout, but it appears that it uses a
> > linked list of blocks, and if that list ends up pointing back onto
> > itself, the kernel goes into an infinite loop in several places chasing
> > its way to the end of the list.
> >
> > The below patch fixed it for me, and I was able to mount and read
> > your filesystem image.
> >
> > Unless someone has a smarter fix, I'll send this to the kernel
> > maintainers in a week or two.
> 
> It seems to me that this patch will find only those infinite loops where
> the last link of the chain points to itself.  But there could be loops
> where the last link points to the middle of the chain.

Agree.

> Additional check on the number of followed links could be useful there.
> No chain should be longer than the number of clusters on the fs.
> Although on large FAT32 filesystems the number of clusters can be high,
> a very long loop is still better than an infinite one.  (In cases where
> we know the file size, this limit can be reduced to
> file_size/cluster_size + 1 links).

hmm..  OK, I'll take a look at that approach.

-

Re: Hard lock when mounting loopback file

[OGAWA Hirofumi]

Andrew Morton <akpm@zip.com.au> writes:

> > Additional check on the number of followed links could be useful there.
> > No chain should be longer than the number of clusters on the fs.
> > Although on large FAT32 filesystems the number of clusters can be high,
> > a very long loop is still better than an infinite one.  (In cases where
> > we know the file size, this limit can be reduced to
> > file_size/cluster_size + 1 links).
> 
> hmm..  OK, I'll take a look at that approach.

The maximum number of directory entries is 65536 in ordinarily.  The
following patch prevents the infinite loop of a directory (include the
limit to added directory).

I think the following patch is a bit useful.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

diff -urN linux-2.5.2-pre11/fs/fat/dir.c fat_loop/fs/fat/dir.c
--- linux-2.5.2-pre11/fs/fat/dir.c	Sat Oct 13 05:48:42 2001
+++ fat_loop/fs/fat/dir.c	Mon Jan 14 17:40:48 2002
@@ -727,7 +727,13 @@
 	offset = curr = 0;
 	*bh = NULL;
 	row = 0;
-	while (fat_get_entry(dir,&curr,bh,de,ino) > -1) {
+	while (fat_get_entry(dir, &curr, bh, de, ino) > -1) {
+		/* check the maximum size of directory */
+		if (curr >= FAT_MAX_DIR_SIZE) {
+			fat_brelse(sb, *bh);
+			return -ENOSPC;
+		}
+
 		if (IS_FREE((*de)->name)) {
 			if (++row == slots)
 				return offset;
@@ -742,7 +748,10 @@
 	if (!new_bh)
 		return -ENOSPC;
 	fat_brelse(sb, new_bh);
-	do fat_get_entry(dir,&curr,bh,de,ino); while (++row<slots);
+	do {
+		fat_get_entry(dir, &curr, bh, de, ino);
+	} while (++row < slots);
+
 	return offset;
 }
 
diff -urN linux-2.5.2-pre11/fs/fat/inode.c fat_loop/fs/fat/inode.c
--- linux-2.5.2-pre11/fs/fat/inode.c	Mon Jan 14 18:35:34 2002
+++ fat_loop/fs/fat/inode.c	Mon Jan 14 03:45:33 2002
@@ -373,11 +373,37 @@
 	return ret;
 }
 
+static void fat_calc_dir_size(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+	int nr;
+
+	inode->i_size = 0;
+	if (MSDOS_I(inode)->i_start == 0)
+		return;
+
+	nr = MSDOS_I(inode)->i_start;
+	do {
+		inode->i_size += 1 << MSDOS_SB(sb)->cluster_bits;
+		if (!(nr = fat_access(sb, nr, -1))) {
+			printk("FAT: Directory %ld: bad FAT\n",
+			       inode->i_ino);
+			break;
+		}
+		if (inode->i_size > FAT_MAX_DIR_SIZE) {
+			fat_fs_panic(sb, "Directory %ld: "
+				     "exceeded the maximum size of directory",
+				     inode->i_ino);
+			inode->i_size = FAT_MAX_DIR_SIZE;
+			break;
+		}
+	} while (nr != -1);
+}
+
 static void fat_read_root(struct inode *inode)
 {
 	struct super_block *sb = inode->i_sb;
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
-	int nr;
 
 	INIT_LIST_HEAD(&MSDOS_I(inode)->i_fat_hash);
 	MSDOS_I(inode)->i_location = 0;
@@ -391,16 +417,7 @@
 	inode->i_fop = &fat_dir_operations;
 	if (sbi->fat_bits == 32) {
 		MSDOS_I(inode)->i_start = sbi->root_cluster;
-		if ((nr = MSDOS_I(inode)->i_start) != 0) {
-			while (nr != -1) {
-				inode->i_size += 1 << sbi->cluster_bits;
-				if (!(nr = fat_access(sb, nr, -1))) {
-					printk("Directory %ld: bad FAT\n",
-					       inode->i_ino);
-					break;
-				}
-			}
-		}
+		fat_calc_dir_size(inode);
 	} else {
 		MSDOS_I(inode)->i_start = 0;
 		inode->i_size = sbi->dir_entries * sizeof(struct msdos_dir_entry);
@@ -882,7 +899,6 @@
 {
 	struct super_block *sb = inode->i_sb;
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
-	int nr;
 
 	INIT_LIST_HEAD(&MSDOS_I(inode)->i_fat_hash);
 	MSDOS_I(inode)->i_location = 0;
@@ -913,15 +929,7 @@
 			inode->i_nlink = 1;
 		}
 #endif
-		if ((nr = MSDOS_I(inode)->i_start) != 0)
-			while (nr != -1) {
-				inode->i_size += 1 << sbi->cluster_bits;
-				if (!(nr = fat_access(sb, nr, -1))) {
-					printk("Directory %ld: bad FAT\n",
-					    inode->i_ino);
-					break;
-				}
-			}
+		fat_calc_dir_size(inode);
 		MSDOS_I(inode)->mmu_private = inode->i_size;
 	} else { /* not a directory */
 		inode->i_generation |= 1;
diff -urN linux-2.5.2-pre11/fs/fat/misc.c fat_loop/fs/fat/misc.c
--- linux-2.5.2-pre11/fs/fat/misc.c	Mon Jan 14 18:35:34 2002
+++ fat_loop/fs/fat/misc.c	Mon Jan 14 06:13:24 2002
@@ -38,15 +38,25 @@
  * read-only. The file system can be made writable again by remounting it.
  */
 
-void fat_fs_panic(struct super_block *s,const char *msg)
+static char panic_msg[512];
+
+void fat_fs_panic(struct super_block *s, const char *fmt, ...)
 {
 	int not_ro;
+	va_list args;
+
+	va_start (args, fmt);
+	vsnprintf (panic_msg, sizeof(panic_msg), fmt, args);
+	va_end (args);
 
 	not_ro = !(s->s_flags & MS_RDONLY);
-	if (not_ro) s->s_flags |= MS_RDONLY;
-	printk("Filesystem panic (dev %s).\n  %s\n", s->s_id, msg);
 	if (not_ro)
-		printk("  File system has been set read-only\n");
+		s->s_flags |= MS_RDONLY;
+
+	printk("FAT: Filesystem panic (dev %s)\n"
+	       "    %s\n", s->s_id, panic_msg);
+	if (not_ro)
+		printk("    File system has been set read-only\n");
 }
 
 
@@ -472,11 +482,13 @@
     int *number,int *ino,struct buffer_head **res_bh,struct msdos_dir_entry
     **res_de)
 {
-	int count,cluster;
+	int count, cluster;
+	unsigned long dir_size;
 
 #ifdef DEBUG
 	printk("raw_scan_nonroot: start=%d\n",start);
 #endif
+	dir_size = 0;
 	do {
 		for (count = 0; count < MSDOS_SB(sb)->cluster_size; count++) {
 			if ((cluster = raw_scan_sector(sb,(start-2)*
@@ -484,6 +496,13 @@
 			    count,name,number,ino,res_bh,res_de)) >= 0)
 				return cluster;
 		}
+		dir_size += 1 << MSDOS_SB(sb)->cluster_bits;
+		if (dir_size > FAT_MAX_DIR_SIZE) {
+			fat_fs_panic(sb, "Directory %d: "
+				     "exceeded the maximum size of directory",
+				     start);
+			break;
+		}
 		if (!(start = fat_access(sb,start,-1))) {
 			fat_fs_panic(sb,"FAT error");
 			break;
@@ -491,8 +510,8 @@
 #ifdef DEBUG
 	printk("next start: %d\n",start);
 #endif
-	}
-	while (start != -1);
+	} while (start != -1);
+
 	return -ENOENT;
 }
 
diff -urN linux-2.5.2-pre11/include/linux/msdos_fs.h fat_loop/include/linux/msdos_fs.h
--- linux-2.5.2-pre11/include/linux/msdos_fs.h	Mon Jan 14 18:35:41 2002
+++ fat_loop/include/linux/msdos_fs.h	Mon Jan 14 05:33:06 2002
@@ -19,6 +19,10 @@
 #define MSDOS_DPS_BITS	4 /* log2(MSDOS_DPS) */
 #define MSDOS_DIR_BITS	5 /* log2(sizeof(struct msdos_dir_entry)) */
 
+/* directory limit */
+#define FAT_MAX_DIR_ENTRIES	(65536)
+#define FAT_MAX_DIR_SIZE	(FAT_MAX_DIR_ENTRIES << MSDOS_DIR_BITS)
+
 #define MSDOS_SUPER_MAGIC 0x4d44 /* MD */
 
 #define FAT_CACHE    8 /* FAT cache size */
@@ -297,7 +301,7 @@
 extern int fat_notify_change(struct dentry * dentry, struct iattr * attr);
 
 /* fat/misc.c */
-extern void fat_fs_panic(struct super_block *s, const char *msg);
+extern void fat_fs_panic(struct super_block *s, const char *fmt, ...);
 extern int fat_is_binary(char conversion, char *extension);
 extern void lock_fat(struct super_block *sb);
 extern void unlock_fat(struct super_block *sb);

Re: Hard lock when mounting loopback file

[Andrew Morton]

OGAWA Hirofumi wrote:
> 
> Andrew Morton <akpm@zip.com.au> writes:
> 
> > > Additional check on the number of followed links could be useful there.
> > > No chain should be longer than the number of clusters on the fs.
> > > Although on large FAT32 filesystems the number of clusters can be high,
> > > a very long loop is still better than an infinite one.  (In cases where
> > > we know the file size, this limit can be reduced to
> > > file_size/cluster_size + 1 links).
> >
> > hmm..  OK, I'll take a look at that approach.
> 
> The maximum number of directory entries is 65536 in ordinarily.  The
> following patch prevents the infinite loop of a directory (include the
> limit to added directory).
> 
> I think the following patch is a bit useful.

Ah lovely.  I was looking for your email address when this came up,
but couldn't find it.

Here's the 2.4.18-pre3 version of your patch.  I've tested it against
the corrupted filesystem image and it works fine:

Jan 14 02:40:50 mnm kernel: FAT: Filesystem panic (dev 07:00)
Jan 14 02:40:50 mnm kernel:     Directory 3329: exceeded the maximum size of directory
Jan 14 02:40:50 mnm kernel:     File system has been set read-only
Jan 14 02:40:50 mnm kernel: FAT: Filesystem panic (dev 07:00)
Jan 14 02:40:50 mnm kernel:     Directory 5: exceeded the maximum size of directory

If you're satisfied that everything is OK, I'd suggest that you
send the diff to Marcelo.

Thanks.



--- linux-2.4.18-pre3/fs/fat/dir.c	Fri Oct 12 13:48:42 2001
+++ linux-akpm/fs/fat/dir.c	Mon Jan 14 02:31:49 2002
@@ -727,7 +727,13 @@ int fat_add_entries(struct inode *dir,in
 	offset = curr = 0;
 	*bh = NULL;
 	row = 0;
-	while (fat_get_entry(dir,&curr,bh,de,ino) > -1) {
+	while (fat_get_entry(dir, &curr, bh, de, ino) > -1) {
+		/* check the maximum size of directory */
+		if (curr >= FAT_MAX_DIR_SIZE) {
+			fat_brelse(sb, *bh);
+			return -ENOSPC;
+		}
+
 		if (IS_FREE((*de)->name)) {
 			if (++row == slots)
 				return offset;
@@ -742,7 +748,10 @@ int fat_add_entries(struct inode *dir,in
 	if (!new_bh)
 		return -ENOSPC;
 	fat_brelse(sb, new_bh);
-	do fat_get_entry(dir,&curr,bh,de,ino); while (++row<slots);
+	do {
+		fat_get_entry(dir, &curr, bh, de, ino);
+	} while (++row < slots);
+
 	return offset;
 }
 
--- linux-2.4.18-pre3/fs/fat/inode.c	Thu Nov 22 23:02:58 2001
+++ linux-akpm/fs/fat/inode.c	Mon Jan 14 02:31:49 2002
@@ -372,11 +372,37 @@ out:
 	return ret;
 }
 
+static void fat_calc_dir_size(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+	int nr;
+
+	inode->i_size = 0;
+	if (MSDOS_I(inode)->i_start == 0)
+		return;
+
+	nr = MSDOS_I(inode)->i_start;
+	do {
+		inode->i_size += 1 << MSDOS_SB(sb)->cluster_bits;
+		if (!(nr = fat_access(sb, nr, -1))) {
+			printk("FAT: Directory %ld: bad FAT\n",
+			       inode->i_ino);
+			break;
+		}
+		if (inode->i_size > FAT_MAX_DIR_SIZE) {
+			fat_fs_panic(sb, "Directory %ld: "
+				     "exceeded the maximum size of directory",
+				     inode->i_ino);
+			inode->i_size = FAT_MAX_DIR_SIZE;
+			break;
+		}
+	} while (nr != -1);
+}
+
 static void fat_read_root(struct inode *inode)
 {
 	struct super_block *sb = inode->i_sb;
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
-	int nr;
 
 	INIT_LIST_HEAD(&MSDOS_I(inode)->i_fat_hash);
 	MSDOS_I(inode)->i_location = 0;
@@ -390,16 +416,7 @@ static void fat_read_root(struct inode *
 	inode->i_fop = &fat_dir_operations;
 	if (sbi->fat_bits == 32) {
 		MSDOS_I(inode)->i_start = sbi->root_cluster;
-		if ((nr = MSDOS_I(inode)->i_start) != 0) {
-			while (nr != -1) {
-				inode->i_size += 1 << sbi->cluster_bits;
-				if (!(nr = fat_access(sb, nr, -1))) {
-					printk("Directory %ld: bad FAT\n",
-					       inode->i_ino);
-					break;
-				}
-			}
-		}
+		fat_calc_dir_size(inode);
 	} else {
 		MSDOS_I(inode)->i_start = 0;
 		inode->i_size = sbi->dir_entries * sizeof(struct msdos_dir_entry);
@@ -885,7 +902,6 @@ static void fat_fill_inode(struct inode 
 {
 	struct super_block *sb = inode->i_sb;
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
-	int nr;
 
 	INIT_LIST_HEAD(&MSDOS_I(inode)->i_fat_hash);
 	MSDOS_I(inode)->i_location = 0;
@@ -916,15 +932,7 @@ static void fat_fill_inode(struct inode 
 			inode->i_nlink = 1;
 		}
 #endif
-		if ((nr = MSDOS_I(inode)->i_start) != 0)
-			while (nr != -1) {
-				inode->i_size += 1 << sbi->cluster_bits;
-				if (!(nr = fat_access(sb, nr, -1))) {
-					printk("Directory %ld: bad FAT\n",
-					    inode->i_ino);
-					break;
-				}
-			}
+		fat_calc_dir_size(inode);
 		MSDOS_I(inode)->mmu_private = inode->i_size;
 	} else { /* not a directory */
 		inode->i_generation |= 1;
--- linux-2.4.18-pre3/fs/fat/misc.c	Fri Oct 12 13:48:42 2001
+++ linux-akpm/fs/fat/misc.c	Mon Jan 14 02:33:45 2002
@@ -38,15 +38,24 @@ static char ascii_extensions[] =
  * read-only. The file system can be made writable again by remounting it.
  */
 
-void fat_fs_panic(struct super_block *s,const char *msg)
+static char panic_msg[512];
+
+void fat_fs_panic(struct super_block *s,const char *fmt, ...)
 {
 	int not_ro;
+	va_list args;
+
+	va_start (args, fmt);
+	vsnprintf (panic_msg, sizeof(panic_msg), fmt, args);
+	va_end (args);
 
 	not_ro = !(s->s_flags & MS_RDONLY);
-	if (not_ro) s->s_flags |= MS_RDONLY;
-	printk("Filesystem panic (dev %s).\n  %s\n", kdevname(s->s_dev), msg);
 	if (not_ro)
-		printk("  File system has been set read-only\n");
+		s->s_flags |= MS_RDONLY;
+	printk("FAT: Filesystem panic (dev %s)\n"
+	       "    %s\n", kdevname(s->s_dev), panic_msg);
+	if (not_ro)
+		printk("    File system has been set read-only\n");
 }
 
 
@@ -472,11 +481,13 @@ static int raw_scan_nonroot(struct super
     int *number,int *ino,struct buffer_head **res_bh,struct msdos_dir_entry
     **res_de)
 {
-	int count,cluster;
+	int count, cluster;
+	unsigned long dir_size;
 
 #ifdef DEBUG
 	printk("raw_scan_nonroot: start=%d\n",start);
 #endif
+	dir_size = 0;
 	do {
 		for (count = 0; count < MSDOS_SB(sb)->cluster_size; count++) {
 			if ((cluster = raw_scan_sector(sb,(start-2)*
@@ -484,6 +495,13 @@ static int raw_scan_nonroot(struct super
 			    count,name,number,ino,res_bh,res_de)) >= 0)
 				return cluster;
 		}
+		dir_size += 1 << MSDOS_SB(sb)->cluster_bits;
+		if (dir_size > FAT_MAX_DIR_SIZE) {
+			fat_fs_panic(sb, "Directory %d: "
+				     "exceeded the maximum size of directory",
+				     start);
+			break;
+		}
 		if (!(start = fat_access(sb,start,-1))) {
 			fat_fs_panic(sb,"FAT error");
 			break;
@@ -491,8 +509,8 @@ static int raw_scan_nonroot(struct super
 #ifdef DEBUG
 	printk("next start: %d\n",start);
 #endif
-	}
-	while (start != -1);
+	} while (start != -1);
+
 	return -ENOENT;
 }
 
--- linux-2.4.18-pre3/include/linux/msdos_fs.h	Fri Oct 12 13:48:42 2001
+++ linux-akpm/include/linux/msdos_fs.h	Mon Jan 14 02:31:49 2002
@@ -19,6 +19,10 @@
 #define MSDOS_DPS_BITS	4 /* log2(MSDOS_DPS) */
 #define MSDOS_DIR_BITS	5 /* log2(sizeof(struct msdos_dir_entry)) */
 
+/* directory limit */
+#define FAT_MAX_DIR_ENTRIES	(65536)
+#define FAT_MAX_DIR_SIZE	(FAT_MAX_DIR_ENTRIES << MSDOS_DIR_BITS)
+
 #define MSDOS_SUPER_MAGIC 0x4d44 /* MD */
 
 #define FAT_CACHE    8 /* FAT cache size */
@@ -297,7 +301,7 @@ extern void fat_write_inode(struct inode
 extern int fat_notify_change(struct dentry * dentry, struct iattr * attr);
 
 /* fat/misc.c */
-extern void fat_fs_panic(struct super_block *s, const char *msg);
+extern void fat_fs_panic(struct super_block *s, const char *fmt, ...);
 extern int fat_is_binary(char conversion, char *extension);
 extern void lock_fat(struct super_block *sb);
 extern void unlock_fat(struct super_block *sb);

Re: Hard lock when mounting loopback file

[OGAWA Hirofumi]

Andrew Morton <akpm@zip.com.au> writes:

> OGAWA Hirofumi wrote:
> > 
> > Andrew Morton <akpm@zip.com.au> writes:
> > 
> > > > Additional check on the number of followed links could be useful there.
> > > > No chain should be longer than the number of clusters on the fs.
> > > > Although on large FAT32 filesystems the number of clusters can be high,
> > > > a very long loop is still better than an infinite one.  (In cases where
> > > > we know the file size, this limit can be reduced to
> > > > file_size/cluster_size + 1 links).
> > >
> > > hmm..  OK, I'll take a look at that approach.
> > 
> > The maximum number of directory entries is 65536 in ordinarily.  The
> > following patch prevents the infinite loop of a directory (include the
> > limit to added directory).
> > 
> > I think the following patch is a bit useful.
> 
> Ah lovely.  I was looking for your email address when this came up,
> but couldn't find it.

Sorry, that patch is first post.

> Here's the 2.4.18-pre3 version of your patch.  I've tested it against
> the corrupted filesystem image and it works fine:
> 
> Jan 14 02:40:50 mnm kernel: FAT: Filesystem panic (dev 07:00)
> Jan 14 02:40:50 mnm kernel:     Directory 3329: exceeded the maximum size of directory
> Jan 14 02:40:50 mnm kernel:     File system has been set read-only
> Jan 14 02:40:50 mnm kernel: FAT: Filesystem panic (dev 07:00)
> Jan 14 02:40:50 mnm kernel:     Directory 5: exceeded the maximum size of directory
> 
> If you're satisfied that everything is OK, I'd suggest that you
> send the diff to Marcelo.

Thanks. That patch doesn't completely prevent the infinite loop
(regular file), but I think the following patch is useful (directory).

Marcelo, please apply the following patch.

--- linux-2.4.18-pre3/fs/fat/dir.c	Fri Oct 12 13:48:42 2001
+++ linux-akpm/fs/fat/dir.c	Mon Jan 14 02:31:49 2002
@@ -727,7 +727,13 @@ int fat_add_entries(struct inode *dir,in
 	offset = curr = 0;
 	*bh = NULL;
 	row = 0;
-	while (fat_get_entry(dir,&curr,bh,de,ino) > -1) {
+	while (fat_get_entry(dir, &curr, bh, de, ino) > -1) {
+		/* check the maximum size of directory */
+		if (curr >= FAT_MAX_DIR_SIZE) {
+			fat_brelse(sb, *bh);
+			return -ENOSPC;
+		}
+
 		if (IS_FREE((*de)->name)) {
 			if (++row == slots)
 				return offset;
@@ -742,7 +748,10 @@ int fat_add_entries(struct inode *dir,in
 	if (!new_bh)
 		return -ENOSPC;
 	fat_brelse(sb, new_bh);
-	do fat_get_entry(dir,&curr,bh,de,ino); while (++row<slots);
+	do {
+		fat_get_entry(dir, &curr, bh, de, ino);
+	} while (++row < slots);
+
 	return offset;
 }
 
--- linux-2.4.18-pre3/fs/fat/inode.c	Thu Nov 22 23:02:58 2001
+++ linux-akpm/fs/fat/inode.c	Mon Jan 14 02:31:49 2002
@@ -372,11 +372,37 @@ out:
 	return ret;
 }
 
+static void fat_calc_dir_size(struct inode *inode)
+{
+	struct super_block *sb = inode->i_sb;
+	int nr;
+
+	inode->i_size = 0;
+	if (MSDOS_I(inode)->i_start == 0)
+		return;
+
+	nr = MSDOS_I(inode)->i_start;
+	do {
+		inode->i_size += 1 << MSDOS_SB(sb)->cluster_bits;
+		if (!(nr = fat_access(sb, nr, -1))) {
+			printk("FAT: Directory %ld: bad FAT\n",
+			       inode->i_ino);
+			break;
+		}
+		if (inode->i_size > FAT_MAX_DIR_SIZE) {
+			fat_fs_panic(sb, "Directory %ld: "
+				     "exceeded the maximum size of directory",
+				     inode->i_ino);
+			inode->i_size = FAT_MAX_DIR_SIZE;
+			break;
+		}
+	} while (nr != -1);
+}
+
 static void fat_read_root(struct inode *inode)
 {
 	struct super_block *sb = inode->i_sb;
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
-	int nr;
 
 	INIT_LIST_HEAD(&MSDOS_I(inode)->i_fat_hash);
 	MSDOS_I(inode)->i_location = 0;
@@ -390,16 +416,7 @@ static void fat_read_root(struct inode *
 	inode->i_fop = &fat_dir_operations;
 	if (sbi->fat_bits == 32) {
 		MSDOS_I(inode)->i_start = sbi->root_cluster;
-		if ((nr = MSDOS_I(inode)->i_start) != 0) {
-			while (nr != -1) {
-				inode->i_size += 1 << sbi->cluster_bits;
-				if (!(nr = fat_access(sb, nr, -1))) {
-					printk("Directory %ld: bad FAT\n",
-					       inode->i_ino);
-					break;
-				}
-			}
-		}
+		fat_calc_dir_size(inode);
 	} else {
 		MSDOS_I(inode)->i_start = 0;
 		inode->i_size = sbi->dir_entries * sizeof(struct msdos_dir_entry);
@@ -885,7 +902,6 @@ static void fat_fill_inode(struct inode 
 {
 	struct super_block *sb = inode->i_sb;
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
-	int nr;
 
 	INIT_LIST_HEAD(&MSDOS_I(inode)->i_fat_hash);
 	MSDOS_I(inode)->i_location = 0;
@@ -916,15 +932,7 @@ static void fat_fill_inode(struct inode 
 			inode->i_nlink = 1;
 		}
 #endif
-		if ((nr = MSDOS_I(inode)->i_start) != 0)
-			while (nr != -1) {
-				inode->i_size += 1 << sbi->cluster_bits;
-				if (!(nr = fat_access(sb, nr, -1))) {
-					printk("Directory %ld: bad FAT\n",
-					    inode->i_ino);
-					break;
-				}
-			}
+		fat_calc_dir_size(inode);
 		MSDOS_I(inode)->mmu_private = inode->i_size;
 	} else { /* not a directory */
 		inode->i_generation |= 1;
--- linux-2.4.18-pre3/fs/fat/misc.c	Fri Oct 12 13:48:42 2001
+++ linux-akpm/fs/fat/misc.c	Mon Jan 14 02:33:45 2002
@@ -38,15 +38,24 @@ static char ascii_extensions[] =
  * read-only. The file system can be made writable again by remounting it.
  */
 
-void fat_fs_panic(struct super_block *s,const char *msg)
+static char panic_msg[512];
+
+void fat_fs_panic(struct super_block *s,const char *fmt, ...)
 {
 	int not_ro;
+	va_list args;
+
+	va_start (args, fmt);
+	vsnprintf (panic_msg, sizeof(panic_msg), fmt, args);
+	va_end (args);
 
 	not_ro = !(s->s_flags & MS_RDONLY);
-	if (not_ro) s->s_flags |= MS_RDONLY;
-	printk("Filesystem panic (dev %s).\n  %s\n", kdevname(s->s_dev), msg);
 	if (not_ro)
-		printk("  File system has been set read-only\n");
+		s->s_flags |= MS_RDONLY;
+	printk("FAT: Filesystem panic (dev %s)\n"
+	       "    %s\n", kdevname(s->s_dev), panic_msg);
+	if (not_ro)
+		printk("    File system has been set read-only\n");
 }
 
 
@@ -472,11 +481,13 @@ static int raw_scan_nonroot(struct super
     int *number,int *ino,struct buffer_head **res_bh,struct msdos_dir_entry
     **res_de)
 {
-	int count,cluster;
+	int count, cluster;
+	unsigned long dir_size;
 
 #ifdef DEBUG
 	printk("raw_scan_nonroot: start=%d\n",start);
 #endif
+	dir_size = 0;
 	do {
 		for (count = 0; count < MSDOS_SB(sb)->cluster_size; count++) {
 			if ((cluster = raw_scan_sector(sb,(start-2)*
@@ -484,6 +495,13 @@ static int raw_scan_nonroot(struct super
 			    count,name,number,ino,res_bh,res_de)) >= 0)
 				return cluster;
 		}
+		dir_size += 1 << MSDOS_SB(sb)->cluster_bits;
+		if (dir_size > FAT_MAX_DIR_SIZE) {
+			fat_fs_panic(sb, "Directory %d: "
+				     "exceeded the maximum size of directory",
+				     start);
+			break;
+		}
 		if (!(start = fat_access(sb,start,-1))) {
 			fat_fs_panic(sb,"FAT error");
 			break;
@@ -491,8 +509,8 @@ static int raw_scan_nonroot(struct super
 #ifdef DEBUG
 	printk("next start: %d\n",start);
 #endif
-	}
-	while (start != -1);
+	} while (start != -1);
+
 	return -ENOENT;
 }
 
--- linux-2.4.18-pre3/include/linux/msdos_fs.h	Fri Oct 12 13:48:42 2001
+++ linux-akpm/include/linux/msdos_fs.h	Mon Jan 14 02:31:49 2002
@@ -19,6 +19,10 @@
 #define MSDOS_DPS_BITS	4 /* log2(MSDOS_DPS) */
 #define MSDOS_DIR_BITS	5 /* log2(sizeof(struct msdos_dir_entry)) */
 
+/* directory limit */
+#define FAT_MAX_DIR_ENTRIES	(65536)
+#define FAT_MAX_DIR_SIZE	(FAT_MAX_DIR_ENTRIES << MSDOS_DIR_BITS)
+
 #define MSDOS_SUPER_MAGIC 0x4d44 /* MD */
 
 #define FAT_CACHE    8 /* FAT cache size */
@@ -297,7 +301,7 @@ extern void fat_write_inode(struct inode
 extern int fat_notify_change(struct dentry * dentry, struct iattr * attr);
 
 /* fat/misc.c */
-extern void fat_fs_panic(struct super_block *s, const char *msg);
+extern void fat_fs_panic(struct super_block *s, const char *fmt, ...);
 extern int fat_is_binary(char conversion, char *extension);
 extern void lock_fat(struct super_block *sb);
 extern void unlock_fat(struct super_block *sb);
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: Hard lock when mounting loopback file

[Prof. Brand]

Andrew Morton <akpm@zip.com.au> said:

[...]

> I don't know a thing about fat layout, but it appears that it uses a
> linked list of blocks, and if that list ends up pointing back onto
> itself, the kernel goes into an infinite loop in several places chasing
> its way to the end of the list.

Right.

> The below patch fixed it for me, and I was able to mount and read
> your filesystem image.

AFAIKS, this just cures the case where the wild pointer points to the first
block of the offending file. You should perhaps count the blocks you go
through, so a file something like:

 -------+-----+
        ^     |
        |     |
        +-----+

or other strange pointer messups don't get you.

BTW, is fsck (the Linux or the Win32 variety) able to fix this?
-- 
Horst von Brand			     http://counter.li.org # 22616