Extract code that parses a Volume Recognition Sequence descriptor
(component), in preparation for calling it twice against different
locations in a block.
Change from v1:
Fix regression in debug logging of "open disc" condition
Signed-off-by: Steven J. Magnani <[email protected]>
--- a/fs/udf/super.c 2019-07-10 18:57:41.192852154 -0500
+++ b/fs/udf/super.c 2019-07-10 20:47:50.438352500 -0500
@@ -685,16 +685,62 @@ out_unlock:
return error;
}
-/* Check Volume Structure Descriptors (ECMA 167 2/9.1) */
-/* We also check any "CD-ROM Volume Descriptor Set" (ECMA 167 2/8.3.1) */
-static loff_t udf_check_vsd(struct super_block *sb)
+static int identify_vsd(const struct volStructDesc *vsd)
+{
+ int vsd_id = 0;
+
+ if (!strncmp(vsd->stdIdent, VSD_STD_ID_CD001, VSD_STD_ID_LEN)) {
+ switch (vsd->structType) {
+ case 0:
+ udf_debug("ISO9660 Boot Record found\n");
+ break;
+ case 1:
+ udf_debug("ISO9660 Primary Volume Descriptor found\n");
+ break;
+ case 2:
+ udf_debug("ISO9660 Supplementary Volume Descriptor found\n");
+ break;
+ case 3:
+ udf_debug("ISO9660 Volume Partition Descriptor found\n");
+ break;
+ case 255:
+ udf_debug("ISO9660 Volume Descriptor Set Terminator found\n");
+ break;
+ default:
+ udf_debug("ISO9660 VRS (%u) found\n", vsd->structType);
+ break;
+ }
+ } else if (!strncmp(vsd->stdIdent, VSD_STD_ID_BEA01, VSD_STD_ID_LEN))
+ vsd_id = 1;
+ else if (!strncmp(vsd->stdIdent, VSD_STD_ID_NSR02, VSD_STD_ID_LEN))
+ vsd_id = 2;
+ else if (!strncmp(vsd->stdIdent, VSD_STD_ID_NSR03, VSD_STD_ID_LEN))
+ vsd_id = 3;
+ else if (!strncmp(vsd->stdIdent, VSD_STD_ID_BOOT2, VSD_STD_ID_LEN))
+ ; /* vsd_id = 0 */
+ else if (!strncmp(vsd->stdIdent, VSD_STD_ID_CDW02, VSD_STD_ID_LEN))
+ ; /* vsd_id = 0 */
+ else {
+ /* TEA01 or invalid id : end of volume recognition area */
+ vsd_id = 255;
+ }
+
+ return vsd_id;
+}
+
+/*
+ * Check Volume Structure Descriptors (ECMA 167 2/9.1)
+ * We also check any "CD-ROM Volume Descriptor Set" (ECMA 167 2/8.3.1)
+ * @return 2 if NSR02 found, 3 if NSR03 found,
+ * -1 if first sector read error, 0 otherwise
+ */
+static int udf_check_vsd(struct super_block *sb)
{
struct volStructDesc *vsd = NULL;
loff_t sector = VSD_FIRST_SECTOR_OFFSET;
int sectorsize;
struct buffer_head *bh = NULL;
- int nsr02 = 0;
- int nsr03 = 0;
+ int nsr = 0;
struct udf_sb_info *sbi;
sbi = UDF_SB(sb);
@@ -718,71 +764,27 @@ static loff_t udf_check_vsd(struct super
* activity. This actually happened with uninitialised SSD partitions
* (all 0xFF) before the check for the limit and all valid IDs were
* added */
- for (; !nsr02 && !nsr03 && sector < VSD_MAX_SECTOR_OFFSET;
+ for (; (nsr < 2) && sector < VSD_MAX_SECTOR_OFFSET;
sector += sectorsize) {
+ int vsd_id;
+
/* Read a block */
bh = udf_tread(sb, sector >> sb->s_blocksize_bits);
if (!bh)
break;
- /* Look for ISO descriptors */
vsd = (struct volStructDesc *)(bh->b_data +
(sector & (sb->s_blocksize - 1)));
- if (!strncmp(vsd->stdIdent, VSD_STD_ID_CD001,
- VSD_STD_ID_LEN)) {
- switch (vsd->structType) {
- case 0:
- udf_debug("ISO9660 Boot Record found\n");
- break;
- case 1:
- udf_debug("ISO9660 Primary Volume Descriptor found\n");
- break;
- case 2:
- udf_debug("ISO9660 Supplementary Volume Descriptor found\n");
- break;
- case 3:
- udf_debug("ISO9660 Volume Partition Descriptor found\n");
- break;
- case 255:
- udf_debug("ISO9660 Volume Descriptor Set Terminator found\n");
- break;
- default:
- udf_debug("ISO9660 VRS (%u) found\n",
- vsd->structType);
- break;
- }
- } else if (!strncmp(vsd->stdIdent, VSD_STD_ID_BEA01,
- VSD_STD_ID_LEN))
- ; /* nothing */
- else if (!strncmp(vsd->stdIdent, VSD_STD_ID_TEA01,
- VSD_STD_ID_LEN)) {
- brelse(bh);
- break;
- } else if (!strncmp(vsd->stdIdent, VSD_STD_ID_NSR02,
- VSD_STD_ID_LEN))
- nsr02 = sector;
- else if (!strncmp(vsd->stdIdent, VSD_STD_ID_NSR03,
- VSD_STD_ID_LEN))
- nsr03 = sector;
- else if (!strncmp(vsd->stdIdent, VSD_STD_ID_BOOT2,
- VSD_STD_ID_LEN))
- ; /* nothing */
- else if (!strncmp(vsd->stdIdent, VSD_STD_ID_CDW02,
- VSD_STD_ID_LEN))
- ; /* nothing */
- else {
- /* invalid id : end of volume recognition area */
- brelse(bh);
- break;
- }
+ vsd_id = identify_vsd(vsd);
+ if (vsd_id > nsr)
+ nsr = vsd_id;
+
brelse(bh);
}
- if (nsr03)
- return nsr03;
- else if (nsr02)
- return nsr02;
+ if ((nsr >= 2) && (nsr <= 3))
+ return nsr;
else if (!bh && sector - (sbi->s_session << sb->s_blocksize_bits) ==
VSD_FIRST_SECTOR_OFFSET)
return -1;
@@ -1936,7 +1938,7 @@ static int udf_load_vrs(struct super_blo
int silent, struct kernel_lb_addr *fileset)
{
struct udf_sb_info *sbi = UDF_SB(sb);
- loff_t nsr_off;
+ int nsr = 0;
int ret;
if (!sb_set_blocksize(sb, uopt->blocksize)) {
@@ -1947,13 +1949,13 @@ static int udf_load_vrs(struct super_blo
sbi->s_last_block = uopt->lastblock;
if (!uopt->novrs) {
/* Check that it is NSR02 compliant */
- nsr_off = udf_check_vsd(sb);
- if (!nsr_off) {
+ nsr = udf_check_vsd(sb);
+ if (!nsr) {
if (!silent)
udf_warn(sb, "No VRS found\n");
return -EINVAL;
}
- if (nsr_off == -1)
+ if (nsr == -1)
udf_debug("Failed to read sector at offset %d. "
"Assuming open disc. Skipping validity "
"check\n", VSD_FIRST_SECTOR_OFFSET);
Some UDF creators (specifically Microsoft, but perhaps others) mishandle
the ECMA-167 corner case that requires descriptors within a Volume
Recognition Sequence to be placed at 4096-byte intervals on media where
the block size is 4K. Instead, the descriptors are placed at the 2048-
byte interval mandated for media with smaller blocks. This nonconformity
currently prevents Linux from recognizing the filesystem as UDF.
Modify the driver to tolerate a misformatted VRS on 4K media.
Signed-off-by: Steven J. Magnani <[email protected]>
--- a/fs/udf/super.c 2019-07-10 20:55:33.334359446 -0500
+++ b/fs/udf/super.c 2019-07-10 21:20:58.138382326 -0500
@@ -741,6 +741,7 @@ static int udf_check_vsd(struct super_bl
int sectorsize;
struct buffer_head *bh = NULL;
int nsr = 0;
+ int quirk_nsr = 0;
struct udf_sb_info *sbi;
sbi = UDF_SB(sb);
@@ -780,11 +781,27 @@ static int udf_check_vsd(struct super_bl
if (vsd_id > nsr)
nsr = vsd_id;
+ /* Special handling for improperly formatted VRS (e.g., Win10)
+ * where components are separated by 2048 bytes
+ * even though sectors are 4K
+ */
+ if ((sb->s_blocksize == 4096) && (quirk_nsr < 2)) {
+ vsd_id = identify_vsd(vsd + 1);
+ if ((nsr == 1) || (quirk_nsr == 1)) {
+ /* BEA01 has been seen, allow quirk NSR */
+ if (vsd_id > quirk_nsr)
+ quirk_nsr = vsd_id;
+ } else if (vsd_id > 3)
+ quirk_nsr = vsd_id; /* 0 -> 255 */
+ }
+
brelse(bh);
}
if ((nsr >= 2) && (nsr <= 3))
return nsr;
+ else if ((quirk_nsr >= 2) && (quirk_nsr <= 3))
+ return quirk_nsr;
else if (!bh && sector - (sbi->s_session << sb->s_blocksize_bits) ==
VSD_FIRST_SECTOR_OFFSET)
return -1;
On Thu 11-07-19 08:38:52, Steven J. Magnani wrote:
> Some UDF creators (specifically Microsoft, but perhaps others) mishandle
> the ECMA-167 corner case that requires descriptors within a Volume
> Recognition Sequence to be placed at 4096-byte intervals on media where
> the block size is 4K. Instead, the descriptors are placed at the 2048-
> byte interval mandated for media with smaller blocks. This nonconformity
> currently prevents Linux from recognizing the filesystem as UDF.
>
> Modify the driver to tolerate a misformatted VRS on 4K media.
>
> Signed-off-by: Steven J. Magnani <[email protected]>
Thanks for the patches! I've added them to my tree and somewhat simplified
the logic since we don't really care about nsr 2 vs 3 or whether we
actually saw BEA or not. Everything seems to work fine for me but I'd
appreciate if you could doublecheck - the result is pushed out to
git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git for_next
Thanks!
Honza
>
> --- a/fs/udf/super.c 2019-07-10 20:55:33.334359446 -0500
> +++ b/fs/udf/super.c 2019-07-10 21:20:58.138382326 -0500
> @@ -741,6 +741,7 @@ static int udf_check_vsd(struct super_bl
> int sectorsize;
> struct buffer_head *bh = NULL;
> int nsr = 0;
> + int quirk_nsr = 0;
> struct udf_sb_info *sbi;
>
> sbi = UDF_SB(sb);
> @@ -780,11 +781,27 @@ static int udf_check_vsd(struct super_bl
> if (vsd_id > nsr)
> nsr = vsd_id;
>
> + /* Special handling for improperly formatted VRS (e.g., Win10)
> + * where components are separated by 2048 bytes
> + * even though sectors are 4K
> + */
> + if ((sb->s_blocksize == 4096) && (quirk_nsr < 2)) {
> + vsd_id = identify_vsd(vsd + 1);
> + if ((nsr == 1) || (quirk_nsr == 1)) {
> + /* BEA01 has been seen, allow quirk NSR */
> + if (vsd_id > quirk_nsr)
> + quirk_nsr = vsd_id;
> + } else if (vsd_id > 3)
> + quirk_nsr = vsd_id; /* 0 -> 255 */
> + }
> +
> brelse(bh);
> }
>
> if ((nsr >= 2) && (nsr <= 3))
> return nsr;
> + else if ((quirk_nsr >= 2) && (quirk_nsr <= 3))
> + return quirk_nsr;
> else if (!bh && sector - (sbi->s_session << sb->s_blocksize_bits) ==
> VSD_FIRST_SECTOR_OFFSET)
> return -1;
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On 7/11/19 10:04 AM, Jan Kara wrote:
> Thanks for the patches! I've added them to my tree and somewhat simplified
> the logic since we don't really care about nsr 2 vs 3 or whether we
> actually saw BEA or not. Everything seems to work fine for me but I'd
> appreciate if you could doublecheck - the result is pushed out to
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git for_next
>
Tested-by: Steven J. Magnani <[email protected]>
The rework is more permissive than what you had suggested initially
(conditioning acceptance of a noncompliant NSR on a preceding BEA).
I had also tried to code the original so that a malformed 2048-byte
interval VRS would not be accepted. But the simplifications do make
the code easier to follow...
Thanks,
------------------------------------------------------------------------
Steven J. Magnani "I claim this network for MARS!
http://www.digidescorp.com Earthling, return my space modulator!"
#include <standard.disclaimer>
On Thursday 11 July 2019 08:38:51 Steven J. Magnani wrote:
> --- a/fs/udf/super.c 2019-07-10 18:57:41.192852154 -0500
> +++ b/fs/udf/super.c 2019-07-10 20:47:50.438352500 -0500
> @@ -685,16 +685,62 @@ out_unlock:
> return error;
> }
>
> -/* Check Volume Structure Descriptors (ECMA 167 2/9.1) */
> -/* We also check any "CD-ROM Volume Descriptor Set" (ECMA 167 2/8.3.1) */
> -static loff_t udf_check_vsd(struct super_block *sb)
> +static int identify_vsd(const struct volStructDesc *vsd)
> +{
> + int vsd_id = 0;
> +
> + if (!strncmp(vsd->stdIdent, VSD_STD_ID_CD001, VSD_STD_ID_LEN)) {
Hi! You probably want to use memcmp() instead of strncmp().
> + switch (vsd->structType) {
> + case 0:
> + udf_debug("ISO9660 Boot Record found\n");
> + break;
> + case 1:
> + udf_debug("ISO9660 Primary Volume Descriptor found\n");
> + break;
> + case 2:
> + udf_debug("ISO9660 Supplementary Volume Descriptor found\n");
> + break;
> + case 3:
> + udf_debug("ISO9660 Volume Partition Descriptor found\n");
> + break;
> + case 255:
> + udf_debug("ISO9660 Volume Descriptor Set Terminator found\n");
> + break;
> + default:
> + udf_debug("ISO9660 VRS (%u) found\n", vsd->structType);
> + break;
> + }
> + } else if (!strncmp(vsd->stdIdent, VSD_STD_ID_BEA01, VSD_STD_ID_LEN))
> + vsd_id = 1;
> + else if (!strncmp(vsd->stdIdent, VSD_STD_ID_NSR02, VSD_STD_ID_LEN))
> + vsd_id = 2;
> + else if (!strncmp(vsd->stdIdent, VSD_STD_ID_NSR03, VSD_STD_ID_LEN))
> + vsd_id = 3;
> + else if (!strncmp(vsd->stdIdent, VSD_STD_ID_BOOT2, VSD_STD_ID_LEN))
> + ; /* vsd_id = 0 */
> + else if (!strncmp(vsd->stdIdent, VSD_STD_ID_CDW02, VSD_STD_ID_LEN))
> + ; /* vsd_id = 0 */
> + else {
> + /* TEA01 or invalid id : end of volume recognition area */
> + vsd_id = 255;
> + }
> +
> + return vsd_id;
> +}
--
Pali Rohár
[email protected]
On Thu 11-07-19 10:56:52, Steve Magnani wrote:
> On 7/11/19 10:04 AM, Jan Kara wrote:
> > Thanks for the patches! I've added them to my tree and somewhat simplified
> > the logic since we don't really care about nsr 2 vs 3 or whether we
> > actually saw BEA or not. Everything seems to work fine for me but I'd
> > appreciate if you could doublecheck - the result is pushed out to
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git for_next
> >
> Tested-by: Steven J. Magnani <[email protected]>
>
> The rework is more permissive than what you had suggested initially
> (conditioning acceptance of a noncompliant NSR on a preceding BEA).
> I had also tried to code the original so that a malformed 2048-byte
> interval VRS would not be accepted. But the simplifications do make
> the code easier to follow...
Yeah, it's simpler to follow and we do this check just to see whether this
may be a valid UDF media before doing more expensive probing. So I don't
think that the code being more permissive matters. Thanks for testing!
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Thu 11-07-19 20:15:21, Pali Roh?r wrote:
> On Thursday 11 July 2019 08:38:51 Steven J. Magnani wrote:
> > --- a/fs/udf/super.c 2019-07-10 18:57:41.192852154 -0500
> > +++ b/fs/udf/super.c 2019-07-10 20:47:50.438352500 -0500
> > @@ -685,16 +685,62 @@ out_unlock:
> > return error;
> > }
> >
> > -/* Check Volume Structure Descriptors (ECMA 167 2/9.1) */
> > -/* We also check any "CD-ROM Volume Descriptor Set" (ECMA 167 2/8.3.1) */
> > -static loff_t udf_check_vsd(struct super_block *sb)
> > +static int identify_vsd(const struct volStructDesc *vsd)
> > +{
> > + int vsd_id = 0;
> > +
> > + if (!strncmp(vsd->stdIdent, VSD_STD_ID_CD001, VSD_STD_ID_LEN)) {
>
> Hi! You probably want to use memcmp() instead of strncmp().
There's no difference in functionality but I agree it makes more sense.
I'll modify the patch. Thanks for review!
Honza
>
> > + switch (vsd->structType) {
> > + case 0:
> > + udf_debug("ISO9660 Boot Record found\n");
> > + break;
> > + case 1:
> > + udf_debug("ISO9660 Primary Volume Descriptor found\n");
> > + break;
> > + case 2:
> > + udf_debug("ISO9660 Supplementary Volume Descriptor found\n");
> > + break;
> > + case 3:
> > + udf_debug("ISO9660 Volume Partition Descriptor found\n");
> > + break;
> > + case 255:
> > + udf_debug("ISO9660 Volume Descriptor Set Terminator found\n");
> > + break;
> > + default:
> > + udf_debug("ISO9660 VRS (%u) found\n", vsd->structType);
> > + break;
> > + }
> > + } else if (!strncmp(vsd->stdIdent, VSD_STD_ID_BEA01, VSD_STD_ID_LEN))
> > + vsd_id = 1;
> > + else if (!strncmp(vsd->stdIdent, VSD_STD_ID_NSR02, VSD_STD_ID_LEN))
> > + vsd_id = 2;
> > + else if (!strncmp(vsd->stdIdent, VSD_STD_ID_NSR03, VSD_STD_ID_LEN))
> > + vsd_id = 3;
> > + else if (!strncmp(vsd->stdIdent, VSD_STD_ID_BOOT2, VSD_STD_ID_LEN))
> > + ; /* vsd_id = 0 */
> > + else if (!strncmp(vsd->stdIdent, VSD_STD_ID_CDW02, VSD_STD_ID_LEN))
> > + ; /* vsd_id = 0 */
> > + else {
> > + /* TEA01 or invalid id : end of volume recognition area */
> > + vsd_id = 255;
> > + }
> > +
> > + return vsd_id;
> > +}
>
> --
> Pali Roh?r
> [email protected]
--
Jan Kara <[email protected]>
SUSE Labs, CR