Re: [PATCH 1/2] udf: Verify domain identifier fields

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jan -

Tested-by: Steven J. Magnani <steve@xxxxxxxxxxxxxxx>

Reviewed-by: Steven J. Magnani <steve@xxxxxxxxxxxxxxx>

On 8/29/19 7:25 AM, Jan Kara wrote:
OSTA UDF standard defines that domain identifier in logical volume
descriptor and file set descriptor should contain a particular string
and the identifier suffix contains flags possibly making media
write-protected. Verify these constraints and allow only read-only mount
if they are not met.

Signed-off-by: Jan Kara <jack@xxxxxxx>
---
  fs/udf/ecma_167.h | 14 +++++++++
  fs/udf/super.c    | 91 ++++++++++++++++++++++++++++++++++++++-----------------
  2 files changed, 78 insertions(+), 27 deletions(-)

diff --git a/fs/udf/ecma_167.h b/fs/udf/ecma_167.h
index 9f24bd1a9f44..fb7f2c7bec9c 100644
--- a/fs/udf/ecma_167.h
+++ b/fs/udf/ecma_167.h
@@ -88,6 +88,20 @@ struct regid {
  #define ENTITYID_FLAGS_DIRTY		0x00
  #define ENTITYID_FLAGS_PROTECTED	0x01
+/* OSTA UDF 2.1.5.2 */
+#define UDF_ID_COMPLIANT "*OSTA UDF Compliant"
+
+/* OSTA UDF 2.1.5.3 */
+struct domainEntityIDSuffix {
+	uint16_t	revision;
+	uint8_t		flags;
+	uint8_t		reserved[5];
+};
+
+/* OSTA UDF 2.1.5.3 */
+#define ENTITYIDSUFFIX_FLAGS_HARDWRITEPROTECT 0
+#define ENTITYIDSUFFIX_FLAGS_SOFTWRITEPROTECT 1
+
  /* Volume Structure Descriptor (ECMA 167r3 2/9.1) */
  #define VSD_STD_ID_LEN			5
  struct volStructDesc {
diff --git a/fs/udf/super.c b/fs/udf/super.c
index a14346137361..42db3dd51dfc 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -94,8 +94,8 @@ static int udf_remount_fs(struct super_block *, int *, char *);
  static void udf_load_logicalvolint(struct super_block *, struct kernel_extent_ad);
  static int udf_find_fileset(struct super_block *, struct kernel_lb_addr *,
  			    struct kernel_lb_addr *);
-static void udf_load_fileset(struct super_block *, struct buffer_head *,
-			     struct kernel_lb_addr *);
+static int udf_load_fileset(struct super_block *, struct fileSetDesc *,
+			    struct kernel_lb_addr *);
  static void udf_open_lvid(struct super_block *);
  static void udf_close_lvid(struct super_block *);
  static unsigned int udf_count_free(struct super_block *);
@@ -757,28 +757,27 @@ static int udf_find_fileset(struct super_block *sb,
  {
  	struct buffer_head *bh = NULL;
  	uint16_t ident;
+	int ret;
- if (fileset->logicalBlockNum != 0xFFFFFFFF ||
-	    fileset->partitionReferenceNum != 0xFFFF) {
-		bh = udf_read_ptagged(sb, fileset, 0, &ident);
-
-		if (!bh) {
-			return 1;
-		} else if (ident != TAG_IDENT_FSD) {
-			brelse(bh);
-			return 1;
-		}
-
-		udf_debug("Fileset at block=%u, partition=%u\n",
-			  fileset->logicalBlockNum,
-			  fileset->partitionReferenceNum);
+	if (fileset->logicalBlockNum == 0xFFFFFFFF &&
+	    fileset->partitionReferenceNum == 0xFFFF)
+		return -EINVAL;
- UDF_SB(sb)->s_partition = fileset->partitionReferenceNum;
-		udf_load_fileset(sb, bh, root);
+	bh = udf_read_ptagged(sb, fileset, 0, &ident);
+	if (!bh)
+		return -EIO;
+	if (ident != TAG_IDENT_FSD) {
  		brelse(bh);
-		return 0;
+		return -EINVAL;
  	}
-	return 1;
+
+	udf_debug("Fileset at block=%u, partition=%u\n",
+		  fileset->logicalBlockNum, fileset->partitionReferenceNum);
+
+	UDF_SB(sb)->s_partition = fileset->partitionReferenceNum;
+	ret = udf_load_fileset(sb, (struct fileSetDesc *)bh->b_data, root);
+	brelse(bh);
+	return ret;
  }
/*
@@ -939,19 +938,53 @@ static int udf_load_metadata_files(struct super_block *sb, int partition,
  	return 0;
  }
-static void udf_load_fileset(struct super_block *sb, struct buffer_head *bh,
-			     struct kernel_lb_addr *root)
+static int udf_verify_domain_identifier(struct super_block *sb,
+					struct regid *ident, char *dname)

The latter two can be 'const'.

  {
-	struct fileSetDesc *fset;
+	struct domainEntityIDSuffix *suffix;
- fset = (struct fileSetDesc *)bh->b_data;
+	if (memcmp(ident->ident, UDF_ID_COMPLIANT, strlen(UDF_ID_COMPLIANT))) {
+		udf_warn(sb, "Not OSTA UDF compliant %s descriptor.\n", dname);
+		goto force_ro;
+	}
+	if (ident->flags & (1 << ENTITYID_FLAGS_DIRTY)) {
+		udf_warn(sb, "Possibly not OSTA UDF compliant %s descriptor.\n",
+			 dname);
+		goto force_ro;
+	}
+	suffix = (struct domainEntityIDSuffix *)ident->identSuffix;
+	if (suffix->flags & (1 << ENTITYIDSUFFIX_FLAGS_HARDWRITEPROTECT) ||
+	    suffix->flags & (1 << ENTITYIDSUFFIX_FLAGS_SOFTWRITEPROTECT)) {
+		if (!sb_rdonly(sb)) {
+			udf_warn(sb, "Descriptor for %s marked write protected."
+				 " Forcing read only mount.\n", dname);
+		}
+		goto force_ro;
+	}
+	return 0;
- *root = lelb_to_cpu(fset->rootDirectoryICB.extLocation);
+force_ro:
+	if (!sb_rdonly(sb))
+		return -EACCES;
+	UDF_SET_FLAG(sb, UDF_FLAG_RW_INCOMPAT);
+	return 0;
+}
+static int udf_load_fileset(struct super_block *sb, struct fileSetDesc *fset,
+			    struct kernel_lb_addr *root)
+{
+	int ret;
+
+	ret = udf_verify_domain_identifier(sb, &fset->domainIdent, "file set");
+	if (ret < 0)
+		return ret;
+
+	*root = lelb_to_cpu(fset->rootDirectoryICB.extLocation);
  	UDF_SB(sb)->s_serial_number = le16_to_cpu(fset->descTag.tagSerialNum);
udf_debug("Rootdir at block=%u, partition=%u\n",
  		  root->logicalBlockNum, root->partitionReferenceNum);
+	return 0;
  }
int udf_compute_nr_groups(struct super_block *sb, u32 partition)
@@ -1364,6 +1397,10 @@ static int udf_load_logicalvol(struct super_block *sb, sector_t block,
  		goto out_bh;
  	}

FYI just a little above this there is a 'BUG_ON(ident != TAG_IDENT_LVD)'
that would probably be better coded as 'ret = -EINVAL; goto out_bh;'

+	ret = udf_verify_domain_identifier(sb, &lvd->domainIdent,
+					   "logical volume");
+	if (ret)
+		goto out_bh;
  	ret = udf_sb_alloc_partition_maps(sb, le32_to_cpu(lvd->numPartitionMaps));
  	if (ret)
  		goto out_bh;
@@ -2216,9 +2253,9 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
  		UDF_SET_FLAG(sb, UDF_FLAG_RW_INCOMPAT);
  	}
- if (udf_find_fileset(sb, &fileset, &rootdir)) {
+	ret = udf_find_fileset(sb, &fileset, &rootdir);
+	if (ret < 0) {

Consider making the "No fileset found" warning conditional on ret != -EACCES,
it's a little confusing to see this in the log:

  UDF-fs: warning (device loop0): udf_verify_domain_identifier: Descriptor for file set marked write protected. Forcing read only mount.
  UDF-fs: warning (device loop0): udf_fill_super: No fileset found

  		udf_warn(sb, "No fileset found\n");
-		ret = -EINVAL;
  		goto error_out;
  	}

------------------------------------------------------------------------
 Steven J. Magnani               "I claim this network for MARS!
 www.digidescorp.com              Earthling, return my space modulator!"

 #include <standard.disclaimer>




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux