| Message ID | 20190829122543.22805-2-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | udf: Verify domain identifier | expand |
Jan - Tested-by: Steven J. Magnani <steve@digidescorp.com> Reviewed-by: Steven J. Magnani <steve@digidescorp.com> 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@suse.cz> > --- > 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>
On Wed 04-09-19 08:12:50, Steve Magnani wrote: > Jan - > > Tested-by: Steven J. Magnani <steve@digidescorp.com> > > Reviewed-by: Steven J. Magnani <steve@digidescorp.com> Thanks! Honza > > 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@suse.cz> > > --- > > 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> >
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) { - 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; } + 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) { udf_warn(sb, "No fileset found\n"); - ret = -EINVAL; goto error_out; }
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@suse.cz> --- fs/udf/ecma_167.h | 14 +++++++++ fs/udf/super.c | 91 ++++++++++++++++++++++++++++++++++++++----------------- 2 files changed, 78 insertions(+), 27 deletions(-)