LKML Archive on lore.kernel.org
 help / color / Atom feed
From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Anatoly Trosinenko <anatoly.trosinenko@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2] Re: FAT: Operating on broken FAT FS causes the write syscall to return negative number not equal to -1
Date: Mon, 16 Jul 2018 00:08:06 +0900
Message-ID: <87o9f8y1t5.fsf_-_@mail.parknet.co.jp> (raw)
In-Reply-To: <20180715143043.GM30522@ZenIV.linux.org.uk>

Al Viro <viro@ZenIV.linux.org.uk> writes:

> On Sun, Jul 15, 2018 at 11:20:06PM +0900, OGAWA Hirofumi wrote:
>> +static inline bool fat_valid_entry(struct msdos_sb_info *sbi, int entry)
>> +{
>> +	if (entry < FAT_START_ENT || sbi->max_cluster <= entry)
>> +		return false;
>> +	return true;
>> +}
>
> Pet peeve: if (...) return false; return true; instead of return !....;
>
> In this case,
> 	return entry >= FAT_START_ENT && entry < sb->max_cluster;

Fixed. Thanks.


[PATCH v2] fat: Validate ->i_start before using

On corrupted FATfs may have invalid ->i_start. To handle it, this
checks ->i_start before using, and return proper error code.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 fs/fat/cache.c  |   19 ++++++++++++-------
 fs/fat/fat.h    |    5 +++++
 fs/fat/fatent.c |    6 +++---
 3 files changed, 20 insertions(+), 10 deletions(-)

diff -puN fs/fat/cache.c~fat-validate-i_start fs/fat/cache.c
--- linux/fs/fat/cache.c~fat-validate-i_start	2018-07-15 23:03:25.167171670 +0900
+++ linux-hirofumi/fs/fat/cache.c	2018-07-15 23:59:11.978489716 +0900
@@ -225,7 +225,8 @@ static inline void cache_init(struct fat
 int fat_get_cluster(struct inode *inode, int cluster, int *fclus, int *dclus)
 {
 	struct super_block *sb = inode->i_sb;
-	const int limit = sb->s_maxbytes >> MSDOS_SB(sb)->cluster_bits;
+	struct msdos_sb_info *sbi = MSDOS_SB(sb);
+	const int limit = sb->s_maxbytes >> sbi->cluster_bits;
 	struct fat_entry fatent;
 	struct fat_cache_id cid;
 	int nr;
@@ -234,6 +235,12 @@ int fat_get_cluster(struct inode *inode,
 
 	*fclus = 0;
 	*dclus = MSDOS_I(inode)->i_start;
+	if (!fat_valid_entry(sbi, *dclus)) {
+		fat_fs_error_ratelimit(sb,
+			"%s: invalid start cluster (i_pos %lld, start %08x)",
+			__func__, MSDOS_I(inode)->i_pos, *dclus);
+		return -EIO;
+	}
 	if (cluster == 0)
 		return 0;
 
@@ -250,9 +257,8 @@ int fat_get_cluster(struct inode *inode,
 		/* prevent the infinite loop of cluster chain */
 		if (*fclus > limit) {
 			fat_fs_error_ratelimit(sb,
-					"%s: detected the cluster chain loop"
-					" (i_pos %lld)", __func__,
-					MSDOS_I(inode)->i_pos);
+				"%s: detected the cluster chain loop (i_pos %lld)",
+				__func__, MSDOS_I(inode)->i_pos);
 			nr = -EIO;
 			goto out;
 		}
@@ -262,9 +268,8 @@ int fat_get_cluster(struct inode *inode,
 			goto out;
 		else if (nr == FAT_ENT_FREE) {
 			fat_fs_error_ratelimit(sb,
-				       "%s: invalid cluster chain (i_pos %lld)",
-				       __func__,
-				       MSDOS_I(inode)->i_pos);
+				"%s: invalid cluster chain (i_pos %lld)",
+				__func__, MSDOS_I(inode)->i_pos);
 			nr = -EIO;
 			goto out;
 		} else if (nr == FAT_ENT_EOF) {
diff -puN fs/fat/fat.h~fat-validate-i_start fs/fat/fat.h
--- linux/fs/fat/fat.h~fat-validate-i_start	2018-07-15 23:03:25.168171670 +0900
+++ linux-hirofumi/fs/fat/fat.h	2018-07-15 23:59:58.896437024 +0900
@@ -348,6 +348,11 @@ static inline void fatent_brelse(struct
 	fatent->fat_inode = NULL;
 }
 
+static inline bool fat_valid_entry(struct msdos_sb_info *sbi, int entry)
+{
+	return FAT_START_ENT <= entry && entry < sbi->max_cluster;
+}
+
 extern void fat_ent_access_init(struct super_block *sb);
 extern int fat_ent_read(struct inode *inode, struct fat_entry *fatent,
 			int entry);
diff -puN fs/fat/fatent.c~fat-validate-i_start fs/fat/fatent.c
--- linux/fs/fat/fatent.c~fat-validate-i_start	2018-07-15 23:03:25.169171668 +0900
+++ linux-hirofumi/fs/fat/fatent.c	2018-07-15 23:59:12.036489650 +0900
@@ -24,7 +24,7 @@ static void fat12_ent_blocknr(struct sup
 {
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	int bytes = entry + (entry >> 1);
-	WARN_ON(entry < FAT_START_ENT || sbi->max_cluster <= entry);
+	WARN_ON(!fat_valid_entry(sbi, entry));
 	*offset = bytes & (sb->s_blocksize - 1);
 	*blocknr = sbi->fat_start + (bytes >> sb->s_blocksize_bits);
 }
@@ -34,7 +34,7 @@ static void fat_ent_blocknr(struct super
 {
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	int bytes = (entry << sbi->fatent_shift);
-	WARN_ON(entry < FAT_START_ENT || sbi->max_cluster <= entry);
+	WARN_ON(!fat_valid_entry(sbi, entry));
 	*offset = bytes & (sb->s_blocksize - 1);
 	*blocknr = sbi->fat_start + (bytes >> sb->s_blocksize_bits);
 }
@@ -354,7 +354,7 @@ int fat_ent_read(struct inode *inode, st
 	int err, offset;
 	sector_t blocknr;
 
-	if (entry < FAT_START_ENT || sbi->max_cluster <= entry) {
+	if (!fat_valid_entry(sbi, entry)) {
 		fatent_brelse(fatent);
 		fat_fs_error(sb, "invalid access to FAT (entry 0x%08x)", entry);
 		return -EIO;
_

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-14 12:24 Anatoly Trosinenko
2018-07-15 14:20 ` OGAWA Hirofumi
2018-07-15 14:30   ` Al Viro
2018-07-15 15:08     ` OGAWA Hirofumi [this message]
2018-07-15 15:08     ` Anatoly Trosinenko
2018-07-15 15:26       ` OGAWA Hirofumi
2018-07-15 15:35         ` Anatoly Trosinenko
2018-07-16 13:17       ` Alan Cox

Reply instructions:

You may reply publically 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=87o9f8y1t5.fsf_-_@mail.parknet.co.jp \
    --to=hirofumi@mail.parknet.co.jp \
    --cc=akpm@linux-foundation.org \
    --cc=anatoly.trosinenko@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox