linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] udf: Fix crash during mount
@ 2018-09-06 16:18 Jan Kara
  2018-09-06 16:18 ` [PATCH 1/4] udf: Prevent write-unsupported filesystem to be remounted read-write Jan Kara
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Jan Kara @ 2018-09-06 16:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Anatoly Trosinenko, Jan Kara

Hello,

this series fixes a crash during mount of a corrupted UDF image. It also
somewhat cleans up the surrounding code when already touching it.

								Honza

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/4] udf: Prevent write-unsupported filesystem to be remounted read-write
  2018-09-06 16:18 [PATCH 0/4] udf: Fix crash during mount Jan Kara
@ 2018-09-06 16:18 ` Jan Kara
  2018-09-06 16:18 ` [PATCH 2/4] udf: Fix crash during mount Jan Kara
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Jan Kara @ 2018-09-06 16:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Anatoly Trosinenko, Jan Kara

There are certain filesystem features which we support for reading but
not for writing. We properly refuse to mount such filesystems read-write
however for some features (such as read-only partitions), we don't check
for these features when remounting the filesystem from read-only to
read-write. Thus such filesystems could be remounted read-write leading
to strange behavior (most likely crashes).

Fix the problem by marking in superblock whether the filesystem has some
features that are supported in read-only mode and check this flag during
remount.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/super.c  | 30 ++++++++++++++++--------------
 fs/udf/udf_sb.h |  2 ++
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 6f515651a2c2..b997e3116e37 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -613,14 +613,11 @@ static int udf_remount_fs(struct super_block *sb, int *flags, char *options)
 	struct udf_options uopt;
 	struct udf_sb_info *sbi = UDF_SB(sb);
 	int error = 0;
-	struct logicalVolIntegrityDescImpUse *lvidiu = udf_sb_lvidiu(sb);
+
+	if (!(*flags & SB_RDONLY) && UDF_QUERY_FLAG(sb, UDF_FLAG_RW_INCOMPAT))
+		return -EACCES;
 
 	sync_filesystem(sb);
-	if (lvidiu) {
-		int write_rev = le16_to_cpu(lvidiu->minUDFWriteRev);
-		if (write_rev > UDF_MAX_WRITE_VERSION && !(*flags & SB_RDONLY))
-			return -EACCES;
-	}
 
 	uopt.flags = sbi->s_flags;
 	uopt.uid   = sbi->s_uid;
@@ -1257,6 +1254,7 @@ static int udf_load_partdesc(struct super_block *sb, sector_t block)
 			ret = -EACCES;
 			goto out_bh;
 		}
+		UDF_SET_FLAG(sb, UDF_FLAG_RW_INCOMPAT);
 		ret = udf_load_vat(sb, i, type1_idx);
 		if (ret < 0)
 			goto out_bh;
@@ -2155,10 +2153,12 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
 				UDF_MAX_READ_VERSION);
 			ret = -EINVAL;
 			goto error_out;
-		} else if (minUDFWriteRev > UDF_MAX_WRITE_VERSION &&
-			   !sb_rdonly(sb)) {
-			ret = -EACCES;
-			goto error_out;
+		} else if (minUDFWriteRev > UDF_MAX_WRITE_VERSION) {
+			if (!sb_rdonly(sb)) {
+				ret = -EACCES;
+				goto error_out;
+			}
+			UDF_SET_FLAG(sb, UDF_FLAG_RW_INCOMPAT);
 		}
 
 		sbi->s_udfrev = minUDFWriteRev;
@@ -2176,10 +2176,12 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
 	}
 
 	if (sbi->s_partmaps[sbi->s_partition].s_partition_flags &
-			UDF_PART_FLAG_READ_ONLY &&
-	    !sb_rdonly(sb)) {
-		ret = -EACCES;
-		goto error_out;
+			UDF_PART_FLAG_READ_ONLY) {
+		if (!sb_rdonly(sb)) {
+			ret = -EACCES;
+			goto error_out;
+		}
+		UDF_SET_FLAG(sb, UDF_FLAG_RW_INCOMPAT);
 	}
 
 	if (udf_find_fileset(sb, &fileset, &rootdir)) {
diff --git a/fs/udf/udf_sb.h b/fs/udf/udf_sb.h
index 9424d7cab790..d12e507e9eb2 100644
--- a/fs/udf/udf_sb.h
+++ b/fs/udf/udf_sb.h
@@ -30,6 +30,8 @@
 #define UDF_FLAG_LASTBLOCK_SET	16
 #define UDF_FLAG_BLOCKSIZE_SET	17
 #define UDF_FLAG_INCONSISTENT	18
+#define UDF_FLAG_RW_INCOMPAT	19	/* Set when we find RW incompatible
+					 * feature */
 
 #define UDF_PART_FLAG_UNALLOC_BITMAP	0x0001
 #define UDF_PART_FLAG_UNALLOC_TABLE	0x0002
-- 
2.16.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/4] udf: Fix crash during mount
  2018-09-06 16:18 [PATCH 0/4] udf: Fix crash during mount Jan Kara
  2018-09-06 16:18 ` [PATCH 1/4] udf: Prevent write-unsupported filesystem to be remounted read-write Jan Kara
@ 2018-09-06 16:18 ` Jan Kara
  2018-09-06 16:18 ` [PATCH 3/4] udf: Drop freed bitmap / table support Jan Kara
  2018-09-06 16:18 ` [PATCH 4/4] udf: Drop pack pragma from udf_sb.h Jan Kara
  3 siblings, 0 replies; 5+ messages in thread
From: Jan Kara @ 2018-09-06 16:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Anatoly Trosinenko, Jan Kara

Fix a crash during an attempt to mount a filesystem that has both
Unallocated Space Table and Unallocated Space Bitmap. Such filesystem
actually violates the UDF standard so we just have to properly detect
such situation and refuse to mount such filesystem read-write. When we
are at it, verify also other constraints on the allocation information
mandated by the standard.

Reported-by: Anatoly Trosinenko <anatoly.trosinenko@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/super.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 5 deletions(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index b997e3116e37..be934bcfa24b 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -985,12 +985,62 @@ static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
 	return bitmap;
 }
 
+static int check_partition_desc(struct super_block *sb,
+				struct partitionDesc *p,
+				struct udf_part_map *map)
+{
+	bool umap, utable, fmap, ftable;
+	struct partitionHeaderDesc *phd;
+
+	switch (le32_to_cpu(p->accessType)) {
+	case PD_ACCESS_TYPE_READ_ONLY:
+	case PD_ACCESS_TYPE_WRITE_ONCE:
+	case PD_ACCESS_TYPE_REWRITABLE:
+	case PD_ACCESS_TYPE_NONE:
+		goto force_ro;
+	}
+
+	/* No Partition Header Descriptor? */
+	if (strcmp(p->partitionContents.ident, PD_PARTITION_CONTENTS_NSR02) &&
+	    strcmp(p->partitionContents.ident, PD_PARTITION_CONTENTS_NSR03))
+		goto force_ro;
+
+	phd = (struct partitionHeaderDesc *)p->partitionContentsUse;
+	utable = phd->unallocSpaceTable.extLength;
+	umap = phd->unallocSpaceBitmap.extLength;
+	ftable = phd->freedSpaceTable.extLength;
+	fmap = phd->freedSpaceBitmap.extLength;
+
+	/* No allocation info? */
+	if (!utable && !umap && !ftable && !fmap)
+		goto force_ro;
+
+	/* We don't support blocks that require erasing before overwrite */
+	if (ftable || fmap)
+		goto force_ro;
+	/* UDF 2.60: 2.3.3 - no mixing of tables & bitmaps, no VAT. */
+	if (utable && umap)
+		goto force_ro;
+
+	if (map->s_partition_type == UDF_VIRTUAL_MAP15 ||
+	    map->s_partition_type == UDF_VIRTUAL_MAP20)
+		goto force_ro;
+
+	return 0;
+force_ro:
+	if (!sb_rdonly(sb))
+		return -EACCES;
+	UDF_SET_FLAG(sb, UDF_FLAG_RW_INCOMPAT);
+	return 0;
+}
+
 static int udf_fill_partdesc_info(struct super_block *sb,
 		struct partitionDesc *p, int p_index)
 {
 	struct udf_part_map *map;
 	struct udf_sb_info *sbi = UDF_SB(sb);
 	struct partitionHeaderDesc *phd;
+	int err;
 
 	map = &sbi->s_partmaps[p_index];
 
@@ -1010,8 +1060,16 @@ static int udf_fill_partdesc_info(struct super_block *sb,
 		  p_index, map->s_partition_type,
 		  map->s_partition_root, map->s_partition_len);
 
-	if (strcmp(p->partitionContents.ident, PD_PARTITION_CONTENTS_NSR02) &&
-	    strcmp(p->partitionContents.ident, PD_PARTITION_CONTENTS_NSR03))
+	err = check_partition_desc(sb, p, map);
+	if (err)
+		return err;
+
+	/*
+	 * Skip loading allocation info it we cannot ever write to the fs.
+	 * This is a correctness thing as we may have decided to force ro mount
+	 * to avoid allocation info we don't support.
+	 */
+	if (UDF_QUERY_FLAG(sb, UDF_FLAG_RW_INCOMPAT))
 		return 0;
 
 	phd = (struct partitionHeaderDesc *)p->partitionContentsUse;
@@ -1047,9 +1105,6 @@ static int udf_fill_partdesc_info(struct super_block *sb,
 			  p_index, bitmap->s_extPosition);
 	}
 
-	if (phd->partitionIntegrityTable.extLength)
-		udf_debug("partitionIntegrityTable (part %d)\n", p_index);
-
 	if (phd->freedSpaceTable.extLength) {
 		struct kernel_lb_addr loc = {
 			.logicalBlockNum = le32_to_cpu(
-- 
2.16.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/4] udf: Drop freed bitmap / table support
  2018-09-06 16:18 [PATCH 0/4] udf: Fix crash during mount Jan Kara
  2018-09-06 16:18 ` [PATCH 1/4] udf: Prevent write-unsupported filesystem to be remounted read-write Jan Kara
  2018-09-06 16:18 ` [PATCH 2/4] udf: Fix crash during mount Jan Kara
@ 2018-09-06 16:18 ` Jan Kara
  2018-09-06 16:18 ` [PATCH 4/4] udf: Drop pack pragma from udf_sb.h Jan Kara
  3 siblings, 0 replies; 5+ messages in thread
From: Jan Kara @ 2018-09-06 16:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Anatoly Trosinenko, Jan Kara

We don't support Free Space Table and Free Space Bitmap as specified by
UDF standard for writing as we don't support erasing blocks before
overwriting them. Just drop the handling of these structures as
partition descriptor checking code already makes sure such filesystems
can be mounted only read-only.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/balloc.c | 24 ------------------------
 fs/udf/super.c  | 44 --------------------------------------------
 fs/udf/udf_sb.h |  6 ------
 3 files changed, 74 deletions(-)

diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
index fcda0fc97b90..377b5d596aee 100644
--- a/fs/udf/balloc.c
+++ b/fs/udf/balloc.c
@@ -652,12 +652,6 @@ void udf_free_blocks(struct super_block *sb, struct inode *inode,
 	} else if (map->s_partition_flags & UDF_PART_FLAG_UNALLOC_TABLE) {
 		udf_table_free_blocks(sb, map->s_uspace.s_table,
 				      bloc, offset, count);
-	} else if (map->s_partition_flags & UDF_PART_FLAG_FREED_BITMAP) {
-		udf_bitmap_free_blocks(sb, map->s_fspace.s_bitmap,
-				       bloc, offset, count);
-	} else if (map->s_partition_flags & UDF_PART_FLAG_FREED_TABLE) {
-		udf_table_free_blocks(sb, map->s_fspace.s_table,
-				      bloc, offset, count);
 	}
 
 	if (inode) {
@@ -684,16 +678,6 @@ inline int udf_prealloc_blocks(struct super_block *sb,
 						      map->s_uspace.s_table,
 						      partition, first_block,
 						      block_count);
-	else if (map->s_partition_flags & UDF_PART_FLAG_FREED_BITMAP)
-		allocated = udf_bitmap_prealloc_blocks(sb,
-						       map->s_fspace.s_bitmap,
-						       partition, first_block,
-						       block_count);
-	else if (map->s_partition_flags & UDF_PART_FLAG_FREED_TABLE)
-		allocated = udf_table_prealloc_blocks(sb,
-						      map->s_fspace.s_table,
-						      partition, first_block,
-						      block_count);
 	else
 		return 0;
 
@@ -717,14 +701,6 @@ inline udf_pblk_t udf_new_block(struct super_block *sb,
 		block = udf_table_new_block(sb,
 					    map->s_uspace.s_table,
 					    partition, goal, err);
-	else if (map->s_partition_flags & UDF_PART_FLAG_FREED_BITMAP)
-		block = udf_bitmap_new_block(sb,
-					     map->s_fspace.s_bitmap,
-					     partition, goal, err);
-	else if (map->s_partition_flags & UDF_PART_FLAG_FREED_TABLE)
-		block = udf_table_new_block(sb,
-					    map->s_fspace.s_table,
-					    partition, goal, err);
 	else {
 		*err = -EIO;
 		return 0;
diff --git a/fs/udf/super.c b/fs/udf/super.c
index be934bcfa24b..8f2f56d9a1bb 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -290,12 +290,8 @@ static void udf_free_partition(struct udf_part_map *map)
 
 	if (map->s_partition_flags & UDF_PART_FLAG_UNALLOC_TABLE)
 		iput(map->s_uspace.s_table);
-	if (map->s_partition_flags & UDF_PART_FLAG_FREED_TABLE)
-		iput(map->s_fspace.s_table);
 	if (map->s_partition_flags & UDF_PART_FLAG_UNALLOC_BITMAP)
 		udf_sb_free_bitmap(map->s_uspace.s_bitmap);
-	if (map->s_partition_flags & UDF_PART_FLAG_FREED_BITMAP)
-		udf_sb_free_bitmap(map->s_fspace.s_bitmap);
 	if (map->s_partition_type == UDF_SPARABLE_MAP15)
 		for (i = 0; i < 4; i++)
 			brelse(map->s_type_specific.s_sparing.s_spar_map[i]);
@@ -1105,37 +1101,6 @@ static int udf_fill_partdesc_info(struct super_block *sb,
 			  p_index, bitmap->s_extPosition);
 	}
 
-	if (phd->freedSpaceTable.extLength) {
-		struct kernel_lb_addr loc = {
-			.logicalBlockNum = le32_to_cpu(
-				phd->freedSpaceTable.extPosition),
-			.partitionReferenceNum = p_index,
-		};
-		struct inode *inode;
-
-		inode = udf_iget_special(sb, &loc);
-		if (IS_ERR(inode)) {
-			udf_debug("cannot load freedSpaceTable (part %d)\n",
-				  p_index);
-			return PTR_ERR(inode);
-		}
-		map->s_fspace.s_table = inode;
-		map->s_partition_flags |= UDF_PART_FLAG_FREED_TABLE;
-		udf_debug("freedSpaceTable (part %d) @ %lu\n",
-			  p_index, map->s_fspace.s_table->i_ino);
-	}
-
-	if (phd->freedSpaceBitmap.extLength) {
-		struct udf_bitmap *bitmap = udf_sb_alloc_bitmap(sb, p_index);
-		if (!bitmap)
-			return -ENOMEM;
-		map->s_fspace.s_bitmap = bitmap;
-		bitmap->s_extPosition = le32_to_cpu(
-				phd->freedSpaceBitmap.extPosition);
-		map->s_partition_flags |= UDF_PART_FLAG_FREED_BITMAP;
-		udf_debug("freedSpaceBitmap (part %d) @ %u\n",
-			  p_index, bitmap->s_extPosition);
-	}
 	return 0;
 }
 
@@ -2490,10 +2455,6 @@ static unsigned int udf_count_free(struct super_block *sb)
 		accum += udf_count_free_bitmap(sb,
 					       map->s_uspace.s_bitmap);
 	}
-	if (map->s_partition_flags & UDF_PART_FLAG_FREED_BITMAP) {
-		accum += udf_count_free_bitmap(sb,
-					       map->s_fspace.s_bitmap);
-	}
 	if (accum)
 		return accum;
 
@@ -2501,11 +2462,6 @@ static unsigned int udf_count_free(struct super_block *sb)
 		accum += udf_count_free_table(sb,
 					      map->s_uspace.s_table);
 	}
-	if (map->s_partition_flags & UDF_PART_FLAG_FREED_TABLE) {
-		accum += udf_count_free_table(sb,
-					      map->s_fspace.s_table);
-	}
-
 	return accum;
 }
 
diff --git a/fs/udf/udf_sb.h b/fs/udf/udf_sb.h
index d12e507e9eb2..937c209474cd 100644
--- a/fs/udf/udf_sb.h
+++ b/fs/udf/udf_sb.h
@@ -35,8 +35,6 @@
 
 #define UDF_PART_FLAG_UNALLOC_BITMAP	0x0001
 #define UDF_PART_FLAG_UNALLOC_TABLE	0x0002
-#define UDF_PART_FLAG_FREED_BITMAP	0x0004
-#define UDF_PART_FLAG_FREED_TABLE	0x0008
 #define UDF_PART_FLAG_READ_ONLY		0x0010
 #define UDF_PART_FLAG_WRITE_ONCE	0x0020
 #define UDF_PART_FLAG_REWRITABLE	0x0040
@@ -95,10 +93,6 @@ struct udf_part_map {
 		struct udf_bitmap	*s_bitmap;
 		struct inode		*s_table;
 	} s_uspace;
-	union {
-		struct udf_bitmap	*s_bitmap;
-		struct inode		*s_table;
-	} s_fspace;
 	__u32	s_partition_root;
 	__u32	s_partition_len;
 	__u16	s_partition_type;
-- 
2.16.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 4/4] udf: Drop pack pragma from udf_sb.h
  2018-09-06 16:18 [PATCH 0/4] udf: Fix crash during mount Jan Kara
                   ` (2 preceding siblings ...)
  2018-09-06 16:18 ` [PATCH 3/4] udf: Drop freed bitmap / table support Jan Kara
@ 2018-09-06 16:18 ` Jan Kara
  3 siblings, 0 replies; 5+ messages in thread
From: Jan Kara @ 2018-09-06 16:18 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Anatoly Trosinenko, Jan Kara

Drop pack pragma. The header file defines only in-memory structures.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/udf_sb.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/udf/udf_sb.h b/fs/udf/udf_sb.h
index 937c209474cd..3d83be54c474 100644
--- a/fs/udf/udf_sb.h
+++ b/fs/udf/udf_sb.h
@@ -50,8 +50,6 @@
 
 #define UDF_INVALID_MODE		((umode_t)-1)
 
-#pragma pack(1) /* XXX(hch): Why?  This file just defines in-core structures */
-
 #define MF_DUPLICATE_MD		0x01
 #define MF_MIRROR_FE_LOADED	0x02
 
-- 
2.16.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-09-06 20:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-06 16:18 [PATCH 0/4] udf: Fix crash during mount Jan Kara
2018-09-06 16:18 ` [PATCH 1/4] udf: Prevent write-unsupported filesystem to be remounted read-write Jan Kara
2018-09-06 16:18 ` [PATCH 2/4] udf: Fix crash during mount Jan Kara
2018-09-06 16:18 ` [PATCH 3/4] udf: Drop freed bitmap / table support Jan Kara
2018-09-06 16:18 ` [PATCH 4/4] udf: Drop pack pragma from udf_sb.h Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).