* UDF filesystem fixes for grub
@ 2010-11-02 13:10 Giuseppe Caizzone
2010-11-06 19:59 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 1 reply; 4+ messages in thread
From: Giuseppe Caizzone @ 2010-11-02 13:10 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1: Type: text/plain, Size: 4676 bytes --]
Hello,
I've made 4 small patches that improve the UDF filesystem support in
grub. With these changes, I've been able to read any regular file in
UDF filesystems created by both Linux and Windows on both optical
storage and USB sticks. Perhaps they might be useful for other grub
users?
I don't know if this list is the most appropriate place to post them,
I apologise for the noise if it isn't.
I'm attaching the patches, which are against last week's trunk, and a
simple bash test case for each.
- - - -
1/4) Support for large (> 2GiB) files
I know that grub isn't a typical consumer of huge files, but since the
fix is apparently trivial...
How to test that the patch is needed (tried with grub trunk on linux 2.6.36):
# dd if=/dev/zero of=/test.bin bs=1M count=3072
# mkfs.udf /test.bin
# mount -o loop /test.bin /mnt/m1
# dd if=/dev/urandom of=/mnt/m1/bigfile.bin bs=1M count=2560
# umount /mnt/m1
# grub-fstest /test.bin crc "(loop0)/bigfile.bin"
Result: grub will segfault instead of reading the whole file (tested
on a 32-bit machine).
The patch just changes an int and a grub_uint32_t to be grub_off_t.
2/4) Support for deleted files
On real-world, non-readonly media, deleted files are likely to appear
inside directories. These files' directory entries have a "deleted"
bit, which grub currently ignores.
Test case:
# dd if=/dev/zero of=/test.bin bs=1M count=32
# mkfs.udf /test.bin
# mount -o loop /test.bin /mnt/m1
# touch /mnt/m1/file{1,2,3,4,5}
# rm /mnt/m1/file3
# umount /mnt/m1
# grub-fstest /test.bin ls "(loop0)/"
Result: only the first two files will be listed, because grub will
"crash" when, encountering the third file's deleted directory entry,
it tries to "dereference" a zeroed field.
The patch just skips directory entries with the "deleted" bit set, in
grub_udf_iterate_dir().
3/4) Support for a generic block size
Currently grub assumes that all UDF filesystems have a logical block
size of 2048 bytes. But the UDF standard states that the block size
should match the physical sector size of the hosting device, so for
instance Windows only creates filesystems with a block size of 512
bytes on USB sticks. Under Linux, mkudffs allows for other block sizes
as well, up to 4096 bytes. None of these are currently readable by
grub.
Test case:
# dd if=/dev/zero of=/test.bin bs=1M count=32
# mkfs.udf --blocksize=512 /test.bin
# grub-fstest /test.bin ls "(loop0)/"
Result: the file system won't be recognised as UDF by grub.
The patch repeats the superblock search in grub_udf_mount() for 512,
1024, 2048 and 4096 block sizes, stopping if it finds a valid one AND
the sector offset recorded in that superblock matches the detected
superblock offset. So for instance it won't misdetect a superblock
located at sector 256 with blocksize 2048 for a superblock located at
sector 512 with blocksize 1024.
The log2(blocksize/512) is then stored in a new field called "lbshift"
in struct grub_udf_data, which gets used instead of the previous
constant GRUB_UDF_LOG2_BLKSZ, while the constant GRUB_UDF_BLKSZ gets
replaced with the lvd.bsize field already present in struct
grub_udf_data.
4/4) Support for allocation descriptor extensions
When files on an UDF file system grow and fragmentation appears, an
"allocation descriptor extension" entry can be recorded, which points
to a new block containing more allocation descriptors for the file.
Currently grub ignores these entries, and is thus unable to read such
fragmented files properly.
Test:
# dd if=/dev/zero of=/test.bin bs=1M count=32
# mkfs.udf --blocksize=512 /test.bin
# mount -o loop /test.bin /mnt/m1
# for ((i=0;i<8192;i++)) ; do touch "/mnt/m1/file$i" ; done
# umount /mnt/m1
# grub-fstest /test.bin ls "(loop0)/"
Result: only the first 4820 files appear (because the directory itself
gets fragmented, and when grub reaches the jump between the two
fragments, it erroneously tries to read the allocation extension
descriptor as the directory's body).
The patch checks, in grub_udf_read_block(), whether the "allocation
descriptor type", previously ignored, is 3, and in this case, it loads
the referenced block, checks that it's a valid allocation extension
descriptor, and sets the "ad" pointer (and the "len" limit) to
continue the iteration from the allocation descriptors contained in
that block.
This last one is the only patch which actually contains a proper new
block of code, and it allocates a big structure on the stack (it's a
sector, so it's up to 4K big): I don't know if this is OK, or if
perhaps I should use grub_malloc() instead. Also, the patch depends on
the previous "generic block size" patch.
Greetings,
-- Giuseppe Caizzone <acaizzo@gmail.com>
[-- Attachment #2: 1-udf-large-files.patch --]
[-- Type: application/octet-stream, Size: 731 bytes --]
diff -Naur grub/grub-core/fs/udf.c grub-test/grub-core/fs/udf.c
--- grub/grub-core/fs/udf.c 2010-05-06 17:30:36.000000000 +0200
+++ grub-test/grub-core/fs/udf.c 2010-10-27 10:18:08.160000033 +0200
@@ -471,7 +471,7 @@
void NESTED_FUNC_ATTR
(*read_hook) (grub_disk_addr_t sector,
unsigned offset, unsigned length),
- int pos, grub_size_t len, char *buf)
+ grub_off_t pos, grub_size_t len, char *buf)
{
switch (U16 (node->fe.icbtag.flags) & GRUB_UDF_ICBTAG_FLAG_AD_MASK)
{
@@ -704,7 +704,7 @@
{
grub_fshelp_node_t child;
struct grub_udf_file_ident dirent;
- grub_uint32_t offset = 0;
+ grub_off_t offset = 0;
child = grub_malloc (sizeof (struct grub_fshelp_node));
if (!child)
[-- Attachment #3: 2-udf-deleted-files.patch --]
[-- Type: application/octet-stream, Size: 3228 bytes --]
diff -Naur grub/grub-core/fs/udf.c grub-test/grub-core/fs/udf.c
--- grub/grub-core/fs/udf.c 2010-05-06 17:30:36.000000000 +0200
+++ grub-test/grub-core/fs/udf.c 2010-10-27 10:19:27.728000018 +0200
@@ -729,57 +729,60 @@
return 0;
}
- child = grub_malloc (sizeof (struct grub_fshelp_node));
- if (!child)
- return 0;
-
- if (grub_udf_read_icb (dir->data, &dirent.icb, child))
- return 0;
-
offset += sizeof (dirent) + U16 (dirent.imp_use_length);
- if (dirent.characteristics & GRUB_UDF_FID_CHAR_PARENT)
- {
- /* This is the parent directory. */
- if (hook ("..", GRUB_FSHELP_DIR, child))
- return 1;
- }
- else
+ if (!(dirent.characteristics & GRUB_UDF_FID_CHAR_DELETED))
{
- enum grub_fshelp_filetype type;
- grub_uint8_t raw[dirent.file_ident_length];
- grub_uint16_t utf16[dirent.file_ident_length - 1];
- grub_uint8_t filename[dirent.file_ident_length * 2];
- grub_size_t utf16len = 0;
-
- type = ((dirent.characteristics & GRUB_UDF_FID_CHAR_DIRECTORY) ?
- (GRUB_FSHELP_DIR) : (GRUB_FSHELP_REG));
-
- if ((grub_udf_read_file (dir, 0, offset,
- dirent.file_ident_length,
- (char *) raw))
- != dirent.file_ident_length)
+ child = grub_malloc (sizeof (struct grub_fshelp_node));
+ if (!child)
return 0;
- if (raw[0] == 8)
- {
- unsigned i;
- utf16len = dirent.file_ident_length - 1;
- for (i = 0; i < utf16len; i++)
- utf16[i] = raw[i + 1];
- }
- if (raw[0] == 16)
+ if (grub_udf_read_icb (dir->data, &dirent.icb, child))
+ return 0;
+
+ if (dirent.characteristics & GRUB_UDF_FID_CHAR_PARENT)
{
- unsigned i;
- utf16len = (dirent.file_ident_length - 1) / 2;
- for (i = 0; i < utf16len; i++)
- utf16[i] = (raw[2 * i + 1] << 8) | raw[2*i + 2];
+ /* This is the parent directory. */
+ if (hook ("..", GRUB_FSHELP_DIR, child))
+ return 1;
}
- if (raw[0] == 8 || raw[0] == 16)
+ else
{
- *grub_utf16_to_utf8 (filename, utf16, utf16len) = '\0';
-
- if (hook ((char *) filename, type, child))
- return 1;
+ enum grub_fshelp_filetype type;
+ grub_uint8_t raw[dirent.file_ident_length];
+ grub_uint16_t utf16[dirent.file_ident_length - 1];
+ grub_uint8_t filename[dirent.file_ident_length * 2];
+ grub_size_t utf16len = 0;
+
+ type = ((dirent.characteristics & GRUB_UDF_FID_CHAR_DIRECTORY) ?
+ (GRUB_FSHELP_DIR) : (GRUB_FSHELP_REG));
+
+ if ((grub_udf_read_file (dir, 0, offset,
+ dirent.file_ident_length,
+ (char *) raw))
+ != dirent.file_ident_length)
+ return 0;
+
+ if (raw[0] == 8)
+ {
+ unsigned i;
+ utf16len = dirent.file_ident_length - 1;
+ for (i = 0; i < utf16len; i++)
+ utf16[i] = raw[i + 1];
+ }
+ if (raw[0] == 16)
+ {
+ unsigned i;
+ utf16len = (dirent.file_ident_length - 1) / 2;
+ for (i = 0; i < utf16len; i++)
+ utf16[i] = (raw[2 * i + 1] << 8) | raw[2*i + 2];
+ }
+ if (raw[0] == 8 || raw[0] == 16)
+ {
+ *grub_utf16_to_utf8 (filename, utf16, utf16len) = '\0';
+
+ if (hook ((char *) filename, type, child))
+ return 1;
+ }
}
}
[-- Attachment #4: 3-udf-generic-block-size.patch --]
[-- Type: application/octet-stream, Size: 5537 bytes --]
diff -Naur grub/grub-core/fs/udf.c grub-test/grub-core/fs/udf.c
--- grub/grub-core/fs/udf.c 2010-05-06 17:30:36.000000000 +0200
+++ grub-test/grub-core/fs/udf.c 2010-10-27 10:20:56.403999341 +0200
@@ -343,7 +343,7 @@
struct grub_udf_pd pds[GRUB_UDF_MAX_PDS];
struct grub_udf_partmap *pms[GRUB_UDF_MAX_PMS];
struct grub_udf_long_ad root_icb;
- int npd, npm;
+ int npd, npm, lbshift;
};
struct grub_fshelp_node
@@ -389,7 +389,7 @@
if (grub_errno)
return grub_errno;
- if (grub_disk_read (data->disk, block << GRUB_UDF_LOG2_BLKSZ, 0,
+ if (grub_disk_read (data->disk, block << data->lbshift, 0,
sizeof (struct grub_udf_file_entry),
&node->fe))
return grub_errno;
@@ -427,7 +427,7 @@
struct grub_udf_short_ad *ad = (struct grub_udf_short_ad *) ptr;
len /= sizeof (struct grub_udf_short_ad);
- filebytes = fileblock * GRUB_UDF_BLKSZ;
+ filebytes = fileblock * U32 (node->data->lvd.bsize);
while (len > 0)
{
if (filebytes < U32 (ad->length))
@@ -435,7 +435,8 @@
(grub_udf_get_block (node->data,
node->part_ref,
ad->position)
- + (filebytes / GRUB_UDF_BLKSZ)));
+ + (filebytes >> (GRUB_DISK_SECTOR_BITS
+ + node->data->lbshift))));
filebytes -= U32 (ad->length);
ad++;
@@ -447,7 +448,7 @@
struct grub_udf_long_ad *ad = (struct grub_udf_long_ad *) ptr;
len /= sizeof (struct grub_udf_long_ad);
- filebytes = fileblock * GRUB_UDF_BLKSZ;
+ filebytes = fileblock * U32 (node->data->lvd.bsize);
while (len > 0)
{
if (filebytes < U32 (ad->length))
@@ -455,7 +456,8 @@
(grub_udf_get_block (node->data,
ad->block.part_ref,
ad->block.block_num)
- + (filebytes / GRUB_UDF_BLKSZ)));
+ + (filebytes >> (GRUB_DISK_SECTOR_BITS
+ + node->data->lbshift))));
filebytes -= U32 (ad->length);
ad++;
@@ -496,21 +498,21 @@
}
return grub_fshelp_read_file (node->data->disk, node, read_hook,
- pos, len, buf, grub_udf_read_block,
- U64 (node->fe.file_size),
- GRUB_UDF_LOG2_BLKSZ);
+ pos, len, buf, grub_udf_read_block,
+ U64 (node->fe.file_size),
+ node->data->lbshift);
}
-static int sblocklist[] = { 256, 512, 0 };
+static unsigned sblocklist[] = { 256, 512, 0 };
static struct grub_udf_data *
grub_udf_mount (grub_disk_t disk)
{
struct grub_udf_data *data = 0;
struct grub_udf_fileset root_fs;
- int *sblklist = sblocklist;
+ unsigned *sblklist;
grub_uint32_t block;
- int i;
+ int i, lbshift;
data = grub_malloc (sizeof (struct grub_udf_data));
if (!data)
@@ -545,31 +547,46 @@
}
}
- /* Search for Anchor Volume Descriptor Pointer (AVDP). */
- while (1)
- {
- struct grub_udf_avdp avdp;
+ /* Search for Anchor Volume Descriptor Pointer (AVDP)
+ * and determine logical block size. */
+ lbshift = 0;
+ block = 0;
+ while (lbshift <= 3)
+ {
+ sblklist = sblocklist;
+ while (*sblklist)
+ {
+ struct grub_udf_avdp avdp;
- if (grub_disk_read (disk, *sblklist << GRUB_UDF_LOG2_BLKSZ, 0,
- sizeof (struct grub_udf_avdp), &avdp))
- {
- grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");
- goto fail;
- }
+ if (grub_disk_read (disk, *sblklist << lbshift, 0,
+ sizeof (struct grub_udf_avdp), &avdp))
+ {
+ grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");
+ goto fail;
+ }
- if (U16 (avdp.tag.tag_ident) == GRUB_UDF_TAG_IDENT_AVDP)
- {
- block = U32 (avdp.vds.start);
- break;
- }
+ if (U16 (avdp.tag.tag_ident) == GRUB_UDF_TAG_IDENT_AVDP &&
+ U32 (avdp.tag.tag_location) == *sblklist)
+ {
+ block = U32 (avdp.vds.start);
+ break;
+ }
- sblklist++;
- if (*sblklist == 0)
- {
- grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");
- goto fail;
+ sblklist++;
}
+
+ if (block)
+ break;
+
+ lbshift++;
+ }
+
+ if (!block)
+ {
+ grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");
+ goto fail;
}
+ data->lbshift = lbshift;
data->npd = data->npm = 0;
/* Locate Partition Descriptor (PD) and Logical Volume Descriptor (LVD). */
@@ -577,7 +594,7 @@
{
struct grub_udf_tag tag;
- if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0,
+ if (grub_disk_read (disk, block << lbshift, 0,
sizeof (struct grub_udf_tag), &tag))
{
grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");
@@ -593,7 +610,7 @@
goto fail;
}
- if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0,
+ if (grub_disk_read (disk, block << lbshift, 0,
sizeof (struct grub_udf_pd),
&data->pds[data->npd]))
{
@@ -609,7 +626,7 @@
struct grub_udf_partmap *ppm;
- if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0,
+ if (grub_disk_read (disk, block << lbshift, 0,
sizeof (struct grub_udf_lvd),
&data->lvd))
{
@@ -673,7 +690,7 @@
if (grub_errno)
goto fail;
- if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0,
+ if (grub_disk_read (disk, block << lbshift, 0,
sizeof (struct grub_udf_fileset), &root_fs))
{
grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");
[-- Attachment #5: 4-udf-ad-extents.patch --]
[-- Type: application/octet-stream, Size: 4452 bytes --]
diff -Naur grub-old/grub-core/fs/udf.c grub-new/grub-core/fs/udf.c
--- grub-old/grub-core/fs/udf.c 2010-10-28 17:14:51.001977447 +0200
+++ grub-new/grub-core/fs/udf.c 2010-10-28 16:54:11.079219549 +0200
@@ -336,6 +336,13 @@
grub_uint8_t part_maps[1608];
} __attribute__ ((packed));
+struct grub_udf_aed
+{
+ struct grub_udf_tag tag;
+ grub_uint32_t prev_ae;
+ grub_uint32_t ae_len;
+} __attribute__ ((packed));
+
struct grub_udf_data
{
grub_disk_t disk;
@@ -406,8 +413,9 @@
static grub_disk_addr_t
grub_udf_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
{
+ char buf[U32 (node->data->lvd.bsize)];
char *ptr;
- int len;
+ grub_ssize_t len;
grub_disk_addr_t filebytes;
if (U16 (node->fe.tag.tag_ident) == GRUB_UDF_TAG_IDENT_FE)
@@ -415,22 +423,51 @@
ptr = (char *) &node->fe.ext_attr[0] + U32 (node->fe.ext_attr_length);
len = U32 (node->fe.alloc_descs_length);
}
- else
+ else if (U16 (node->fe.tag.tag_ident == GRUB_UDF_TAG_IDENT_EFE))
{
ptr = (char *) &node->efe.ext_attr[0] + U32 (node->efe.ext_attr_length);
len = U32 (node->efe.alloc_descs_length);
}
+ else
+ {
+ grub_error (GRUB_ERR_BAD_FS, "invalid file entry");
+ return 0;
+ }
if ((U16 (node->fe.icbtag.flags) & GRUB_UDF_ICBTAG_FLAG_AD_MASK)
== GRUB_UDF_ICBTAG_FLAG_AD_SHORT)
{
struct grub_udf_short_ad *ad = (struct grub_udf_short_ad *) ptr;
- len /= sizeof (struct grub_udf_short_ad);
filebytes = fileblock * U32 (node->data->lvd.bsize);
- while (len > 0)
+ while (len >= (grub_ssize_t) sizeof (struct grub_udf_short_ad))
{
- if (filebytes < U32 (ad->length))
+ grub_uint32_t adlen = U32 (ad->length) & 0x3fffffff;
+ grub_uint32_t adtype = U32 (ad->length) >> 30;
+ if (adtype == 3)
+ {
+ struct grub_udf_aed *extension;
+ grub_disk_addr_t sec = grub_udf_get_block(node->data,
+ node->part_ref,
+ ad->position);
+ if (grub_disk_read (node->data->disk, sec << node->data->lbshift,
+ 0, adlen, buf))
+ return 0;
+
+ extension = (struct grub_udf_aed *) buf;
+ if (U16 (extension->tag.tag_ident) != GRUB_UDF_TAG_IDENT_AED)
+ {
+ grub_error (GRUB_ERR_BAD_FS, "invalid aed tag");
+ return 0;
+ }
+
+ len = U32 (extension->ae_len);
+ ad = (struct grub_udf_short_ad *)
+ (buf + sizeof (struct grub_udf_aed));
+ continue;
+ }
+
+ if (filebytes < adlen)
return ((U32 (ad->position) & GRUB_UDF_EXT_MASK) ? 0 :
(grub_udf_get_block (node->data,
node->part_ref,
@@ -438,20 +475,44 @@
+ (filebytes >> (GRUB_DISK_SECTOR_BITS
+ node->data->lbshift))));
- filebytes -= U32 (ad->length);
+ filebytes -= adlen;
ad++;
- len--;
+ len -= sizeof (struct grub_udf_short_ad);
}
}
else
{
struct grub_udf_long_ad *ad = (struct grub_udf_long_ad *) ptr;
- len /= sizeof (struct grub_udf_long_ad);
filebytes = fileblock * U32 (node->data->lvd.bsize);
- while (len > 0)
+ while (len >= (grub_ssize_t) sizeof (struct grub_udf_long_ad))
{
- if (filebytes < U32 (ad->length))
+ grub_uint32_t adlen = U32 (ad->length) & 0x3fffffff;
+ grub_uint32_t adtype = U32 (ad->length) >> 30;
+ if (adtype == 3)
+ {
+ struct grub_udf_aed *extension;
+ grub_disk_addr_t sec = grub_udf_get_block(node->data,
+ ad->block.part_ref,
+ ad->block.block_num);
+ if (grub_disk_read (node->data->disk, sec << node->data->lbshift,
+ 0, adlen, buf))
+ return 0;
+
+ extension = (struct grub_udf_aed *) buf;
+ if (U16 (extension->tag.tag_ident) != GRUB_UDF_TAG_IDENT_AED)
+ {
+ grub_error (GRUB_ERR_BAD_FS, "invalid aed tag");
+ return 0;
+ }
+
+ len = U32 (extension->ae_len);
+ ad = (struct grub_udf_long_ad *)
+ (buf + sizeof (struct grub_udf_aed));
+ continue;
+ }
+
+ if (filebytes < adlen)
return ((U32 (ad->block.block_num) & GRUB_UDF_EXT_MASK) ? 0 :
(grub_udf_get_block (node->data,
ad->block.part_ref,
@@ -459,9 +520,9 @@
+ (filebytes >> (GRUB_DISK_SECTOR_BITS
+ node->data->lbshift))));
- filebytes -= U32 (ad->length);
+ filebytes -= adlen;
ad++;
- len--;
+ len -= sizeof (struct grub_udf_long_ad);
}
}
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: UDF filesystem fixes for grub 2010-11-02 13:10 UDF filesystem fixes for grub Giuseppe Caizzone @ 2010-11-06 19:59 ` Vladimir 'φ-coder/phcoder' Serbinenko 2010-11-11 8:56 ` Giuseppe Caizzone 0 siblings, 1 reply; 4+ messages in thread From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-11-06 19:59 UTC (permalink / raw) To: grub-devel [-- Attachment #1: Type: text/plain, Size: 6145 bytes --] On 11/02/2010 02:10 PM, Giuseppe Caizzone wrote: > Hello, > > I've made 4 small patches that improve the UDF filesystem support in > grub. With these changes, I've been able to read any regular file in > UDF filesystems created by both Linux and Windows on both optical > storage and USB sticks. Perhaps they might be useful for other grub > users? > I don't know if this list is the most appropriate place to post them, > I apologise for the noise if it isn't. > > I'm attaching the patches, which are against last week's trunk, and a > simple bash test case for each. > > - - - - > > 1/4) Support for large (> 2GiB) files > > I know that grub isn't a typical consumer of huge files, but since the > fix is apparently trivial... > > How to test that the patch is needed (tried with grub trunk on linux 2.6.36): > > # dd if=/dev/zero of=/test.bin bs=1M count=3072 > # mkfs.udf /test.bin > # mount -o loop /test.bin /mnt/m1 > # dd if=/dev/urandom of=/mnt/m1/bigfile.bin bs=1M count=2560 > # umount /mnt/m1 > # grub-fstest /test.bin crc "(loop0)/bigfile.bin" > > Result: grub will segfault instead of reading the whole file (tested > on a 32-bit machine). > The patch just changes an int and a grub_uint32_t to be grub_off_t. > > Good patch. Can you write the ChangeLog entry for it? > 2/4) Support for deleted files > > On real-world, non-readonly media, deleted files are likely to appear > inside directories. These files' directory entries have a "deleted" > bit, which grub currently ignores. > > Test case: > > # dd if=/dev/zero of=/test.bin bs=1M count=32 > # mkfs.udf /test.bin > # mount -o loop /test.bin /mnt/m1 > # touch /mnt/m1/file{1,2,3,4,5} > # rm /mnt/m1/file3 > # umount /mnt/m1 > # grub-fstest /test.bin ls "(loop0)/" > > Result: only the first two files will be listed, because grub will > "crash" when, encountering the third file's deleted directory entry, > it tries to "dereference" a zeroed field. > The patch just skips directory entries with the "deleted" bit set, in > grub_udf_iterate_dir(). > > This one is good too. Changelog entry? > 3/4) Support for a generic block size > > Currently grub assumes that all UDF filesystems have a logical block > size of 2048 bytes. But the UDF standard states that the block size > should match the physical sector size of the hosting device, so for > instance Windows only creates filesystems with a block size of 512 > bytes on USB sticks. Under Linux, mkudffs allows for other block sizes > as well, up to 4096 bytes. None of these are currently readable by > grub. > > Test case: > > # dd if=/dev/zero of=/test.bin bs=1M count=32 > # mkfs.udf --blocksize=512 /test.bin > # grub-fstest /test.bin ls "(loop0)/" > > Result: the file system won't be recognised as UDF by grub. > The patch repeats the superblock search in grub_udf_mount() for 512, > 1024, 2048 and 4096 block sizes, stopping if it finds a valid one AND > the sector offset recorded in that superblock matches the detected > superblock offset. So for instance it won't misdetect a superblock > located at sector 256 with blocksize 2048 for a superblock located at > sector 512 with blocksize 1024. > The log2(blocksize/512) is then stored in a new field called "lbshift" > in struct grub_udf_data, which gets used instead of the previous > constant GRUB_UDF_LOG2_BLKSZ, while the constant GRUB_UDF_BLKSZ gets > replaced with the lvd.bsize field already present in struct > grub_udf_data. > > + lbshift = 0; + block = 0; + while (lbshift <= 3) + { For loop is way more appropriate than a while loop + while (lbshift <= 3) + { + sblklist = sblocklist; + while (*sblklist) + { Same here. ChangeLog entry? > 4/4) Support for allocation descriptor extensions > > When files on an UDF file system grow and fragmentation appears, an > "allocation descriptor extension" entry can be recorded, which points > to a new block containing more allocation descriptors for the file. > Currently grub ignores these entries, and is thus unable to read such > fragmented files properly. > > Test: > > # dd if=/dev/zero of=/test.bin bs=1M count=32 > # mkfs.udf --blocksize=512 /test.bin > # mount -o loop /test.bin /mnt/m1 > # for ((i=0;i<8192;i++)) ; do touch "/mnt/m1/file$i" ; done > # umount /mnt/m1 > # grub-fstest /test.bin ls "(loop0)/" > > Result: only the first 4820 files appear (because the directory itself > gets fragmented, and when grub reaches the jump between the two > fragments, it erroneously tries to read the allocation extension > descriptor as the directory's body). > The patch checks, in grub_udf_read_block(), whether the "allocation > descriptor type", previously ignored, is 3, and in this case, it loads > the referenced block, checks that it's a valid allocation extension > descriptor, and sets the "ad" pointer (and the "len" limit) to > continue the iteration from the allocation descriptors contained in > that block. > > This last one is the only patch which actually contains a proper new > block of code, and it allocates a big structure on the stack (it's a > sector, so it's up to 4K big): I don't know if this is OK, or if > perhaps I should use grub_malloc() instead. Also, the patch depends on > the previous "generic block size" patch. > > + char buf[U32 (node->data->lvd.bsize)]; Will segfault if bsize is too big. You need to allocate on heap - else + else if (U16 (node->fe.tag.tag_ident == GRUB_UDF_TAG_IDENT_EFE)) { Parenthesis are wrong. + grub_disk_addr_t sec = grub_udf_get_block(node->data, + node->part_ref, + ad->position); ad->position isn't byte-swapped. ChangeLog entry? > Greetings, > -- Giuseppe Caizzone <acaizzo@gmail.com> > > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > -- Regards Vladimir 'φ-coder/phcoder' Serbinenko [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 294 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: UDF filesystem fixes for grub 2010-11-06 19:59 ` Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-11-11 8:56 ` Giuseppe Caizzone 2010-11-14 16:05 ` Vladimir 'φ-coder/phcoder' Serbinenko 0 siblings, 1 reply; 4+ messages in thread From: Giuseppe Caizzone @ 2010-11-11 8:56 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 6368 bytes --] 2010/11/6 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>: > On 11/02/2010 02:10 PM, Giuseppe Caizzone wrote: >> Hello, >> >> I've made 4 small patches that improve the UDF filesystem support in >> grub. With these changes, I've been able to read any regular file in >> UDF filesystems created by both Linux and Windows on both optical >> storage and USB sticks. >> [...] >> 1/4) Support for large (> 2GiB) files >> [...] >> The patch just changes an int and a grub_uint32_t to be grub_off_t. >> >> > Good patch. Can you write the ChangeLog entry for it? OK - I've never written one, so I tried to mimic grub's. Support reading files larger than 2 GiB. * grub-core/fs/udf.c (grub_udf_iterate_dir): change type of variable "offset" from grub_uint32_t to grub_off_t. (grub_udf_read_file): change type of parameter "pos" from int to grub_off_t. >> 2/4) Support for deleted files >> [...] >> The patch just skips directory entries with the "deleted" bit set, in >> grub_udf_iterate_dir(). >> >> > This one is good too. Changelog entry? Properly handle deleted files. * grub-core/fs/udf.c (grub_udf_iterate_dir): Skip directory entries whose "characteristics" field has the bit GRUB_UDF_FID_CHAR_DELETED set. >> 3/4) Support for a generic block size >> [...] >> The patch repeats the superblock search in grub_udf_mount() for 512, >> 1024, 2048 and 4096 block sizes, stopping if it finds a valid one AND >> the sector offset recorded in that superblock matches the detected >> superblock offset. So for instance it won't misdetect a superblock >> located at sector 256 with blocksize 2048 for a superblock located at >> sector 512 with blocksize 1024. >> The log2(blocksize/512) is then stored in a new field called "lbshift" >> in struct grub_udf_data, which gets used instead of the previous >> constant GRUB_UDF_LOG2_BLKSZ, while the constant GRUB_UDF_BLKSZ gets >> replaced with the lvd.bsize field already present in struct >> grub_udf_data. >> >> > + lbshift = 0; > + block = 0; > + while (lbshift <= 3) > + { > For loop is way more appropriate than a while loop Changed it into a for loop. > + while (lbshift <= 3) > + { > + sblklist = sblocklist; > + while (*sblklist) > + { > Same here. Changed that too. I also made another change since the previous version of the patch: I moved the VRS check after the AVDP search, because it needs to know the logical block size. I originally thought that it wasn't necessary, because the VRS is made up of records with a fixed size of 2048 bytes, but the catch is that the standard says that each record must start on a new sector, so if block size is > 2048, one has to skip more bytes than 2048 to get to the next record. Tested it with mkudffs because I have no medium with an actual block size of 4096. > ChangeLog entry? Add generic logical block size support. * grub-core/fs/udf.c (GRUB_UDF_LOG2_BLKSIZE): Removed macro. (GRUB_UDF_BLKSZ): Removed macro. (struct grub_udf_data): New field "lbshift" to hold the logical block size of the file system in log2 format. (grub_udf_read_icb): Replace usage of macro GRUB_UDF_LOG2_BLKSZ with field lbshift from struct grub_udf_data. (grub_udf_read_file): Likewise. (grub_udf_read_block): Replace usage of macro GRUB_UDF_BLKSZ with field "lvd.bsize" from struct grub_udf_data. Replace division with right shift. (sblocklist): Change type to unsigned. (grub_udf_mount): Change type of "sblklist" to unsigned. New variable "vblock" to be used during VRS recognition. New variable "lbshift" to hold the detected logical block size. Move AVDP search before VRS recognition, because the latter requires knowledge of the logical block size, which is detected during the former. Detect and validate logical block size during AVDP search, adding support for block sizes 512, 1024 and 4096. Make VRS recognition independent of block size. Replace usages of macro GRUB_UDF_LOG2_BLKSZ with variable lbshift. >> 4/4) Support for allocation descriptor extensions >> [...] >> The patch checks, in grub_udf_read_block(), whether the "allocation >> descriptor type", previously ignored, is 3, and in this case, it loads >> the referenced block, checks that it's a valid allocation extension >> descriptor, and sets the "ad" pointer (and the "len" limit) to >> continue the iteration from the allocation descriptors contained in >> that block. >> >> This last one is the only patch which actually contains a proper new >> block of code, and it allocates a big structure on the stack (it's a >> sector, so it's up to 4K big): I don't know if this is OK, or if >> perhaps I should use grub_malloc() instead. Also, the patch depends on >> the previous "generic block size" patch. >> >> > + char buf[U32 (node->data->lvd.bsize)]; > Will segfault if bsize is too big. You need to allocate on heap Done. > - else > + else if (U16 (node->fe.tag.tag_ident == GRUB_UDF_TAG_IDENT_EFE)) > { > Parenthesis are wrong. While I was at it, I changed the 'if-else if' to a switch. > + grub_disk_addr_t sec = grub_udf_get_block(node->data, > + node->part_ref, > + ad->position); > ad->position isn't byte-swapped. grub_udf_get_block() byte-swaps it itself, so I must not byte-swap it before. > ChangeLog entry? Add support for allocation extent descriptors, needed on fragmented volumes. * grub-core/fs/udf.c (grub_udf_aed): New struct. (grub_udf_read_block): Change type of variable "len" to grub_ssize_t. Add sanity check for file entry type. Replace divisions with right shifts. Check the upper bits of the allocation descriptor's length and honour them by loading an allocation extent descriptor if they indicate so. Change all failure return paths to free the allocation extent descriptor if it was allocated. I'm attaching the full patch set (including the unchanged ones for convenience), and also the change log entries in attachment form in case gmail tampers with whitespace. Thank you, -- Giuseppe Caizzone <acaizzo@gmail.com> [-- Attachment #2: 1-udf-large-files.patch --] [-- Type: application/octet-stream, Size: 731 bytes --] diff -Naur grub/grub-core/fs/udf.c grub-test/grub-core/fs/udf.c --- grub/grub-core/fs/udf.c 2010-05-06 17:30:36.000000000 +0200 +++ grub-test/grub-core/fs/udf.c 2010-10-27 10:18:08.160000033 +0200 @@ -471,7 +471,7 @@ void NESTED_FUNC_ATTR (*read_hook) (grub_disk_addr_t sector, unsigned offset, unsigned length), - int pos, grub_size_t len, char *buf) + grub_off_t pos, grub_size_t len, char *buf) { switch (U16 (node->fe.icbtag.flags) & GRUB_UDF_ICBTAG_FLAG_AD_MASK) { @@ -704,7 +704,7 @@ { grub_fshelp_node_t child; struct grub_udf_file_ident dirent; - grub_uint32_t offset = 0; + grub_off_t offset = 0; child = grub_malloc (sizeof (struct grub_fshelp_node)); if (!child) [-- Attachment #3: 1-udf-large-files.changelog --] [-- Type: application/octet-stream, Size: 236 bytes --] Support reading files larger than 2 GiB. * grub-core/fs/udf.c (grub_udf_iterate_dir): change type of variable "offset" from grub_uint32_t to grub_off_t. (grub_udf_read_file): change type of parameter "pos" from int to grub_off_t. [-- Attachment #4: 2-udf-deleted-files.patch --] [-- Type: application/octet-stream, Size: 3228 bytes --] diff -Naur grub/grub-core/fs/udf.c grub-test/grub-core/fs/udf.c --- grub/grub-core/fs/udf.c 2010-05-06 17:30:36.000000000 +0200 +++ grub-test/grub-core/fs/udf.c 2010-10-27 10:19:27.728000018 +0200 @@ -729,57 +729,60 @@ return 0; } - child = grub_malloc (sizeof (struct grub_fshelp_node)); - if (!child) - return 0; - - if (grub_udf_read_icb (dir->data, &dirent.icb, child)) - return 0; - offset += sizeof (dirent) + U16 (dirent.imp_use_length); - if (dirent.characteristics & GRUB_UDF_FID_CHAR_PARENT) - { - /* This is the parent directory. */ - if (hook ("..", GRUB_FSHELP_DIR, child)) - return 1; - } - else + if (!(dirent.characteristics & GRUB_UDF_FID_CHAR_DELETED)) { - enum grub_fshelp_filetype type; - grub_uint8_t raw[dirent.file_ident_length]; - grub_uint16_t utf16[dirent.file_ident_length - 1]; - grub_uint8_t filename[dirent.file_ident_length * 2]; - grub_size_t utf16len = 0; - - type = ((dirent.characteristics & GRUB_UDF_FID_CHAR_DIRECTORY) ? - (GRUB_FSHELP_DIR) : (GRUB_FSHELP_REG)); - - if ((grub_udf_read_file (dir, 0, offset, - dirent.file_ident_length, - (char *) raw)) - != dirent.file_ident_length) + child = grub_malloc (sizeof (struct grub_fshelp_node)); + if (!child) return 0; - if (raw[0] == 8) - { - unsigned i; - utf16len = dirent.file_ident_length - 1; - for (i = 0; i < utf16len; i++) - utf16[i] = raw[i + 1]; - } - if (raw[0] == 16) + if (grub_udf_read_icb (dir->data, &dirent.icb, child)) + return 0; + + if (dirent.characteristics & GRUB_UDF_FID_CHAR_PARENT) { - unsigned i; - utf16len = (dirent.file_ident_length - 1) / 2; - for (i = 0; i < utf16len; i++) - utf16[i] = (raw[2 * i + 1] << 8) | raw[2*i + 2]; + /* This is the parent directory. */ + if (hook ("..", GRUB_FSHELP_DIR, child)) + return 1; } - if (raw[0] == 8 || raw[0] == 16) + else { - *grub_utf16_to_utf8 (filename, utf16, utf16len) = '\0'; - - if (hook ((char *) filename, type, child)) - return 1; + enum grub_fshelp_filetype type; + grub_uint8_t raw[dirent.file_ident_length]; + grub_uint16_t utf16[dirent.file_ident_length - 1]; + grub_uint8_t filename[dirent.file_ident_length * 2]; + grub_size_t utf16len = 0; + + type = ((dirent.characteristics & GRUB_UDF_FID_CHAR_DIRECTORY) ? + (GRUB_FSHELP_DIR) : (GRUB_FSHELP_REG)); + + if ((grub_udf_read_file (dir, 0, offset, + dirent.file_ident_length, + (char *) raw)) + != dirent.file_ident_length) + return 0; + + if (raw[0] == 8) + { + unsigned i; + utf16len = dirent.file_ident_length - 1; + for (i = 0; i < utf16len; i++) + utf16[i] = raw[i + 1]; + } + if (raw[0] == 16) + { + unsigned i; + utf16len = (dirent.file_ident_length - 1) / 2; + for (i = 0; i < utf16len; i++) + utf16[i] = (raw[2 * i + 1] << 8) | raw[2*i + 2]; + } + if (raw[0] == 8 || raw[0] == 16) + { + *grub_utf16_to_utf8 (filename, utf16, utf16len) = '\0'; + + if (hook ((char *) filename, type, child)) + return 1; + } } } [-- Attachment #5: 2-udf-deleted-files.changelog --] [-- Type: application/octet-stream, Size: 177 bytes --] Properly handle deleted files. * grub-core/fs/udf.c (grub_udf_iterate_dir): Skip directory entries whose "characteristics" field has the bit GRUB_UDF_FID_CHAR_DELETED set. [-- Attachment #6: 3-udf-generic-block-size-v2.patch --] [-- Type: application/octet-stream, Size: 6299 bytes --] --- grub-old/grub-core/fs/udf.c 2010-11-07 09:19:09.732728000 +0100 +++ grub-new/grub-core/fs/udf.c 2010-11-08 14:30:01.588347621 +0100 @@ -34,9 +34,6 @@ #define U32 grub_le_to_cpu32 #define U64 grub_le_to_cpu64 -#define GRUB_UDF_LOG2_BLKSZ 2 -#define GRUB_UDF_BLKSZ 2048 - #define GRUB_UDF_TAG_IDENT_PVD 0x0001 #define GRUB_UDF_TAG_IDENT_AVDP 0x0002 #define GRUB_UDF_TAG_IDENT_VDP 0x0003 @@ -343,7 +340,7 @@ struct grub_udf_pd pds[GRUB_UDF_MAX_PDS]; struct grub_udf_partmap *pms[GRUB_UDF_MAX_PMS]; struct grub_udf_long_ad root_icb; - int npd, npm; + int npd, npm, lbshift; }; struct grub_fshelp_node @@ -389,7 +386,7 @@ if (grub_errno) return grub_errno; - if (grub_disk_read (data->disk, block << GRUB_UDF_LOG2_BLKSZ, 0, + if (grub_disk_read (data->disk, block << data->lbshift, 0, sizeof (struct grub_udf_file_entry), &node->fe)) return grub_errno; @@ -427,7 +424,7 @@ struct grub_udf_short_ad *ad = (struct grub_udf_short_ad *) ptr; len /= sizeof (struct grub_udf_short_ad); - filebytes = fileblock * GRUB_UDF_BLKSZ; + filebytes = fileblock * U32 (node->data->lvd.bsize); while (len > 0) { if (filebytes < U32 (ad->length)) @@ -435,7 +432,8 @@ (grub_udf_get_block (node->data, node->part_ref, ad->position) - + (filebytes / GRUB_UDF_BLKSZ))); + + (filebytes >> (GRUB_DISK_SECTOR_BITS + + node->data->lbshift)))); filebytes -= U32 (ad->length); ad++; @@ -447,7 +445,7 @@ struct grub_udf_long_ad *ad = (struct grub_udf_long_ad *) ptr; len /= sizeof (struct grub_udf_long_ad); - filebytes = fileblock * GRUB_UDF_BLKSZ; + filebytes = fileblock * U32 (node->data->lvd.bsize); while (len > 0) { if (filebytes < U32 (ad->length)) @@ -455,7 +453,8 @@ (grub_udf_get_block (node->data, ad->block.part_ref, ad->block.block_num) - + (filebytes / GRUB_UDF_BLKSZ))); + + (filebytes >> (GRUB_DISK_SECTOR_BITS + + node->data->lbshift)))); filebytes -= U32 (ad->length); ad++; @@ -496,21 +495,21 @@ } return grub_fshelp_read_file (node->data->disk, node, read_hook, - pos, len, buf, grub_udf_read_block, - U64 (node->fe.file_size), - GRUB_UDF_LOG2_BLKSZ); + pos, len, buf, grub_udf_read_block, + U64 (node->fe.file_size), + node->data->lbshift); } -static int sblocklist[] = { 256, 512, 0 }; +static unsigned sblocklist[] = { 256, 512, 0 }; static struct grub_udf_data * grub_udf_mount (grub_disk_t disk) { struct grub_udf_data *data = 0; struct grub_udf_fileset root_fs; - int *sblklist = sblocklist; - grub_uint32_t block; - int i; + unsigned *sblklist; + grub_uint32_t block, vblock; + int i, lbshift; data = grub_malloc (sizeof (struct grub_udf_data)); if (!data) @@ -518,12 +517,48 @@ data->disk = disk; + /* Search for Anchor Volume Descriptor Pointer (AVDP) + * and determine logical block size. */ + block = 0; + for (lbshift = 0; lbshift < 4; lbshift++) + { + for (sblklist = sblocklist; *sblklist; sblklist++) + { + struct grub_udf_avdp avdp; + + if (grub_disk_read (disk, *sblklist << lbshift, 0, + sizeof (struct grub_udf_avdp), &avdp)) + { + grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem"); + goto fail; + } + + if (U16 (avdp.tag.tag_ident) == GRUB_UDF_TAG_IDENT_AVDP && + U32 (avdp.tag.tag_location) == *sblklist) + { + block = U32 (avdp.vds.start); + break; + } + } + + if (block) + break; + } + + if (!block) + { + grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem"); + goto fail; + } + data->lbshift = lbshift; + /* Search for Volume Recognition Sequence (VRS). */ - for (block = 16;; block++) + for (vblock = (32767 >> (lbshift + GRUB_DISK_SECTOR_BITS)) + 1;; + vblock += (2047 >> (lbshift + GRUB_DISK_SECTOR_BITS)) + 1) { struct grub_udf_vrs vrs; - if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0, + if (grub_disk_read (disk, vblock << lbshift, 0, sizeof (struct grub_udf_vrs), &vrs)) { grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem"); @@ -545,39 +580,13 @@ } } - /* Search for Anchor Volume Descriptor Pointer (AVDP). */ - while (1) - { - struct grub_udf_avdp avdp; - - if (grub_disk_read (disk, *sblklist << GRUB_UDF_LOG2_BLKSZ, 0, - sizeof (struct grub_udf_avdp), &avdp)) - { - grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem"); - goto fail; - } - - if (U16 (avdp.tag.tag_ident) == GRUB_UDF_TAG_IDENT_AVDP) - { - block = U32 (avdp.vds.start); - break; - } - - sblklist++; - if (*sblklist == 0) - { - grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem"); - goto fail; - } - } - data->npd = data->npm = 0; /* Locate Partition Descriptor (PD) and Logical Volume Descriptor (LVD). */ while (1) { struct grub_udf_tag tag; - if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0, + if (grub_disk_read (disk, block << lbshift, 0, sizeof (struct grub_udf_tag), &tag)) { grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem"); @@ -593,7 +602,7 @@ goto fail; } - if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0, + if (grub_disk_read (disk, block << lbshift, 0, sizeof (struct grub_udf_pd), &data->pds[data->npd])) { @@ -609,7 +618,7 @@ struct grub_udf_partmap *ppm; - if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0, + if (grub_disk_read (disk, block << lbshift, 0, sizeof (struct grub_udf_lvd), &data->lvd)) { @@ -673,7 +682,7 @@ if (grub_errno) goto fail; - if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0, + if (grub_disk_read (disk, block << lbshift, 0, sizeof (struct grub_udf_fileset), &root_fs)) { grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem"); [-- Attachment #7: 3-udf-generic-block-size-v2.changelog --] [-- Type: application/octet-stream, Size: 1140 bytes --] Add generic logical block size support. * grub-core/fs/udf.c (GRUB_UDF_LOG2_BLKSIZE): Removed macro. (GRUB_UDF_BLKSZ): Removed macro. (struct grub_udf_data): New field "lbshift" to hold the logical block size of the file system in log2 format. (grub_udf_read_icb): Replace usage of macro GRUB_UDF_LOG2_BLKSZ with field lbshift from struct grub_udf_data. (grub_udf_read_file): Likewise. (grub_udf_read_block): Replace usage of macro GRUB_UDF_BLKSZ with field "lvd.bsize" from struct grub_udf_data. Replace division with right shift. (sblocklist): Change type to unsigned. (grub_udf_mount): Change type of "sblklist" to unsigned. New variable "vblock" to be used during VRS recognition. New variable "lbshift" to hold the detected logical block size. Move AVDP search before VRS recognition, because the latter requires knowledge of the logical block size, which is detected during the former. Detect and validate logical block size during AVDP search, adding support for block sizes 512, 1024 and 4096. Make VRS recognition independent of block size. Replace usages of macro GRUB_UDF_LOG2_BLKSZ with variable lbshift. [-- Attachment #8: 4-udf-ad-extents-v2.patch --] [-- Type: application/octet-stream, Size: 5511 bytes --] diff -Naur grub-old/grub-core/fs/udf.c grub-new/grub-core/fs/udf.c --- grub-old/grub-core/fs/udf.c 2010-11-09 09:23:53.713106502 +0100 +++ grub-new/grub-core/fs/udf.c 2010-11-09 09:21:08.351056468 +0100 @@ -333,6 +333,13 @@ grub_uint8_t part_maps[1608]; } __attribute__ ((packed)); +struct grub_udf_aed +{ + struct grub_udf_tag tag; + grub_uint32_t prev_ae; + grub_uint32_t ae_len; +} __attribute__ ((packed)); + struct grub_udf_data { grub_disk_t disk; @@ -403,19 +410,26 @@ static grub_disk_addr_t grub_udf_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock) { + char *buf = NULL; char *ptr; - int len; + grub_ssize_t len; grub_disk_addr_t filebytes; - if (U16 (node->fe.tag.tag_ident) == GRUB_UDF_TAG_IDENT_FE) + switch (U16 (node->fe.tag.tag_ident)) { + case GRUB_UDF_TAG_IDENT_FE: ptr = (char *) &node->fe.ext_attr[0] + U32 (node->fe.ext_attr_length); len = U32 (node->fe.alloc_descs_length); - } - else - { + break; + + case GRUB_UDF_TAG_IDENT_EFE: ptr = (char *) &node->efe.ext_attr[0] + U32 (node->efe.ext_attr_length); len = U32 (node->efe.alloc_descs_length); + break; + + default: + grub_error (GRUB_ERR_BAD_FS, "invalid file entry"); + return 0; } if ((U16 (node->fe.icbtag.flags) & GRUB_UDF_ICBTAG_FLAG_AD_MASK) @@ -423,45 +437,115 @@ { struct grub_udf_short_ad *ad = (struct grub_udf_short_ad *) ptr; - len /= sizeof (struct grub_udf_short_ad); filebytes = fileblock * U32 (node->data->lvd.bsize); - while (len > 0) + while (len >= (grub_ssize_t) sizeof (struct grub_udf_short_ad)) { - if (filebytes < U32 (ad->length)) - return ((U32 (ad->position) & GRUB_UDF_EXT_MASK) ? 0 : - (grub_udf_get_block (node->data, - node->part_ref, - ad->position) - + (filebytes >> (GRUB_DISK_SECTOR_BITS - + node->data->lbshift)))); + grub_uint32_t adlen = U32 (ad->length) & 0x3fffffff; + grub_uint32_t adtype = U32 (ad->length) >> 30; + if (adtype == 3) + { + struct grub_udf_aed *extension; + grub_disk_addr_t sec = grub_udf_get_block(node->data, + node->part_ref, + ad->position); + if (!buf) + { + buf = grub_malloc (U32 (node->data->lvd.bsize)); + if (!buf) + return 0; + } + if (grub_disk_read (node->data->disk, sec << node->data->lbshift, + 0, adlen, buf)) + goto fail; + + extension = (struct grub_udf_aed *) buf; + if (U16 (extension->tag.tag_ident) != GRUB_UDF_TAG_IDENT_AED) + { + grub_error (GRUB_ERR_BAD_FS, "invalid aed tag"); + goto fail; + } + + len = U32 (extension->ae_len); + ad = (struct grub_udf_short_ad *) + (buf + sizeof (struct grub_udf_aed)); + continue; + } + + if (filebytes < adlen) + { + grub_uint32_t ad_pos = ad->position; + grub_free (buf); + return ((U32 (ad_pos) & GRUB_UDF_EXT_MASK) ? 0 : + (grub_udf_get_block (node->data, node->part_ref, ad_pos) + + (filebytes >> (GRUB_DISK_SECTOR_BITS + + node->data->lbshift)))); + } - filebytes -= U32 (ad->length); + filebytes -= adlen; ad++; - len--; + len -= sizeof (struct grub_udf_short_ad); } } else { struct grub_udf_long_ad *ad = (struct grub_udf_long_ad *) ptr; - len /= sizeof (struct grub_udf_long_ad); filebytes = fileblock * U32 (node->data->lvd.bsize); - while (len > 0) + while (len >= (grub_ssize_t) sizeof (struct grub_udf_long_ad)) { - if (filebytes < U32 (ad->length)) - return ((U32 (ad->block.block_num) & GRUB_UDF_EXT_MASK) ? 0 : - (grub_udf_get_block (node->data, - ad->block.part_ref, - ad->block.block_num) - + (filebytes >> (GRUB_DISK_SECTOR_BITS - + node->data->lbshift)))); + grub_uint32_t adlen = U32 (ad->length) & 0x3fffffff; + grub_uint32_t adtype = U32 (ad->length) >> 30; + if (adtype == 3) + { + struct grub_udf_aed *extension; + grub_disk_addr_t sec = grub_udf_get_block(node->data, + ad->block.part_ref, + ad->block.block_num); + if (!buf) + { + buf = grub_malloc (U32 (node->data->lvd.bsize)); + if (!buf) + return 0; + } + if (grub_disk_read (node->data->disk, sec << node->data->lbshift, + 0, adlen, buf)) + goto fail; + + extension = (struct grub_udf_aed *) buf; + if (U16 (extension->tag.tag_ident) != GRUB_UDF_TAG_IDENT_AED) + { + grub_error (GRUB_ERR_BAD_FS, "invalid aed tag"); + goto fail; + } + + len = U32 (extension->ae_len); + ad = (struct grub_udf_long_ad *) + (buf + sizeof (struct grub_udf_aed)); + continue; + } + + if (filebytes < adlen) + { + grub_uint32_t ad_block_num = ad->block.block_num; + grub_uint32_t ad_part_ref = ad->block.part_ref; + grub_free (buf); + return ((U32 (ad_block_num) & GRUB_UDF_EXT_MASK) ? 0 : + (grub_udf_get_block (node->data, ad_part_ref, + ad_block_num) + + (filebytes >> (GRUB_DISK_SECTOR_BITS + + node->data->lbshift)))); + } - filebytes -= U32 (ad->length); + filebytes -= adlen; ad++; - len--; + len -= sizeof (struct grub_udf_long_ad); } } +fail: + if (buf) + grub_free (buf); + return 0; } [-- Attachment #9: 4-udf-ad-extents-v2.changelog --] [-- Type: application/octet-stream, Size: 515 bytes --] Add support for allocation extent descriptors, needed on fragmented volumes. * grub-core/fs/udf.c (grub_udf_aed): New struct. (grub_udf_read_block): Change type of variable "len" to grub_ssize_t. Add sanity check for file entry type. Replace divisions with right shifts. Check the upper bits of the allocation descriptor's length and honour them by loading an allocation extent descriptor if they indicate so. Change all failure return paths to free the allocation extent descriptor if it was allocated. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: UDF filesystem fixes for grub 2010-11-11 8:56 ` Giuseppe Caizzone @ 2010-11-14 16:05 ` Vladimir 'φ-coder/phcoder' Serbinenko 0 siblings, 0 replies; 4+ messages in thread From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-11-14 16:05 UTC (permalink / raw) To: grub-devel [-- Attachment #1: Type: text/plain, Size: 6939 bytes --] All patches committed Thanks On 11/11/2010 09:56 AM, Giuseppe Caizzone wrote: > 2010/11/6 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>: > >> On 11/02/2010 02:10 PM, Giuseppe Caizzone wrote: >> >>> Hello, >>> >>> I've made 4 small patches that improve the UDF filesystem support in >>> grub. With these changes, I've been able to read any regular file in >>> UDF filesystems created by both Linux and Windows on both optical >>> storage and USB sticks. >>> [...] >>> 1/4) Support for large (> 2GiB) files >>> [...] >>> The patch just changes an int and a grub_uint32_t to be grub_off_t. >>> >>> >>> >> Good patch. Can you write the ChangeLog entry for it? >> > OK - I've never written one, so I tried to mimic grub's. > > Support reading files larger than 2 GiB. > > * grub-core/fs/udf.c (grub_udf_iterate_dir): change type of variable > "offset" from grub_uint32_t to grub_off_t. > (grub_udf_read_file): change type of parameter "pos" from int to > grub_off_t. > > >>> 2/4) Support for deleted files >>> [...] >>> The patch just skips directory entries with the "deleted" bit set, in >>> grub_udf_iterate_dir(). >>> >>> >>> >> This one is good too. Changelog entry? >> > Properly handle deleted files. > > * grub-core/fs/udf.c (grub_udf_iterate_dir): Skip directory entries > whose "characteristics" field has the bit GRUB_UDF_FID_CHAR_DELETED > set. > > >>> 3/4) Support for a generic block size >>> [...] >>> The patch repeats the superblock search in grub_udf_mount() for 512, >>> 1024, 2048 and 4096 block sizes, stopping if it finds a valid one AND >>> the sector offset recorded in that superblock matches the detected >>> superblock offset. So for instance it won't misdetect a superblock >>> located at sector 256 with blocksize 2048 for a superblock located at >>> sector 512 with blocksize 1024. >>> The log2(blocksize/512) is then stored in a new field called "lbshift" >>> in struct grub_udf_data, which gets used instead of the previous >>> constant GRUB_UDF_LOG2_BLKSZ, while the constant GRUB_UDF_BLKSZ gets >>> replaced with the lvd.bsize field already present in struct >>> grub_udf_data. >>> >>> >>> >> + lbshift = 0; >> + block = 0; >> + while (lbshift <= 3) >> + { >> For loop is way more appropriate than a while loop >> > Changed it into a for loop. > > >> + while (lbshift <= 3) >> + { >> + sblklist = sblocklist; >> + while (*sblklist) >> + { >> Same here. >> > Changed that too. > > I also made another change since the previous version of the patch: I > moved the VRS check after the AVDP search, because it needs to know > the logical block size. I originally thought that it wasn't necessary, > because the VRS is made up of records with a fixed size of 2048 bytes, > but the catch is that the standard says that each record must start on > a new sector, so if block size is > 2048, one has to skip more bytes > than 2048 to get to the next record. Tested it with mkudffs because I > have no medium with an actual block size of 4096. > > >> ChangeLog entry? >> > Add generic logical block size support. > > * grub-core/fs/udf.c (GRUB_UDF_LOG2_BLKSIZE): Removed macro. > (GRUB_UDF_BLKSZ): Removed macro. > (struct grub_udf_data): New field "lbshift" to hold the logical block > size of the file system in log2 format. > (grub_udf_read_icb): Replace usage of macro GRUB_UDF_LOG2_BLKSZ with > field lbshift from struct grub_udf_data. > (grub_udf_read_file): Likewise. > (grub_udf_read_block): Replace usage of macro GRUB_UDF_BLKSZ with > field "lvd.bsize" from struct grub_udf_data. > Replace division with right shift. > (sblocklist): Change type to unsigned. > (grub_udf_mount): Change type of "sblklist" to unsigned. > New variable "vblock" to be used during VRS recognition. > New variable "lbshift" to hold the detected logical block size. > Move AVDP search before VRS recognition, because the latter requires > knowledge of the logical block size, which is detected during the > former. > Detect and validate logical block size during AVDP search, adding > support for block sizes 512, 1024 and 4096. > Make VRS recognition independent of block size. > Replace usages of macro GRUB_UDF_LOG2_BLKSZ with variable lbshift. > > >>> 4/4) Support for allocation descriptor extensions >>> [...] >>> The patch checks, in grub_udf_read_block(), whether the "allocation >>> descriptor type", previously ignored, is 3, and in this case, it loads >>> the referenced block, checks that it's a valid allocation extension >>> descriptor, and sets the "ad" pointer (and the "len" limit) to >>> continue the iteration from the allocation descriptors contained in >>> that block. >>> >>> This last one is the only patch which actually contains a proper new >>> block of code, and it allocates a big structure on the stack (it's a >>> sector, so it's up to 4K big): I don't know if this is OK, or if >>> perhaps I should use grub_malloc() instead. Also, the patch depends on >>> the previous "generic block size" patch. >>> >>> >>> >> + char buf[U32 (node->data->lvd.bsize)]; >> Will segfault if bsize is too big. You need to allocate on heap >> > Done. > > >> - else >> + else if (U16 (node->fe.tag.tag_ident == GRUB_UDF_TAG_IDENT_EFE)) >> { >> Parenthesis are wrong. >> > While I was at it, I changed the 'if-else if' to a switch. > > >> + grub_disk_addr_t sec = grub_udf_get_block(node->data, >> + node->part_ref, >> + ad->position); >> ad->position isn't byte-swapped. >> > grub_udf_get_block() byte-swaps it itself, so I must not byte-swap it before. > > >> ChangeLog entry? >> > Add support for allocation extent descriptors, needed on fragmented > volumes. > > * grub-core/fs/udf.c (grub_udf_aed): New struct. > (grub_udf_read_block): Change type of variable "len" to grub_ssize_t. > Add sanity check for file entry type. > Replace divisions with right shifts. > Check the upper bits of the allocation descriptor's length and honour > them by loading an allocation extent descriptor if they indicate so. > Change all failure return paths to free the allocation extent > descriptor if it was allocated. > > I'm attaching the full patch set (including the unchanged ones for > convenience), and also the change log entries in attachment form in > case gmail tampers with whitespace. > > Thank you, > > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > -- Regards Vladimir 'φ-coder/phcoder' Serbinenko [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 294 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-11-14 16:05 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-02 13:10 UDF filesystem fixes for grub Giuseppe Caizzone 2010-11-06 19:59 ` Vladimir 'φ-coder/phcoder' Serbinenko 2010-11-11 8:56 ` Giuseppe Caizzone 2010-11-14 16:05 ` Vladimir 'φ-coder/phcoder' Serbinenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).