2010-02-02 13:01:01

by Stefan Seyfried

Subject: [PATCH 2.4] FAT: do not continue in fat_get_block if bmap fails

From: Stefan Seyfried <[email protected]>

There is no use in continuing the write operation after fat_bmap() fails.
(This successfully killed a VFAT FS for me).
The corresponding code in 2.6 does return here as well, AFAICT.

Signed-off-by: Stefan Seyfried <[email protected]>
---
fs/fat/file.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/fat/file.c b/fs/fat/file.c
index ade1a71..75efe74 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -72,8 +72,10 @@ int fat_get_block(struct inode *inode, long iblock, struct buffer_head *bh_resul
}
MSDOS_I(inode)->mmu_private += sb->s_blocksize;
phys = fat_bmap(inode, iblock);
- if (!phys)
+ if (!phys) {
BUG();
+ return -EIO;
+ }
bh_result->b_dev = inode->i_dev;
bh_result->b_blocknr = phys;
bh_result->b_state |= (1UL << BH_Mapped);
--
1.6.4.2

2010-02-02 22:06:19

by Willy Tarreau

Subject: Re: [PATCH 2.4] FAT: do not continue in fat_get_block if bmap fails

Hello Stefan,

On Tue, Feb 02, 2010 at 02:00:35PM +0100, Stefan Seyfried wrote:
> From: Stefan Seyfried <[email protected]>
>
> There is no use in continuing the write operation after fat_bmap() fails.
> (This successfully killed a VFAT FS for me).
> The corresponding code in 2.6 does return here as well, AFAICT.

OK then that's fine, I'm merging it.

Thanks!
Willy

2010-02-03 09:15:21

by Stefan Seyfried

Subject: Re: [PATCH 2.4] FAT: do not continue in fat_get_block if bmap fails

Hi Willy,

On Tue, 2 Feb 2010 23:06:31 +0100
Willy Tarreau <[email protected]> wrote:

> Hello Stefan,
>
> On Tue, Feb 02, 2010 at 02:00:35PM +0100, Stefan Seyfried wrote:
> > From: Stefan Seyfried <[email protected]>
> >
> > There is no use in continuing the write operation after fat_bmap() fails.
> > (This successfully killed a VFAT FS for me).
> > The corresponding code in 2.6 does return here as well, AFAICT.
>
> OK then that's fine, I'm merging it.

I'd like to add that I am not a filesystem expert at all, so if
somebody wants to suggest a better return code, I'm all for that.

And the dosfs code in 2.6 is substantially different, thus the "AFAICT"
above ;)

Anyway, continuing at that place (when phys == 0) is definitely
wrong, since writing to block 0 later on will kill the filesystem 100%.

I triggered this with a corrumpted file, which an application wanted to
modify, dosfsck had this to say about the file system:

strolchi:~ # dosfsck -nv /dev/sdb1
dosfsck 2.11 (12 Mar 2005)
dosfsck 2.11, 12 Mar 2005, FAT32, LFN
Checking we can access the last sector of the filesystem
Boot sector contents:
System ID "MSDOS5.0"
Media byte 0xf8 (hard disk)
512 bytes per logical sector
16384 bytes per cluster
1 reserved sector
First FAT starts at byte 512 (sector 1)
2 FATs, 16 bit entries
124928 bytes per FAT (= 244 sectors)
Root directory starts at byte 250368 (sector 489)
512 root directory entries
Data area starts at byte 266752 (sector 521)
62283 data clusters (1020444672 bytes)
63 sectors/track, 32 heads
247 hidden sectors
1993577 sectors total
/test/test.db
File size is 188928 bytes, cluster chain length is 163840 bytes.
Truncating file to 163840 bytes.
Checking for unused clusters.
Reclaimed 2 unused clusters (32768 bytes).
Leaving file system unchanged.
/dev/sdb1: 201 files, 51608/62283 clusters

Thanks for merging and taking care of the "old lady" 2.4 ;)

Stefan
--
Stefan Seyfried

"Any ideas, John?"
"Well, surrounding them's out."