[PATCH 0/2] udf: A cleanup and a mount fix

[PATCH 0/2] udf: A cleanup and a mount fix

[Jan Kara]

Hi,

this series removes some dead code from UDF and fixes mounting of
Win7-created media. I plan to merge these fixes through my tree. Review
is welcome.

								Honza

[PATCH 1/2] udf: Remove dead code from udf_find_fileset()

[Jan Kara]

Remove dead code and slightly simplify code in udf_find_fileset().

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/super.c | 62 +---------------------------------------------------------
 1 file changed, 1 insertion(+), 61 deletions(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 3040dc2a32f6..68d57b61f3af 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -764,9 +764,7 @@ static int udf_find_fileset(struct super_block *sb,
 			    struct kernel_lb_addr *root)
 {
 	struct buffer_head *bh = NULL;
-	long lastblock;
 	uint16_t ident;
-	struct udf_sb_info *sbi;
 
 	if (fileset->logicalBlockNum != 0xFFFFFFFF ||
 	    fileset->partitionReferenceNum != 0xFFFF) {
@@ -779,69 +777,11 @@ static int udf_find_fileset(struct super_block *sb,
 			return 1;
 		}
 
-	}
-
-	sbi = UDF_SB(sb);
-	if (!bh) {
-		/* Search backwards through the partitions */
-		struct kernel_lb_addr newfileset;
-
-/* --> cvg: FIXME - is it reasonable? */
-		return 1;
-
-		for (newfileset.partitionReferenceNum = sbi->s_partitions - 1;
-		     (newfileset.partitionReferenceNum != 0xFFFF &&
-		      fileset->logicalBlockNum == 0xFFFFFFFF &&
-		      fileset->partitionReferenceNum == 0xFFFF);
-		     newfileset.partitionReferenceNum--) {
-			lastblock = sbi->s_partmaps
-					[newfileset.partitionReferenceNum]
-						.s_partition_len;
-			newfileset.logicalBlockNum = 0;
-
-			do {
-				bh = udf_read_ptagged(sb, &newfileset, 0,
-						      &ident);
-				if (!bh) {
-					newfileset.logicalBlockNum++;
-					continue;
-				}
-
-				switch (ident) {
-				case TAG_IDENT_SBD:
-				{
-					struct spaceBitmapDesc *sp;
-					sp = (struct spaceBitmapDesc *)
-								bh->b_data;
-					newfileset.logicalBlockNum += 1 +
-						((le32_to_cpu(sp->numOfBytes) +
-						  sizeof(struct spaceBitmapDesc)
-						  - 1) >> sb->s_blocksize_bits);
-					brelse(bh);
-					break;
-				}
-				case TAG_IDENT_FSD:
-					*fileset = newfileset;
-					break;
-				default:
-					newfileset.logicalBlockNum++;
-					brelse(bh);
-					bh = NULL;
-					break;
-				}
-			} while (newfileset.logicalBlockNum < lastblock &&
-				 fileset->logicalBlockNum == 0xFFFFFFFF &&
-				 fileset->partitionReferenceNum == 0xFFFF);
-		}
-	}
-
-	if ((fileset->logicalBlockNum != 0xFFFFFFFF ||
-	     fileset->partitionReferenceNum != 0xFFFF) && bh) {
 		udf_debug("Fileset at block=%u, partition=%u\n",
 			  fileset->logicalBlockNum,
 			  fileset->partitionReferenceNum);
 
-		sbi->s_partition = fileset->partitionReferenceNum;
+		UDF_SB(sb)->s_partition = fileset->partitionReferenceNum;
 		udf_load_fileset(sb, bh, root);
 		brelse(bh);
 		return 0;
-- 
2.16.4

[PATCH 2/2] udf: Fix mounting of Win7 created UDF filesystems

[Jan Kara]

Win7 is creating UDF filesystems with single partition with number 8192.
Current partition descriptor scanning code does not handle this well as
it incorrectly assumes that partition numbers will form mostly contiguous
space of small numbers. This results in unmountable media due to errors
like:

UDF-fs: error (device dm-1): udf_read_tagged: tag version 0x0000 != 0x0002 || 0x0003, block 0
UDF-fs: warning (device dm-1): udf_fill_super: No fileset found

Fix the problem by handling partition descriptors in a way that sparse
partition numbering does not matter.

Reported-by: jean-luc malet <jeanluc.malet@gmail.com>
CC: stable@vger.kernel.org
Fixes: 7b78fd02fb19530fd101ae137a1f46aa466d9bb6
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/super.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 68d57b61f3af..6f515651a2c2 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -1510,10 +1510,16 @@ static void udf_load_logicalvolint(struct super_block *sb, struct kernel_extent_
  */
 #define PART_DESC_ALLOC_STEP 32
 
+struct part_desc_seq_scan_data {
+	struct udf_vds_record rec;
+	u32 partnum;
+};
+
 struct desc_seq_scan_data {
 	struct udf_vds_record vds[VDS_POS_LENGTH];
 	unsigned int size_part_descs;
-	struct udf_vds_record *part_descs_loc;
+	unsigned int num_part_descs;
+	struct part_desc_seq_scan_data *part_descs_loc;
 };
 
 static struct udf_vds_record *handle_partition_descriptor(
@@ -1522,10 +1528,14 @@ static struct udf_vds_record *handle_partition_descriptor(
 {
 	struct partitionDesc *desc = (struct partitionDesc *)bh->b_data;
 	int partnum;
+	int i;
 
 	partnum = le16_to_cpu(desc->partitionNumber);
-	if (partnum >= data->size_part_descs) {
-		struct udf_vds_record *new_loc;
+	for (i = 0; i < data->num_part_descs; i++)
+		if (partnum == data->part_descs_loc[i].partnum)
+			return &(data->part_descs_loc[i].rec);
+	if (data->num_part_descs >= data->size_part_descs) {
+		struct part_desc_seq_scan_data *new_loc;
 		unsigned int new_size = ALIGN(partnum, PART_DESC_ALLOC_STEP);
 
 		new_loc = kcalloc(new_size, sizeof(*new_loc), GFP_KERNEL);
@@ -1537,7 +1547,7 @@ static struct udf_vds_record *handle_partition_descriptor(
 		data->part_descs_loc = new_loc;
 		data->size_part_descs = new_size;
 	}
-	return &(data->part_descs_loc[partnum]);
+	return &(data->part_descs_loc[data->num_part_descs++].rec);
 }
 
 
@@ -1587,6 +1597,7 @@ static noinline int udf_process_sequence(
 
 	memset(data.vds, 0, sizeof(struct udf_vds_record) * VDS_POS_LENGTH);
 	data.size_part_descs = PART_DESC_ALLOC_STEP;
+	data.num_part_descs = 0;
 	data.part_descs_loc = kcalloc(data.size_part_descs,
 				      sizeof(*data.part_descs_loc),
 				      GFP_KERNEL);
@@ -1598,7 +1609,6 @@ static noinline int udf_process_sequence(
 	 * are in it.
 	 */
 	for (; (!done && block <= lastblock); block++) {
-
 		bh = udf_read_tagged(sb, block, block, &ident);
 		if (!bh)
 			break;
@@ -1670,13 +1680,10 @@ static noinline int udf_process_sequence(
 	}
 
 	/* Now handle prevailing Partition Descriptors */
-	for (i = 0; i < data.size_part_descs; i++) {
-		if (data.part_descs_loc[i].block) {
-			ret = udf_load_partdesc(sb,
-						data.part_descs_loc[i].block);
-			if (ret < 0)
-				return ret;
-		}
+	for (i = 0; i < data.num_part_descs; i++) {
+		ret = udf_load_partdesc(sb, data.part_descs_loc[i].rec.block);
+		if (ret < 0)
+			return ret;
 	}
 
 	return 0;
-- 
2.16.4

Re: [PATCH 0/2] udf: A cleanup and a mount fix

[Jan Kara]

Hi,

On Wed 22-08-18 15:25:51, jean-luc malet wrote:
> I didn't had time to test the patch yet but there is an issue in udf detection
> of mount (using udf formated partition with mkfs.udf under linux)
> when I do�
> # mount /dev/sdx1 /some/mount/point�
> it gives an error, syslog show and error saying exfat marker wasn't found (the
> partion id is set to one of the windows compatible one, either exfat/ntfs or
> one of the fat32 or 0x7 as described into�https://serverfault.com/questions/
> 35243/what-is-the-partition-id-filesystem-type-for-udf : Short answer: I would
> suggest to use MBR id 0x07 for UDF partitions. so that it's detected by windows
> and have a drive letter assigned)
> but
> #mount /dev/sdx1 /some/mount/point -t udf�
> works fine
> don't know where it is relevant to file the bug...�

This is a different issue. Older versions of mkfs.udf did not wipe all
locations on disk where other filesystems (previous filesystems on the
media) could have their identifications. So mount finds exfat signature,
tries to mount the device as FAT which fails (as UDF has already
overwritten lots of its metadata). Right, Pali?

								Honza

> 
> thanks and regards
> 
> On Tue, Aug 21, 2018 at 3:54 PM Jan Kara <jack@suse.cz> wrote:
> 
> 
>     Hi,
> 
>     this series removes some dead code from UDF and fixes mounting of
>     Win7-created media. I plan to merge these fixes through my tree. Review
>     is welcome.
> 
>     � � � � � � � � � � � � � � � � � � � � � � � � � � � � � � � � Honza
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/2] udf: A cleanup and a mount fix

[Pali Rohár]

[-- Attachment #1: Type: text/plain, Size: 1713 bytes --]

On Wednesday 22 August 2018 17:24:35 Jan Kara wrote:
> Hi,
> 
> On Wed 22-08-18 15:25:51, jean-luc malet wrote:
> > I didn't had time to test the patch yet but there is an issue in udf detection
> > of mount (using udf formated partition with mkfs.udf under linux)
> > when I do 
> > # mount /dev/sdx1 /some/mount/point 
> > it gives an error, syslog show and error saying exfat marker wasn't found (the
> > partion id is set to one of the windows compatible one, either exfat/ntfs or
> > one of the fat32 or 0x7 as described into https://serverfault.com/questions/
> > 35243/what-is-the-partition-id-filesystem-type-for-udf : Short answer: I would
> > suggest to use MBR id 0x07 for UDF partitions. so that it's detected by windows
> > and have a drive letter assigned)
> > but
> > #mount /dev/sdx1 /some/mount/point -t udf 
> > works fine
> > don't know where it is relevant to file the bug... 
> 
> This is a different issue. Older versions of mkfs.udf did not wipe all
> locations on disk where other filesystems (previous filesystems on the
> media) could have their identifications. So mount finds exfat signature,
> tries to mount the device as FAT which fails (as UDF has already
> overwritten lots of its metadata). Right, Pali?

Yes, older versions of mkudffs (mkfs.udf) prior to 2.0 did not do any
cleanup. Just write new needed UDF structures to device. UDF itself does
not use first 32kB of disk, therefore any existing FAT/NTFS/ext*
signatures stay there. So ideally prior using older mkudffs erase first
10MB of disk, this should be enough to wipe all existing filesystem
signatures. Or use new version of mkudffs 2.0+.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]