* [PATCH 0/2] udf: Verify domain identifier
@ 2019-08-29 12:25 Jan Kara
2019-08-29 12:25 ` [PATCH 1/2] udf: Verify domain identifier fields Jan Kara
2019-08-29 12:25 ` [PATCH 2/2] udf: Drop forward function declarations Jan Kara
0 siblings, 2 replies; 5+ messages in thread
From: Jan Kara @ 2019-08-29 12:25 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Steve Magnani, Pali Rohár, Jan Kara
Hello,
These two patches make udf verify domain identifier and honor write protection
fields in domain identifier suffix for logical volume descriptor and fileset
descriptor. Steve, can you verify the patch works for the media you've spotted?
It worked fine for the images I had laying around. Thanks!
Honza
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] udf: Verify domain identifier fields
2019-08-29 12:25 [PATCH 0/2] udf: Verify domain identifier Jan Kara
@ 2019-08-29 12:25 ` Jan Kara
2019-09-04 13:12 ` Steve Magnani
2019-08-29 12:25 ` [PATCH 2/2] udf: Drop forward function declarations Jan Kara
1 sibling, 1 reply; 5+ messages in thread
From: Jan Kara @ 2019-08-29 12:25 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Steve Magnani, Pali Rohár, Jan Kara
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)
{
- 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;
}
--
2.16.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] udf: Drop forward function declarations
2019-08-29 12:25 [PATCH 0/2] udf: Verify domain identifier Jan Kara
2019-08-29 12:25 ` [PATCH 1/2] udf: Verify domain identifier fields Jan Kara
@ 2019-08-29 12:25 ` Jan Kara
1 sibling, 0 replies; 5+ messages in thread
From: Jan Kara @ 2019-08-29 12:25 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Steve Magnani, Pali Rohár, Jan Kara
Move some functions to make forward declarations unnecessary.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/super.c | 102 +++++++++++++++++++++++++++------------------------------
1 file changed, 49 insertions(+), 53 deletions(-)
diff --git a/fs/udf/super.c b/fs/udf/super.c
index 42db3dd51dfc..fa0f1d947526 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -92,10 +92,6 @@ static void udf_put_super(struct super_block *);
static int udf_sync_fs(struct super_block *, int);
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 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 *);
@@ -751,6 +747,55 @@ static loff_t udf_check_vsd(struct super_block *sb)
return 0;
}
+static int udf_verify_domain_identifier(struct super_block *sb,
+ struct regid *ident, char *dname)
+{
+ struct domainEntityIDSuffix *suffix;
+
+ 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;
+
+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;
+}
+
static int udf_find_fileset(struct super_block *sb,
struct kernel_lb_addr *fileset,
struct kernel_lb_addr *root)
@@ -938,55 +983,6 @@ static int udf_load_metadata_files(struct super_block *sb, int partition,
return 0;
}
-static int udf_verify_domain_identifier(struct super_block *sb,
- struct regid *ident, char *dname)
-{
- struct domainEntityIDSuffix *suffix;
-
- 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;
-
-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)
{
struct udf_part_map *map = &UDF_SB(sb)->s_partmaps[partition];
--
2.16.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] udf: Verify domain identifier fields
2019-08-29 12:25 ` [PATCH 1/2] udf: Verify domain identifier fields Jan Kara
@ 2019-09-04 13:12 ` Steve Magnani
2019-09-04 16:22 ` Jan Kara
0 siblings, 1 reply; 5+ messages in thread
From: Steve Magnani @ 2019-09-04 13:12 UTC (permalink / raw)
To: Jan Kara, linux-fsdevel; +Cc: Pali Rohár
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] udf: Verify domain identifier fields
2019-09-04 13:12 ` Steve Magnani
@ 2019-09-04 16:22 ` Jan Kara
0 siblings, 0 replies; 5+ messages in thread
From: Jan Kara @ 2019-09-04 16:22 UTC (permalink / raw)
To: Steve Magnani; +Cc: Jan Kara, linux-fsdevel, Pali Rohár
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>
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-04 16:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-29 12:25 [PATCH 0/2] udf: Verify domain identifier Jan Kara
2019-08-29 12:25 ` [PATCH 1/2] udf: Verify domain identifier fields Jan Kara
2019-09-04 13:12 ` Steve Magnani
2019-09-04 16:22 ` Jan Kara
2019-08-29 12:25 ` [PATCH 2/2] udf: Drop forward function declarations Jan Kara
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.